cmd/compile: add line numbers for values and blocks at HTML SSA output? #17314

Open
dAdAbird opened this Issue Oct 1, 2016 · 8 comments

Comments

Projects
None yet
7 participants
@dAdAbird
Contributor

dAdAbird commented Oct 1, 2016

We already have it in the "genssa" section, and it's pretty simplifies understanding what's going on. I propose to do the same for other sections of the output.

Since writing (sourcefile.go:42) will make output noisier it would be better to show just line numbers in the left column and make them gray colored and with smaller font size than the code (so it looks like in editors).

If you ok with that I can show my vision of design for approval and then provide CL.

@quentinmit quentinmit changed the title from Proposal: cmd/compile: add line numbers for values and blocks at HTML SSA output to proposal: cmd/compile: add line numbers for values and blocks at HTML SSA output Oct 3, 2016

@quentinmit quentinmit added the Proposal label Oct 3, 2016

@quentinmit quentinmit added this to the Proposal milestone Oct 3, 2016

@randall77

This comment has been minimized.

Show comment
Hide comment
@randall77

randall77 Oct 3, 2016

Contributor

I find the html output pretty cluttered already. I feel like adding line numbers would only make that worse. I guess I'm not philosophically opposed, but I'd like to see some more arguments for why it is useful. I've never encountered a situation where line numbers might be useful (except maybe for debugging line number generation).

Contributor

randall77 commented Oct 3, 2016

I find the html output pretty cluttered already. I feel like adding line numbers would only make that worse. I guess I'm not philosophically opposed, but I'd like to see some more arguments for why it is useful. I've never encountered a situation where line numbers might be useful (except maybe for debugging line number generation).

@dAdAbird

This comment has been minimized.

Show comment
Hide comment
@dAdAbird

dAdAbird Oct 4, 2016

Contributor

Line numbers may be useful to link SSA instructions with the function code. It gives a better understanding of how the Go code represents in SSA form. Especially, that is actual for people who just start dealing with this.
For example, line numbers in the "genssa" output really help to figure out what asm instructions does when you can see how it relates to your code.

On the other hand, I agree that it'll add more noise into HTML output.
But what if to do something like that:
line numbers proof of concept

Contributor

dAdAbird commented Oct 4, 2016

Line numbers may be useful to link SSA instructions with the function code. It gives a better understanding of how the Go code represents in SSA form. Especially, that is actual for people who just start dealing with this.
For example, line numbers in the "genssa" output really help to figure out what asm instructions does when you can see how it relates to your code.

On the other hand, I agree that it'll add more noise into HTML output.
But what if to do something like that:
line numbers proof of concept

@dr2chase

This comment has been minimized.

Show comment
Hide comment
@dr2chase

dr2chase Oct 5, 2016

Contributor

Could it be switchable? There's a mess of Javascript in there already, this might not be too much.

One sometime use of the SSA output is to copy a column out for processing in an editor (e.g., diffing against another compilation of same code), so a mitigating bell/whistle might be a "copy" button for each column, since those line numbers might be mess up the copy (which sometimes requires post-processing anyhow).

Contributor

dr2chase commented Oct 5, 2016

Could it be switchable? There's a mess of Javascript in there already, this might not be too much.

One sometime use of the SSA output is to copy a column out for processing in an editor (e.g., diffing against another compilation of same code), so a mitigating bell/whistle might be a "copy" button for each column, since those line numbers might be mess up the copy (which sometimes requires post-processing anyhow).

@dAdAbird

This comment has been minimized.

Show comment
Hide comment
@dAdAbird

dAdAbird Oct 5, 2016

Contributor

The other solution is to prevent line numbers from being selected and copied with CSS. Without any JS and additional elements.

screen shot 2016-10-05 at 16 39 28

Contributor

dAdAbird commented Oct 5, 2016

The other solution is to prevent line numbers from being selected and copied with CSS. Without any JS and additional elements.

screen shot 2016-10-05 at 16 39 28

@dr2chase

This comment has been minimized.

Show comment
Hide comment
@dr2chase

dr2chase Oct 5, 2016

Contributor

Just now realized, what happens with line numbers when we inline functions, and how would we want to express that, assuming we haven't made a mess of that information already in the compiler? Color-coded? Tool-tips for the function name?

Note that once we start inlining calls that contain safepoints, we'll need to get this right anyhow for stack traces and debugging.

Contributor

dr2chase commented Oct 5, 2016

Just now realized, what happens with line numbers when we inline functions, and how would we want to express that, assuming we haven't made a mess of that information already in the compiler? Color-coded? Tool-tips for the function name?

Note that once we start inlining calls that contain safepoints, we'll need to get this right anyhow for stack traces and debugging.

@dAdAbird

This comment has been minimized.

Show comment
Hide comment
@dAdAbird

dAdAbird Oct 17, 2016

Contributor

Yeah, that's a good point.

So, I'd like to add that information into compiler (add CallLine and CallFile values into ssa.Value, ssa.Block and gc.Node types) and add DW_AT_call_file and DW_AT_call_line attributes into DWARF. As you mentioned, we'll need it anyhow.

As for the HTML output, what if to mark inlined instructions with a color and show more info on mouse over. Something like this:
screen shot 2016-10-17 at 19 14 14

Contributor

dAdAbird commented Oct 17, 2016

Yeah, that's a good point.

So, I'd like to add that information into compiler (add CallLine and CallFile values into ssa.Value, ssa.Block and gc.Node types) and add DW_AT_call_file and DW_AT_call_line attributes into DWARF. As you mentioned, we'll need it anyhow.

As for the HTML output, what if to mark inlined instructions with a color and show more info on mouse over. Something like this:
screen shot 2016-10-17 at 19 14 14

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Nov 7, 2016

Member

Deferring to @randall77 to decide. Removing proposal label.

Member

bradfitz commented Nov 7, 2016

Deferring to @randall77 to decide. Removing proposal label.

@bradfitz bradfitz removed the Proposal label Nov 7, 2016

@bradfitz bradfitz changed the title from proposal: cmd/compile: add line numbers for values and blocks at HTML SSA output to cmd/compile: add line numbers for values and blocks at HTML SSA output? Nov 7, 2016

@rsc rsc added the NeedsDecision label Nov 7, 2016

@rsc rsc modified the milestones: Go1.9, Proposal Nov 7, 2016

@randall77

This comment has been minimized.

Show comment
Hide comment
@randall77

randall77 Nov 15, 2016

Contributor

Adding this is ok, but please put it behind a flag that is off by default.

Contributor

randall77 commented Nov 15, 2016

Adding this is ok, but please put it behind a flag that is off by default.

@josharian josharian modified the milestones: Unplanned, Go1.9 May 11, 2017

@rsc rsc added NeedsFix and removed NeedsDecision labels May 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment