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: memcombine should learn alignment of byte slices - hard mode - arithmetic #71780

Open
Jorropo opened this issue Feb 16, 2025 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Implementation Issues describing a semantics-preserving change to the Go implementation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@Jorropo
Copy link
Member

Jorropo commented Feb 16, 2025

Go version

go version devel go1.25-b38415d7e9 Sat Feb 15 21:47:27 2025 -0800 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='0'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='riscv64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/tmp/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/hugo/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3509629757=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/home/hugo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/hugo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GORISCV64='rva20u64'
GOROOT='/home/hugo/k/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/hugo/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/home/hugo/k/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.25-b38415d7e9 Sat Feb 15 21:47:27 2025 -0800'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Compile:

package a

import "encoding/binary"
import "unsafe"

func xorByte(s []byte, xor byte) {
        for ; len(s) != 0 && uintptr(unsafe.Pointer(unsafe.SliceData(s)))%8 != 0; s = s[1:] {
                s[0] ^= xor
        }
        x := uint64(xor)
        packed := x | x<<8 | x<<16 | x<<24 | x<<32 | x<<40 | x<<48 | x<<56
        for ; len(s) >= 8; s = s[8:] {
                binary.NativeEndian.PutUint64(s, binary.NativeEndian.Uint64(s)^packed)
        }
        for ; len(s) != 0; s = s[1:] {
                s[0] ^= xor
        }
}

What did you see happen?

The two calls to binary.NativeEndian.* compiles a series of 8 bits loads, shifts and bitwise OR & AND.

What did you expect to see?

64 bits load and store is used.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 16, 2025
@Jorropo
Copy link
Member Author

Jorropo commented Feb 16, 2025

This is similar to #71778.
I've never wrote go code with the expectation this would be optimized altho I've done many times in C. This will probably change when we get SIMD intrinsics.

I guess I could extend prove to learn this kinds of things.

@Jorropo
Copy link
Member Author

Jorropo commented Feb 16, 2025

My example is particularly hard because when exiting the first loop the alignment is in fact not known at compile time but this will only happen if length is zero then the 8 bytes loop body wont execute.
If we can optimize this it would be enough to work with given some code massaging:

package a

import "encoding/binary"
import "unsafe"

func xorByte(s []byte, xor byte) {
        if len(s) == 0 { return }
        for ; uintptr(unsafe.Pointer(unsafe.SliceData(s)))%8 != 0; s = s[1:] {
               if len(s) == 0 { return }
                s[0] ^= xor
        }
        x := uint64(xor)
        packed := x | x<<8 | x<<16 | x<<24 | x<<32 | x<<40 | x<<48 | x<<56
        for ; len(s) >= 8; s = s[8:] {
                binary.NativeEndian.PutUint64(s, binary.NativeEndian.Uint64(s)^packed)
        }
        for ; len(s) != 0; s = s[1:] {
                s[0] ^= xor
        }
}

@gabyhelp gabyhelp added the Implementation Issues describing a semantics-preserving change to the Go implementation. label Feb 16, 2025
@mknyszek mknyszek changed the title cmd/compile: memcombine should learn allignement of byte slices - hard mode - arithmetic cmd/compile: memcombine should learn alignement of byte slices - hard mode - arithmetic Feb 18, 2025
@mknyszek mknyszek changed the title cmd/compile: memcombine should learn alignement of byte slices - hard mode - arithmetic cmd/compile: memcombine should learn alignment of byte slices - hard mode - arithmetic Feb 18, 2025
@mknyszek mknyszek added this to the Unplanned milestone Feb 18, 2025
@mknyszek mknyszek added Performance NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 18, 2025
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. Implementation Issues describing a semantics-preserving change to the Go implementation. 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