golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.04k stars 17.54k forks source link

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

Open niaow opened 3 years ago

niaow commented 3 years ago

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 commented 3 years ago

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.

josharian commented 3 years ago

cc @zdjones

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

zdjones commented 3 years ago

Removing the bounds check falls under #25086