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:

zigo101 · 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                                            ..

cuonglm · 2024-07-16T13:57:45Z

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

For this issue, could we just move the check bce pass after late use, then just skipping invalid block when running checkbce?

randall77 · 2024-07-16T15:31:36Z

Yes, pushing checkbce later may help. I think it won't help -d=ssa/prove/debug=1 output though, so that may not be a full fix. (But maybe a full fix isn't needed.)

gopherbot · 2024-07-16T17:04:23Z

Change https://go.dev/cl/598635 mentions this issue: cmd/compile: run checkbce after fuseLate pass

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

dmitshur added this to Go Compiler / Runtime May 15, 2024

randall77 added the Performance label May 15, 2024

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

randall77 moved this to Todo in Go Compiler / Runtime May 15, 2024

dmitshur 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 Jul 23, 2024

dmitshur moved this from Todo to All-But-Submitted in Go Compiler / Runtime Jul 23, 2024

dmitshur modified the milestones: Backlog, Go1.24 Jul 23, 2024

gopherbot closed this as completed in 864aa86 Jul 23, 2024

github-project-automation bot moved this from All-But-Submitted to Done in Go Compiler / Runtime Jul 23, 2024

gabyhelp mentioned this issue Mar 6, 2025

cmd/compile: missing bounds check elimination for complex conditionals #72132

Open

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

zigo101 commented May 13, 2024 •

edited

Loading

janishorsts commented May 13, 2024

randall77 commented May 15, 2024

janishorsts commented May 16, 2024

cuonglm commented Jul 16, 2024

randall77 commented Jul 16, 2024

gopherbot commented Jul 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?

zigo101 commented May 13, 2024 • edited Loading

janishorsts commented May 13, 2024

randall77 commented May 15, 2024

janishorsts commented May 16, 2024

cuonglm commented Jul 16, 2024

randall77 commented Jul 16, 2024

gopherbot commented Jul 16, 2024

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

zigo101 commented May 13, 2024 •

edited

Loading