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.

Sign up for GitHub

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: negative shift check with trailing zeros #42094

Open
niaow opened this issue Oct 20, 2020 · 3 comments
Open

cmd/compile: negative shift check with trailing zeros #42094

niaow opened this issue Oct 20, 2020 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Unplanned

Comments

@niaow
Copy link

niaow commented Oct 20, 2020

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

$ go version
go version go1.15.2 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build070218119=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tried to remove the trailing bit in a loop.
A simple example of this pattern which counts each bit: https://play.golang.org/p/iQc490A5DRV
Same thing with instruction dump: https://godbolt.org/z/E54rY4

What did you expect to see?

The compiler produces a tight loop, optimizing away the negative shift check.
It would be nice if the bounds check could be eliminated too, but that does not seem to be quite as straightforward to prove.

What did you see instead?

There is a runtime check to see if bits.TrailingZeros64 is negative.
@randall77
Copy link
Contributor

Yes, it would be nice if the compiler knew the output range of the bit ops so it could optimize its uses.

The optimizer in me notes that you can do v &= v - 1 instead of v &^= 1 << i.

@randall77 randall77 added this to the Unplanned milestone Oct 20, 2020
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 20, 2020
@josharian
Copy link
Contributor

cc @zdjones

The compiler knows a bunch of tricks along these lines, so it mightn't be too hard to add this one.

@zdjones
Copy link
Contributor

zdjones commented Nov 27, 2020

Removing the bounds check falls under #25086

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@josharian @cagedmantis @zdjones @randall77 @gopherbot @niaow