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

Jump to bottom

cmd/compile: duplicate blocks calling runtime.panicXXX in some procedures #24478

Closed
laboger opened this issue Mar 21, 2018 · 3 comments
Closed

cmd/compile: duplicate blocks calling runtime.panicXXX in some procedures #24478

laboger opened this issue Mar 21, 2018 · 3 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unplanned

Comments

@laboger
Copy link
Contributor

laboger commented Mar 21, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

tip

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

Tried ppc64le and x86

What did you do?

While looking at some objdumps I noticed some functions that had several blocks of code at the end of the procedure with calls to runtime.panicXXX functions. In some cases there were many duplicate blocks calling the same function.

What did you expect to see?

The blocks containing identical calls could be collapsed into a single block, with all predecessors branching to the same block.

What did you see instead?

Here is an example on ppc64le from function runtime.selectgo. Similar code appears in the x86 objdump.

    100402a0:   81 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402a4:   00 00 00 00     .long 0x0
    100402a8:   79 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402ac:   00 00 00 00     .long 0x0
    100402b0:   71 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402b4:   00 00 00 00     .long 0x0
    100402b8:   69 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402bc:   00 00 00 00     .long 0x0
    100402c0:   61 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402c4:   00 00 00 00     .long 0x0
    100402c8:   59 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402cc:   00 00 00 00     .long 0x0
    100402d0:   51 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402d4:   00 00 00 00     .long 0x0
    100402d8:   49 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402dc:   00 00 00 00     .long 0x0
    100402e0:   41 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402e4:   00 00 00 00     .long 0x0
    100402e8:   39 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402ec:   00 00 00 00     .long 0x0
    100402f0:   31 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402f4:   00 00 00 00     .long 0x0
    100402f8:   29 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    100402fc:   00 00 00 00     .long 0x0
    10040300:   21 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    10040304:   00 00 00 00     .long 0x0
    10040308:   19 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    1004030c:   00 00 00 00     .long 0x0
    10040310:   11 b9 fe 4b     bl      1002bc20 <runtime.panicindex>
    10040314:   00 00 00 00     .long 0x0

Fixing this would reduce the size of the function, possibly enabling more inlining, and reduce the size of the binary.
@randall77
Copy link
Contributor

There are multiple calls because each corresponds to a different source line. We wouldn't get accurate line numbers in tracebacks if we coalesced all the panicindex calls in a function.

We do combine multiple identical panics from the same line. For instance, a[i] + b[j] should only generate a single panicindex call.

@andybons
Copy link
Member

@randall77 what is the next step, here? Are you saying this should be closed or is more investigation needed?

@andybons andybons added this to the Unplanned milestone Mar 21, 2018
@andybons andybons added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 21, 2018
@randall77
Copy link
Contributor

I guess we should close this unless someone comes up with a better way to encode line numbers during panic.

@golang golang locked and limited conversation to collaborators Mar 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@andybons @randall77 @gopherbot @laboger