cmd/compile: missed BCE in multi expression switch cases #67329

janishorsts · 2024-05-12T10:45:52Z

Go version

go version go1.22.3 darwin/arm64

Output of `go env` in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/jhorsts/Library/Caches/go-build'
GOENV='/Users/jhorsts/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/jhorsts/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/jhorsts/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.3/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.3/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/jhorsts/projects/playground/x/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g -I/Users/jhorsts/projects/others/freetds-1.3.11/include -I/opt/homebrew/Cellar/unixodbc/2.3.12/include'
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 -ffile-prefix-map=/var/folders/v0/8cfqwhrd5zdg6q7vz501vgl80000gn/T/go-build2944401573=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

In Found, the bounds check is not eliminated in the switch having multiple values per case. In NotFound, it is.

package x

func Found(x []string) string {
	switch len(x) {
	default:
		return x[0] // ./x.go:6:11: Found IsInBounds
	case 0, 1:
		return ""
	}
}

func NotFound(x []string) string {
	switch len(x) {
	default:
		return x[0]
	case 0:
		return ""
	case 1:
		return ""
	}
}

What did you see happen?

➜ go build -gcflags="-d=ssa/check_bce/debug=1" .
./x.go:6:11: Found IsInBounds
➜

What did you expect to see?

➜  go build -gcflags="-d=ssa/check_bce/debug=1" .
➜

No bounds check.

The text was updated successfully, but these errors were encountered:

go101 · 2024-05-13T18:03:53Z

Such cases might be too vast to be handled by the compiler. Consider that the constant index may be any positive integer ...
So my suggestion is to write code in BCE friendly way.

[edit] But here, as the second way has been handled whereas the first one has not, maybe it is worth making them consistent.

janishorsts · 2024-05-13T19:48:08Z

@go101, I noticed the missed BCE in a much more complex switch statement - I provided minimal code to demonstrate it. In the end, I got rid of the switch statement after refactoring.

GitHub reveals it is more widespread than I anticipated (not all found files contain missed BCE) - https://github.com/search?q=%22case+0%2C+1%22+AND+%22len%28%22+AND+language%3AGo+&type=code

randall77 · 2024-05-15T21:18:26Z

In fact there is no bounds check in the generated code. -d=ssa/check_bce/debug=1 is lying. Or at least, it isn't correctly reporting the removal of the bounds check after the check_bce detection pass.

So it might help if someone wants to look into how we might fix that reporting issue.

The issue itself reminds me of #66826, which I've already started on a strategy to fix. This reporting issue may resolve itself after that issue is fixed, as I think it will push the bounds check removal earlier.

janishorsts · 2024-05-16T08:54:40Z

Indeed. go tool compile -S x.go reveals no bound check. It slipped my mind to check the compiled output.

<unlinkable>.Found STEXT size=48 args=0x18 locals=0x0 funcid=0x0 align=0x0 leaf
	0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:3)	TEXT	<unlinkable>.Found(SB), LEAF|NOFRAME|ABIInternal, $0-24
	0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:3)	MOVD	R0, <unlinkable>.x(FP)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3)	FUNCDATA	$0, gclocals·wgcWObbY2HYnK2SU/U22lA==(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3)	FUNCDATA	$1, gclocals·J5F+7Qw7O7ve2QcWC7DpeQ==(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3)	FUNCDATA	$5, <unlinkable>.Found.arginfo1(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3)	FUNCDATA	$6, <unlinkable>.Found.argliveinfo(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3)	PCDATA	$3, $1
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:7)	CMP	$1, R1
	0x0008 00008 (/Users/jhorsts/projects/playground/bce-switch/x.go:7)	BGT	24
	0x000c 00012 (/Users/jhorsts/projects/playground/bce-switch/x.go:8)	MOVD	ZR, R0
	0x0010 00016 (/Users/jhorsts/projects/playground/bce-switch/x.go:8)	MOVD	ZR, R1
	0x0014 00020 (/Users/jhorsts/projects/playground/bce-switch/x.go:8)	RET	(R30)
	0x0018 00024 (/Users/jhorsts/projects/playground/bce-switch/x.go:6)	MOVD	(R0), R2
	0x001c 00028 (/Users/jhorsts/projects/playground/bce-switch/x.go:6)	MOVD	8(R0), R1
	0x0020 00032 (/Users/jhorsts/projects/playground/bce-switch/x.go:6)	MOVD	R2, R0
	0x0024 00036 (/Users/jhorsts/projects/playground/bce-switch/x.go:6)	RET	(R30)
	0x0000 e0 07 00 f9 3f 04 00 f1 8c 00 00 54 e0 03 1f aa  ....?......T....
	0x0010 e1 03 1f aa c0 03 5f d6 02 00 40 f9 01 04 40 f9  ......_...@...@.
	0x0020 e0 03 02 aa c0 03 5f d6 00 00 00 00 00 00 00 00  ......_.........
