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: duplicated panicslice calls in a single function #30437

Closed
seebs opened this issue Feb 27, 2019 · 3 comments
Closed

cmd/compile: duplicated panicslice calls in a single function #30437

seebs opened this issue Feb 27, 2019 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.13

Comments

@seebs
Copy link
Contributor

seebs commented Feb 27, 2019

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

1.10/1.11 (on godbolt.org)

Does this issue reproduce with the latest release?

Not sure, godbolt doesn't have 1.12 yet.

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

x86

What did you do?

Messed with some benchmarks, looked to see whether they were generating bounds checks.

What did you expect to see?

Less bounds checking.

What did you see instead?

This isn't actually about the amount of bounds checking generated. It's about the fact that identical blocks of code were generated several times consecutively.

https://godbolt.org/z/IfZOAD

Look in particular around lines 172-180 or so of the go1.11 output, or 134-150 of the go1.10 output. There's multiple instances of identical panic calls with distinct labels:

XorBytesUnrolled_pc198:
        pcdata  $2, $0
        pcdata  $0, $3
        call    runtime.panicslice(SB)
        undef
XorBytesUnrolled_pc205:
        call    runtime.panicslice(SB)
        undef
XorBytesUnrolled_pc212:
        call    runtime.panicslice(SB)
        undef
XorBytesUnrolled_pc219:
        call    runtime.panicslice(SB)
        undef

I wouldn't expect the compiler to realize that it could insert a label halfway through the _pc198 code, but I would expect it to be able to recognize that 205, 212, and 219 are all the same code.

(I was also hoping the bounds checker wouldn't need to generate those at all, but we can't have everything.)
@ALTree
Copy link
Member

ALTree commented Feb 27, 2019 •
edited

Is it possible to merge those calls while also keeping accurate line numbers in tracebacks? They are for difference source code lines..

@ALTree ALTree changed the title duplicated panicslice calls in a single function cmd/compile: duplicated panicslice calls in a single function Feb 27, 2019
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 27, 2019
@ALTree ALTree added this to the Go1.13 milestone Feb 27, 2019
@seebs
Copy link
Contributor Author

seebs commented Feb 27, 2019

Probably not. I hadn't thought about the traceback line numbers. I thought this looked too easy...

@randall77
Copy link
Contributor

@ALTree is right.
We do merge panicslice calls if the slice expressions are both on the same line.

@golang golang locked and limited conversation to collaborators Feb 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
4 participants
@seebs @ALTree @randall77 @gopherbot