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: regalloc can leave unused instructions after performing rematerialization and removing the only dependent #67911

Closed
renthraysk opened this issue Jun 9, 2024 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.

Comments

@renthraysk
Copy link

Go version

go version go1.22.4 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/ren/.cache/go-build'
GOENV='/home/ren/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/ren/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/ren/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.4'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/ren/dev/junk/bugXXX/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1837702511=/tmp/go-build -gno-record-gcc-switches'

What did you do?

func isIn(c byte, lo, hi uint64) bool {
	var m uint64
	if c < 128 {
		m = hi
		if c < 64 {
			m = lo
		}
	}
	return (1<<(c%64))&m != 0
}

https://go.godbolt.org/z/EMn16qvPY

What did you see happen?

On amd64, an unnecessary CMPB instruction is generated, immediately followed by another CMPB.

CMPB	AL, $-128
CMPB	AL, $64
CMOVQCS	BX, CX
CMPB	AL, $-128
MOVL	$0, DX
CMOVQCS	CX, DX
BTQ	AX, DX
SETCS	AL
RET

Similarly on arm64, two CMPWs one after the other.

MOVBU	R0, R3
MOVD	$1, R4
LSL	R0, R4, R4
CMPW	$128, R3
CMPW	$64, R3
CSEL	LO, R1, R2, R1
CMPW	$128, R3
CSEL	LO, R1, ZR, R1
TST	R1, R4
CSET	NE, R0
RET	(R30)

What did you expect to see?

No unnecessary compares.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 9, 2024
@Jorropo
Copy link
Member

Jorropo commented Jun 9, 2024

Thx for the bug report.
Minimized example:

func isIn(c byte, lo, hi uint64) uint64 {
	var m uint64
	if c < 128 {
		m = hi
		if c < 64 {
			m = lo
		}
	}
	return m
}

This is probably fine, as in completely free, the compare are fast and this one does not havet have any dependencies.
There is still cost in fetching and caching bytecodes and maybe sometime it makes loops unaligned but that chance.

@Jorropo Jorropo changed the title cmd/compile: extraneous compare instructions when compiler rewrites code to use conditional moves cmd/compile: regalloc can leave unused instructions after performing rematerialization and removing the only dependent Jun 9, 2024
@Jorropo Jorropo assigned Jorropo and unassigned Jorropo Jun 9, 2024
@gabyhelp
Copy link

gabyhelp commented Jun 9, 2024

@Jorropo
Copy link
Member

Jorropo commented Jun 9, 2024

Dup of #65039
Nice bot

@Jorropo Jorropo closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

4 participants