<unlinkable>.NotFound STEXT size=64 args=0x18 locals=0x0 funcid=0x0 align=0x0 leaf
	0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:12)	TEXT	<unlinkable>.NotFound(SB), LEAF|NOFRAME|ABIInternal, $0-24
	0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:12)	MOVD	R0, <unlinkable>.x(FP)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12)	FUNCDATA	$0, gclocals·wgcWObbY2HYnK2SU/U22lA==(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12)	FUNCDATA	$1, gclocals·J5F+7Qw7O7ve2QcWC7DpeQ==(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12)	FUNCDATA	$5, <unlinkable>.NotFound.arginfo1(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12)	FUNCDATA	$6, <unlinkable>.NotFound.argliveinfo(SB)
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12)	PCDATA	$3, $1
	0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:16)	CBZ	R1, 44
	0x0008 00008 (/Users/jhorsts/projects/playground/bce-switch/x.go:18)	CMP	$1, R1
	0x000c 00012 (/Users/jhorsts/projects/playground/bce-switch/x.go:18)	BNE	28
	0x0010 00016 (/Users/jhorsts/projects/playground/bce-switch/x.go:19)	MOVD	ZR, R0
	0x0014 00020 (/Users/jhorsts/projects/playground/bce-switch/x.go:19)	MOVD	ZR, R1
	0x0018 00024 (/Users/jhorsts/projects/playground/bce-switch/x.go:19)	RET	(R30)
	0x001c 00028 (/Users/jhorsts/projects/playground/bce-switch/x.go:15)	MOVD	(R0), R2
	0x0020 00032 (/Users/jhorsts/projects/playground/bce-switch/x.go:15)	MOVD	8(R0), R1
	0x0024 00036 (/Users/jhorsts/projects/playground/bce-switch/x.go:15)	MOVD	R2, R0
	0x0028 00040 (/Users/jhorsts/projects/playground/bce-switch/x.go:15)	RET	(R30)
	0x002c 00044 (/Users/jhorsts/projects/playground/bce-switch/x.go:17)	MOVD	ZR, R0
	0x0030 00048 (/Users/jhorsts/projects/playground/bce-switch/x.go:17)	MOVD	ZR, R1
	0x0034 00052 (/Users/jhorsts/projects/playground/bce-switch/x.go:17)	RET	(R30)
	0x0000 e0 07 00 f9 41 01 00 b4 3f 04 00 f1 81 00 00 54  ....A...?......T
	0x0010 e0 03 1f aa e1 03 1f aa c0 03 5f d6 02 00 40 f9  .........._...@.
	0x0020 01 04 40 f9 e0 03 02 aa c0 03 5f d6 e0 03 1f aa  ..@......._.....
	0x0030 e1 03 1f aa c0 03 5f d6 00 00 00 00 00 00 00 00  ......_.........
go:cuinfo.producer.<unlinkable> SDWARFCUINFO dupok size=0
	0x0000 72 65 67 61 62 69                                regabi
go:cuinfo.packagename.<unlinkable> SDWARFCUINFO dupok size=0
	0x0000 78                                               x
gclocals·wgcWObbY2HYnK2SU/U22lA== SRODATA dupok size=10
	0x0000 02 00 00 00 01 00 00 00 01 00                    ..........
gclocals·J5F+7Qw7O7ve2QcWC7DpeQ== SRODATA dupok size=8
	0x0000 02 00 00 00 00 00 00 00                          ........
<unlinkable>.Found.arginfo1 SRODATA static dupok size=9
	0x0000 fe 00 08 08 08 10 08 fd ff                       .........
<unlinkable>.Found.argliveinfo SRODATA static dupok size=2
	0x0000 00 00                                            ..
<unlinkable>.NotFound.arginfo1 SRODATA static dupok size=9
	0x0000 fe 00 08 08 08 10 08 fd ff                       .........
<unlinkable>.NotFound.argliveinfo SRODATA static dupok size=2
	0x0000 00 00                                            ..

seankhliao changed the title ~~missed BCE in switch (multiple cases)~~ cmd/compile: missed BCE in multi expression switch cases May 12, 2024

gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 12, 2024

dmitshur added this to the Backlog milestone May 13, 2024

dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 13, 2024

randall77 added the Performance label May 15, 2024

randall77 added help wanted and removed Performance labels May 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/compile: missed BCE in multi expression switch cases #67329

cmd/compile: missed BCE in multi expression switch cases #67329

janishorsts commented May 12, 2024

go101 commented May 13, 2024 •

edited

janishorsts commented May 13, 2024

randall77 commented May 15, 2024

janishorsts commented May 16, 2024

cmd/compile: missed BCE in multi expression switch cases #67329

cmd/compile: missed BCE in multi expression switch cases #67329

Comments

janishorsts commented May 12, 2024

Go version

Output of go env in your module/workspace:

What did you do?

What did you see happen?

What did you expect to see?

go101 commented May 13, 2024 • edited

janishorsts commented May 13, 2024

randall77 commented May 15, 2024

janishorsts commented May 16, 2024

Output of `go env` in your module/workspace:

go101 commented May 13, 2024 •

edited