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: unsafe conversion from slice to struct pointer generates worse code on amd64 than on 386 #65330

Open
dominikh opened this issue Jan 27, 2024 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@dominikh
Copy link
Member

dominikh commented Jan 27, 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-build1255434568=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Compile

package pkg

import "unsafe"

type T struct {
	A, B int
}

func Foo(x []byte) {
	Sink = (*T)(*(*unsafe.Pointer)(unsafe.Pointer(&x))).A
}

var Sink int

What did you see happen?

When compiling for GOARCH=386, the function body compiles to

0x0012 00018 (/home/dominikh/prj/src/example.com/foo.go:10)     MOVL    command-line-arguments.x+4(SP), AX
0x0016 00022 (/home/dominikh/prj/src/example.com/foo.go:10)     MOVL    (AX), AX
0x0018 00024 (/home/dominikh/prj/src/example.com/foo.go:10)     MOVL    AX, command-line-arguments.Sink(SB)

but when compiling for GOARCH=amd64, it compiles to

0x0000 00000 (/home/dominikh/prj/src/example.com/foo.go:9)      MOVQ    AX, command-line-arguments.x+8(SP)
0x0005 00005 (/home/dominikh/prj/src/example.com/foo.go:9)      MOVQ    BX, command-line-arguments.x+16(SP)
0x000a 00010 (/home/dominikh/prj/src/example.com/foo.go:9)      MOVQ    CX, command-line-arguments.x+24(SP)
0x000f 00015 (/home/dominikh/prj/src/example.com/foo.go:10)     MOVQ    command-line-arguments.x+8(SP), AX
0x0014 00020 (/home/dominikh/prj/src/example.com/foo.go:10)     MOVQ    (AX), AX
0x0017 00023 (/home/dominikh/prj/src/example.com/foo.go:10)     MOVQ    AX, command-line-arguments.Sink(SB)

Where the first 4 instructions are unnecessary.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 27, 2024
@dominikh
Copy link
Member Author

Possibly related/the same: #65192

@randall77
Copy link
Contributor

This is a consequence of regabi vs. the old calling convention.
for 386, the slice argument is already on the stack so it just needs to load the parts it needs from the stack.
For amd64, the slice argument is in registers, so it first needs to be spilled to the stack before loading from it.

Taking the address of the slice forces it to be put in memory somewhere, as the compiler doesn't have the analysis capable of figuring out which parts of the address-taken variable are actually read.
Maybe there is something simple we can do for this case.

Using unsafe.SliceData is probably the clearer way to do this anyway, and as a bonus generates the efficient code on all architectures.

@dominikh
Copy link
Member Author

dominikh commented Jan 27, 2024

as the compiler doesn't have the analysis capable of figuring out which parts of the address-taken variable are actually read.

Indeed, the problem reproduces with this simpler (but more synthetic) example:

func Foo(x []byte) {
	a := &x
	Sink = len(*a)
}

Using SliceData works here but doesn't address the same kind of issue that arises when unsafely casting between two arbitrary types and accessing a subset of fields.

@mknyszek mknyszek added this to the Unplanned milestone Jan 29, 2024
@mknyszek mknyszek added Performance NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest labels Jan 29, 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. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Development

No branches or pull requests

4 participants