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: bits.Len of constant not constant-folded #58588

Closed
greatroar opened this issue Feb 18, 2023 · 2 comments
Closed

cmd/compile: bits.Len of constant not constant-folded #58588

greatroar opened this issue Feb 18, 2023 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@greatroar
Copy link

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

$ gotip version
go version devel go1.21-169203f3ee Sat Feb 18 05:22:26 2023 +0000 linux/amd64

What did you do?

b >>= 8 - bits.Len(100)

What did you expect to see?

A single shift instruction after constant folding.

What did you see instead?

With GOARCH=amd64, GOAMD64=v2:

MOVL    $100, DX
BSRQ    DX, CX
MOVQ    $-1, DX
CMOVQEQ DX, CX
ADDQ    $-7, CX
NEGQ    CX
NOP
TESTQ   CX, CX
JLT     49
SHRB    CL, AL
CMPQ    CX, $8
SBBL    DX, DX
ANDL    DX, AX
// ...
CALL    runtime.panicshift(SB)
XCHGL   AX, AX

Adding a uint cast gets rid of the panicshift call, but still produces more than a dozen instructions.

GOAMD64=v3 produces shorter, but still suboptimal code.

Interestingly, GOARCH=386 does trigger constant folding, but still gets a useless check for 1 < 0:

MOVL    $1, AX
TESTL   AX, AX
JLT     40
MOVBLZX command-line-arguments.b+4(SP), AX
SHRB    $1, AX
// ...
CALL    runtime.panicshift(SB)
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 18, 2023
@randall77 randall77 added this to the Unplanned milestone Feb 18, 2023
@randall77 randall77 added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 18, 2023
@renthraysk
Copy link

renthraysk commented Feb 18, 2023

Pretty sure this has come up before?

#21872

@greatroar
Copy link
Author

Oh, I thought that bits.Len was supposed to be constant-folded based on the 386 behavior and amd64 was just broken. Closing as duplicate.

@golang golang locked and limited conversation to collaborators Feb 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

4 participants