search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.02k stars 17.54k forks source link

proposal: cmd/compile: allow bound check skip via directive #66318

Closed lollllcat closed 6 months ago

lollllcat commented 6 months ago

Proposal Details

We propose adding a new directive to skip bound checks in the compile toolchain. The new directive will allow the unnecessary bound check to be eliminated if the flag AllowBCSkip is turned on. The current bound check elimination passes (BCE and CSE) are relatively simple and leave many redundant bound checks unremoved. Such unnecessary checks bring extra performance overhead. Improving compiler optimization pass is another solution but it can be time consuming and requires sophisticated analysis. This proposal aims for a straightforward optimization if users know the bound check is always unnecessary but unremoved.

Specifically, -allowbcskip flag from the command line during compilation will enable this new feature. //go:noboundcheck directive at the function level will allow the bound check to be skipped for the specific function if the check is always redundant. It works similarly as go:noinline directive. boundsCheck within ssagen will check the new flag and directive to optionally skipping bound check code generation.

Grit link: https://go-review.googlesource.com/c/go/+/571539. Filing a new issue to make it clearer what we are proposing. Thanks.

cc @jinlin-bayarea

seankhliao commented 6 months ago

Duplicate of #47529

randall77 commented 6 months ago

I think this is just proposing the already-existing -gcflags=-B flag?

lollllcat commented 6 months ago

Hi @randall77 , it's not. The directive is per function level. -B disable bound check for all functions per https://www.google.com/url?q=https://github.com/golang/go/blob/4b0e086348c99eaa458f18e4b3e4498f0aaadc1a/src/cmd/compile/internal/ssagen/ssa.go%23L5758&sa=D&source=docs&ust=1710449767788367&usg=AOvVaw1Gk6tn_ixNeGJvqAC8z_8J

randall77 commented 6 months ago

So -allowbcskip just changes a //go:noboundcheck directive from a no-op to its intended semantics? Not sure why that additional step is needed. When would you have //go:noboundcheck directives but then not use -allowbcskip? As @seankhliao says, the individual directive has already been proposed and declined.