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: flagalloc doesn't delete dead instructions #65039

Open
dominikh opened this issue Jan 9, 2024 · 4 comments
Open

cmd/compile: flagalloc doesn't delete dead instructions #65039

dominikh opened this issue Jan 9, 2024 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone
Unplanned

Comments

@dominikh
Copy link
Member

dominikh commented Jan 9, 2024

Go version

go version devel go1.22-b7c630dc3a Tue Jan 9 01:36:54 2024 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/dominikh/.cache/go-build'
GOENV='/home/dominikh/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/dominikh/prj/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/dominikh/prj'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/dominikh/prj/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/dominikh/prj/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.22-b7c630dc3a Tue Jan 9 01:36:54 2024 +0000'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/dominikh/prj/src/example.com/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-build1294566987=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Compile https://go.dev/play/p/4RMX7fsEHw9

package pkg

import (
	"math/bits"
)

var Sink uint

func Foo(x uint64) {
	log2 := uint(63 - bits.LeadingZeros64(x))
	if x&(x-1) != 0 {
		log2++
	}
	Sink = log2
}

What did you see happen?

The generated assembly contains a dead TESTQ instruction at 0x0005

command-line-arguments.Foo STEXT nosplit size=42 args=0x8 locals=0x0 funcid=0x0 align=0x0
	0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:9)	TEXT	command-line-arguments.Foo(SB), NOSPLIT|NOFRAME|ABIInternal, $0-8
	0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:9)	FUNCDATA	$0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
	0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:9)	FUNCDATA	$1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
	0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:9)	FUNCDATA	$5, command-line-arguments.Foo.arginfo1(SB)
	0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:9)	FUNCDATA	$6, command-line-arguments.Foo.argliveinfo(SB)
	0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:9)	PCDATA	$3, $1
	0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:11)	LEAQ	-1(AX), CX
	0x0004 00004 (/home/dominikh/prj/src/example.com/foo.go:10)	XCHGL	AX, AX
	0x0005 00005 (/home/dominikh/prj/src/example.com/foo.go:11)	TESTQ	AX, CX
	0x0008 00008 (/home/dominikh/prj/go/src/math/bits/bits.go:37)	BSRQ	AX, DX
	0x000c 00012 (/home/dominikh/prj/go/src/math/bits/bits.go:37)	MOVQ	$-1, BX
	0x0013 00019 (/home/dominikh/prj/go/src/math/bits/bits.go:37)	CMOVQEQ	BX, DX
	0x0017 00023 (/home/dominikh/prj/src/example.com/foo.go:12)	LEAQ	1(DX), BX
	0x001b 00027 (/home/dominikh/prj/src/example.com/foo.go:11)	TESTQ	AX, CX
	0x001e 00030 (/home/dominikh/prj/src/example.com/foo.go:14)	CMOVQNE	BX, DX
	0x0022 00034 (/home/dominikh/prj/src/example.com/foo.go:14)	MOVQ	DX, command-line-arguments.Sink(SB)
	0x0029 00041 (/home/dominikh/prj/src/example.com/foo.go:11)	RET

What did you expect to see?

CMOVQNE at 0x001e uses ZF set by TESTQ AX, CX. The ZF set by TESTQ at 0x0005 gets immediately clobbered by the BSRQ that follows. flagalloc inserts a second TESTQ at 0x001b, but doesn't clean up the old TESTQ, which is now dead.
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 9, 2024
@dominikh
Copy link
Member Author

dominikh commented Jan 9, 2024

Related: #24537 (comment)

@dmitshur
Copy link
Contributor

dmitshur commented Jan 9, 2024

CC @golang/compiler.

@dmitshur dmitshur added this to the Backlog milestone Jan 9, 2024
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 9, 2024
@randall77 randall77 added Performance NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 9, 2024
@mknyszek mknyszek modified the milestones: Backlog, Unplanned Jan 17, 2024
@kungfukennyg
Copy link

Has anyone claimed this or can I take a stab at it?

@randall77
Copy link
Contributor

@kungfukennyg go for it.

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. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@dominikh @mknyszek @dmitshur @kungfukennyg @randall77 @gopherbot