Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: use expandable columns in ssa.html? #25286

Closed
josharian opened this issue May 8, 2018 · 5 comments
Closed

cmd/compile: use expandable columns in ssa.html? #25286

josharian opened this issue May 8, 2018 · 5 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@josharian
Copy link
Contributor

As the number of SSA passes has grown, ssa.html has gotten wider and thus harder to navigate. I'd like to float the idea of having the initial display show just a few columns, with the others be expandable stubs (with names). As to the initial list of columns, maybe something like: start, after early deadcode, after opt, after lower, after late deadcode, after regalloc, genssa.

There's also the question of whether this can be reasonably implemented in standalone javascript, a question about which I know little.

Opinions?

cc @dr2chase @randall77 @cherrymui

@josharian josharian added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 8, 2018
@josharian josharian added this to the Unplanned milestone May 8, 2018
@randall77
Copy link
Contributor

Yes, please.
It is not possible that your javascript knowledge is less than mine.

@ALTree ALTree added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 8, 2018
@ysmolski
Copy link
Member

ysmolski commented Jun 7, 2018

@josharian is it okay when you reload ssa.html [in a browser] you would get to default state of collapsed columns?

@josharian
Copy link
Contributor Author

Yes. It’d be nice to do some saved local state or query params, but as a first pass, I think always reverting to the default is fine. In the normal case I imagine the reader will be interested in one or two columns at most, so restoring state manually isn’t that big a deal.

@gopherbot
Copy link

Change https://golang.org/cl/117275 mentions this issue: cmd/compile: use expandable columns in ssa.html

@ysmolski ysmolski self-assigned this Jun 8, 2018
@ysmolski
Copy link
Member

ysmolski commented Jun 11, 2018

In case if you want to give a feedback without downloading the patch above:

screen shot 2018-06-12 at 00 12 42

screen shot 2018-06-12 at 00 13 50

The vertical text (a column) can be expanded by clicking on it. If you want something else, just let me know.

I would be glad to help because I learned a thing or two about SSA while making the change. Also I would like to implement some of the TODOs buired in ssa.go regarding graphic representation (nodes->ssa?) or maybe just dumping the log from ssa to the ssa.html, so file can be shared as standalone piece.
/cc @josharian

EDITED: typos and formatting

dna2github pushed a commit to dna2fork/go that referenced this issue Jun 14, 2018
Display just a few columns in ssa.html, other
columns can be expanded by clicking on collapsed column.

Use sans serif font for the text, slightly smaller font size
for non program text.

Fixes golang#25286

Change-Id: I1094695135401602d90b97b69e42f6dda05871a2
Reviewed-on: https://go-review.googlesource.com/117275
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@golang golang locked and limited conversation to collaborators Jun 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants