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: add some code motion optimizations #56620

Open
erifan opened this issue Nov 7, 2022 · 3 comments
Open

cmd/compile: add some code motion optimizations #56620

erifan opened this issue Nov 7, 2022 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@erifan
Copy link
Contributor

erifan commented Nov 7, 2022

What version of Go are you using (go version)?

$ go version
go version devel go1.20-3b6b39c869 Mon Nov 7 09:02:20 2022 +0800 darwin/arm64

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
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/erifan01/Library/Caches/go-build"
GOENV="/Users/erifan01/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/usr/local/gopath/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/usr/local/gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/erifan01/go-master"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/erifan01/go-master/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="devel go1.20-3b6b39c869 Mon Nov 7 09:02:20 2022 +0800"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/erifan01/go-master/src/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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vm/32796pv57pg0hxrpyyvkww040000gt/T/go-build2092224947=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run the following benchmark:
$ go test -run ^$ -count 5 -bench BenchmarkEfaceEq -cpuprofile cpu.out

type TS uint16

var (
        e  any
        ts TS
)

func BenchmarkEfaceEq(b *testing.B) {
        for n := 0; n < b.N; n++ {
                _ = e != ts
        }
}

The core code is the following lines:

                _ = e != ts
  0x1000efd94           f0000abb                ADRP 1404928(PC), R27
  0x1000efd98           794a2f63                MOVHU 1302(R27), R3
  0x1000efd9c           79005fe3                MOVH R3, 46(RSP)
  0x1000efda0           d000091b                ADRP 1187840(PC), R27
  0x1000efda4           f946eb63                MOVD 3536(R27), R3
  0x1000efda8           d000091b                ADRP 1187840(PC), R27      // hotspot but useless instruction for the test
  0x1000efdac           f946ef64                MOVD 3544(R27), R4         // hotspot but useless instruction for the test
  0x1000efdb0           b00001e5                ADRP 249856(PC), R5
  0x1000efdb4           913600a5                ADD $3456, R5, R5
  0x1000efdb8           eb05007f                CMP R5, R3
  0x1000efdbc           54fffe41                BNE -14(PC)

However, the hot-spot instructions for this test are two useless instructions for the test because the test does not go to that code path ( a call to runtime.efaceeq). If we can move these instructions out of the execution path of this test and into another branch, I believe the performance of this test can be greatly improved.

This is just a demo case, I believe there are more places where code motion/sinking optimization is needed. But currently there is no such optimization in Go?

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 7, 2022
@mknyszek mknyszek added this to the Backlog milestone Nov 7, 2022
@mknyszek
Copy link
Contributor

mknyszek commented Nov 7, 2022

CC @golang/compiler

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 7, 2022
@randall77
Copy link
Contributor

randall77 commented Nov 7, 2022

This looks like a restriction of the tighten pass, where it can't handle moving ops that use memory (a load, in this case).
See cmd/compile/internal/ssa/tighten.go:29.
That restriction could be lifted in some cases, like this one. We'd just need to compute memory states at the start of each block first.

@randall77 randall77 modified the milestones: Backlog, Unplanned Nov 7, 2022
@randall77 randall77 added 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 Nov 7, 2022
@erifan
Copy link
Contributor Author

erifan commented Nov 8, 2022

Indeed, I didn't see that we already have some code motion optimizations. Thanks @randall77

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 NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

4 participants