diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ba31cc9..efe0baa 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -61,8 +61,13 @@ jobs: - name: Test tsvc (amd64, SIMD) run: make tsvc-test - - name: Install qemu-user-static - run: sudo apt-get update && sudo apt-get install -y qemu-user-static + # ubuntu-26.04 has no qemu-user-static: qemu-user ships the static + # emulators there, without the -static names the Makefile runs. + - name: Install qemu-user + run: | + sudo apt-get update && sudo apt-get install -y qemu-user + command -v qemu-aarch64-static || sudo ln -s "$(command -v qemu-aarch64)" /usr/local/bin/qemu-aarch64-static + qemu-aarch64-static --version - name: Test tsvc (arm64, qemu) run: make tsvc-test-qemu-arm64 diff --git a/BUGS.md b/BUGS.md index 843087e..55231c5 100644 --- a/BUGS.md +++ b/BUGS.md @@ -2,11 +2,11 @@ loopvec rewrites code to use `GOEXPERIMENT=simd`, which is new, so most of what it hits is in the Go toolchain. This file lists the Go issues that affect loopvec. "tip" means -gotip, "1.27.1" is the release toolchain loopvec is tested against. +gotip, "1.27.3" is the release toolchain loopvec is tested against. | Issue | Status | Affects | loopvec's answer | |---|---|---|---| -| [#80657](https://github.com/golang/go/issues/80657) simd in a method: internal compiler error | fixed on tip (CL 839405), still crashes on 1.27.1 | `-methods` | methods are skipped unless `-methods` is passed | +| [#80657](https://github.com/golang/go/issues/80657) simd in a method: internal compiler error | fixed on tip (CL 839405), still crashes on 1.27.3 | `-methods` | methods are skipped unless `-methods` is passed | | [#80835](https://github.com/golang/go/issues/80835) legacy SSE encodings in functions using simd cause AVX-SSE transition penalties | open, fix pending (CLs 825145, 825146, in tip) | speed of any scalar float code that runs after a rewritten loop, in the same function or another | none; the portable `simd` package has no `ClearAVXUpperBits` | | [#81844](https://github.com/golang/go/issues/81844) blank identifier parameters rejected | fixed on tip (CL 841145) | every rewrite of a function with a `_` parameter | the generated file renames them `_p0`, `_p1`, ... | | [#81845](https://github.com/golang/go/issues/81845) calls left unresolved when inlined into a package initializer | closed as a duplicate of [#80689](https://github.com/golang/go/issues/80689), which is open | loops in package-level `var x = func() {...}()` | not rewritten (the generated initializer fails to link), see `package_initializer_no_rewrite.txt` and `toolexec_package_initializer.txt` | @@ -19,7 +19,7 @@ gotip, "1.27.1" is the release toolchain loopvec is tested against. A function with a receiver that contains simd code failed to compile with `internal compiler error: missing Types entry`. loopvec does not rewrite loops in methods -by default; `-methods` turns it on. The fix is on tip. On 1.27.1 the crash is still there, +by default; `-methods` turns it on. The fix is on tip. On 1.27.3 the crash is still there, and `flag_methods_ice.txt` pins it, so that test starts failing when the pinned toolchain gets the fix. Reported by someone else; the fix is [CL 839405](https://go.dev/cl/839405). diff --git a/Makefile b/Makefile index b4226b3..3804653 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -export GOTOOLCHAIN ?= go1.27.1 +export GOTOOLCHAIN ?= go1.27.3 .DEFAULT_GOAL := build diff --git a/cmd/loopvec-toolexec/compile.go b/cmd/loopvec-toolexec/compile.go index 13f0d91..999160f 100644 --- a/cmd/loopvec-toolexec/compile.go +++ b/cmd/loopvec-toolexec/compile.go @@ -321,6 +321,7 @@ func check(fset *token.FileSet, files []*ast.File, c compileArgs, cfg *importcfg Defs: make(map[*ast.Ident]types.Object), Uses: make(map[*ast.Ident]types.Object), Selections: make(map[*ast.SelectorExpr]*types.Selection), + Scopes: make(map[ast.Node]*types.Scope), } var errs []error goarch := os.Getenv("GOARCH") diff --git a/go.mod b/go.mod index ed04302..9457290 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/mauri870/loopvec go 1.27.1 require ( - golang.org/x/tools v0.51.0 + golang.org/x/tools v0.52.1-0.20261030190145-0f433775b36d rsc.io/script v0.0.2 ) @@ -207,10 +207,10 @@ require ( go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/exp/typeparams v0.0.0-20260811152304-ee035b5b010f // indirect - golang.org/x/mod v0.41.0 // indirect - golang.org/x/sync v0.23.0 // indirect - golang.org/x/sys v0.48.0 // indirect - golang.org/x/telemetry v0.0.0-20260924152758-ed294f943157 // indirect + golang.org/x/mod v0.42.0 // indirect + golang.org/x/sync v0.24.0 // indirect + golang.org/x/sys v0.49.0 // indirect + golang.org/x/telemetry v0.0.0-20261009151841-97e8ea77aa2a // indirect golang.org/x/text v0.41.0 // indirect google.golang.org/protobuf v1.36.12 // indirect gopkg.in/ini.v1 v1.67.0 // indirect diff --git a/go.sum b/go.sum index 04c2946..bb2b241 100644 --- a/go.sum +++ b/go.sum @@ -688,8 +688,8 @@ golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c= -golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o= +golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= +golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -724,8 +724,8 @@ golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.59.0 h1:5zfYln+w5XCxwrnMMJPufRgNoXEaGxl0wo5GqPXyues= -golang.org/x/net v0.59.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= +golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw= +golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -745,8 +745,8 @@ golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= -golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= +golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA= +golang.org/x/sync v0.24.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -790,10 +790,10 @@ golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= -golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= -golang.org/x/telemetry v0.0.0-20260924152758-ed294f943157 h1:DftxASHKl6JolFPqoN+2WrFO53JvkHwRXvFqf8gsivA= -golang.org/x/telemetry v0.0.0-20260924152758-ed294f943157/go.mod h1:i+ivNqjDnTF3WTElsdk5g9V5DTSBYgdNo7xTU9SDwYA= +golang.org/x/sys v0.49.0 h1:XbzkgYJHdqh/8m2Uu0W/dQv8nktxx4BFHp1M0gROTXA= +golang.org/x/sys v0.49.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= +golang.org/x/telemetry v0.0.0-20261009151841-97e8ea77aa2a h1:wnveMBKHwx8/nIb8WYT5P9vtefdXOvIWxzycCkc5n6Q= +golang.org/x/telemetry v0.0.0-20261009151841-97e8ea77aa2a/go.mod h1:i+ivNqjDnTF3WTElsdk5g9V5DTSBYgdNo7xTU9SDwYA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= @@ -857,8 +857,8 @@ golang.org/x/tools v0.1.1-0.20210302220138-2ac05c832e1a/go.mod h1:9bzcO0MWcOuT0t golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.51.0 h1:k4Xc/1Om9jwkBJBo4NVLMSARBoWtK10mx+W5BnXCeAI= -golang.org/x/tools v0.51.0/go.mod h1:9eEncMayCV6zRMGhR5eZEC2iBx98qWcF1HZ9Z7wJOoA= +golang.org/x/tools v0.52.1-0.20261030190145-0f433775b36d h1:F5ot9G6RM6pgx3TTdKF6IZa2nocit4tcsrVQlq4Zh5w= +golang.org/x/tools v0.52.1-0.20261030190145-0f433775b36d/go.mod h1:ItcP5UNaVGG+Yc4eIYmaFLNJ4soEopkx5Y2d4lcEurs= golang.org/x/tools/go/expect v0.1.1-deprecated h1:jpBZDwmgPhXsKZC6WhL20P4b/wmnpsEAGHaNy0n/rJM= golang.org/x/tools/go/expect v0.1.1-deprecated/go.mod h1:eihoPOH+FgIqa3FpoTwguz/bVUSGBlGQU67vpBeOrBY= golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated h1:1h2MnaIAIXISqTFKdENegdpAgUXz6NrPEsbIeWaBRvM= diff --git a/internal/analysis/analyzer.go b/internal/analysis/analyzer.go index d686b59..cacc065 100644 --- a/internal/analysis/analyzer.go +++ b/internal/analysis/analyzer.go @@ -42,9 +42,10 @@ func Analyze(file *ast.File, info *types.Info, opts Options) []*loopir.Plan { continue } results := loopir.NamedResults(fn, info) + reserved := loopir.UsesReservedNames(fn) ast.Inspect(fn.Body, func(n ast.Node) bool { if stmt, ok := n.(ast.Stmt); ok { - if plan, _ := analyzeLoop(stmt, info, results, opts.Plan()); plan != nil { + if plan, _ := analyzeLoop(stmt, info, info.Scopes[file], results, reserved, opts.Plan()); plan != nil { plans = append(plans, plan) } } @@ -55,8 +56,10 @@ func Analyze(file *ast.File, info *types.Info, opts Options) []*loopir.Plan { } // analyzeLoop lowers stmt, which must be a for or range statement, and plans -// it. It returns the reason when the loop is not vectorizable. -func analyzeLoop(stmt ast.Stmt, info *types.Info, results map[types.Object]bool, plan loopir.Options) (*loopir.Plan, loopir.Reason) { +// it. It returns the reason when the loop is not vectorizable. reserved is set +// when the enclosing function uses a name that begins with an underscore (see +// loopir.UsesReservedNames); file is the scope of the file stmt is in. +func analyzeLoop(stmt ast.Stmt, info *types.Info, file *types.Scope, results map[types.Object]bool, reserved bool, plan loopir.Options) (*loopir.Plan, loopir.Reason) { switch stmt.(type) { case *ast.RangeStmt, *ast.ForStmt: default: @@ -66,5 +69,21 @@ func analyzeLoop(stmt ast.Stmt, info *types.Info, results map[types.Object]bool, if reason != "" { return nil, reason } - return loopir.NewPlan(loop, plan) + p, reason := loopir.NewPlan(loop, plan) + if p == nil { + return nil, reason + } + // The names are checked last, so that a loop that cannot be vectorized + // anyway keeps the reason that matters. + if reserved { + return nil, loopir.ReasonReservedName + } + var scope *types.Scope + if file != nil { + scope = file.Innermost(stmt.Pos()) + } + if loopir.ShadowsEmittedName(scope, stmt.Pos()) || loopir.HeaderShadowsName(stmt) { + return nil, loopir.ReasonShadowedName + } + return p, reason } diff --git a/internal/analysis/explain.go b/internal/analysis/explain.go index 10a47ec..c0f4017 100644 --- a/internal/analysis/explain.go +++ b/internal/analysis/explain.go @@ -32,6 +32,7 @@ func AnalyzeExplain(fset *token.FileSet, file *ast.File, info *types.Info, opts } isMethod := fn.Recv != nil results := loopir.NamedResults(fn, info) + reserved := loopir.UsesReservedNames(fn) ast.Inspect(fn.Body, func(n ast.Node) bool { var body *ast.BlockStmt switch stmt := n.(type) { @@ -54,7 +55,7 @@ func AnalyzeExplain(fset *token.FileSet, file *ast.File, info *types.Info, opts return true } - plan, reason := analyzeLoop(n.(ast.Stmt), info, results, opts.Plan()) + plan, reason := analyzeLoop(n.(ast.Stmt), info, info.Scopes[file], results, reserved, opts.Plan()) c.Vectorized = plan != nil c.Reason = reason out = append(out, c) diff --git a/internal/loopir/caps_gen.go b/internal/loopir/caps_gen.go index bccacfa..ba7ddff 100644 --- a/internal/loopir/caps_gen.go +++ b/internal/loopir/caps_gen.go @@ -1,4 +1,4 @@ -// Code generated by capsgen from the simd package of go1.27.1. DO NOT EDIT. +// Code generated by capsgen from the simd package of go1.27.3. DO NOT EDIT. package loopir diff --git a/internal/loopir/lower.go b/internal/loopir/lower.go index c4a7719..f9df98a 100644 --- a/internal/loopir/lower.go +++ b/internal/loopir/lower.go @@ -7,6 +7,7 @@ import ( "go/token" "go/types" "math" + "slices" "strconv" "strings" ) @@ -286,6 +287,12 @@ func (l *lowerer) bound(e ast.Expr) (bound, bool) { } b := bound{text: types.ExprString(e)} b.value, b.isConst = l.constant(e) + if b.isConst { + // A named constant or a constant expression (s0, 1 - 1) is spelled as its + // value, so the checks that read a constant start or limit see it and the + // emitted code never indexes at a negative constant they would have caught. + b.text = strconv.FormatInt(b.value, 10) + } if call, ok := ast.Unparen(e).(*ast.CallExpr); ok { b.slice, _ = l.lenOf(call) } @@ -370,6 +377,121 @@ func isIntType(t types.Type) bool { return ok && (basic.Kind() == types.Int || basic.Kind() == types.UntypedInt) } +// UsesReservedNames reports whether fn mentions an identifier that begins with +// an underscore, other than the blank identifier: every name loopvec adds does, +// so code it inserts into fn could capture one. +func UsesReservedNames(fn *ast.FuncDecl) bool { + found := false + ast.Inspect(fn, func(n ast.Node) bool { + if id, ok := n.(*ast.Ident); ok && len(id.Name) > 1 && id.Name[0] == '_' { + found = true + } + return !found + }) + return found +} + +// emittedNames are the predeclared and package names the rewritten code uses: +// len, min, max and copy, uint64 for shift counts, the element types of +// reductions (byte and rune among them, as go/types spells those), and the +// simd and unsafe packages. +var emittedNames = []string{ + "len", "min", "max", "copy", "uint64", + "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "float32", "float64", + "byte", "rune", + "simd", "unsafe", +} + +// HeaderShadowsName reports whether a variable stmt, a for or range statement, +// declares in its header could capture a name of the rewritten code. The +// scalar loop that finishes a reduction declares the same variables as stmt and +// evaluates the bound inside their scope, while stmt evaluates its range +// operand, or the start it counts down from, outside: an index named len would +// break len(a), and for n := range n or for n := n - 1; n >= 0; n-- would +// compare the index with itself. So stmt must not declare one of emittedNames, +// nor a name used by its range operand or its initial values. +func HeaderShadowsName(stmt ast.Stmt) bool { + var vars, outside []ast.Expr + switch s := stmt.(type) { + case *ast.RangeStmt: + if s.Tok == token.DEFINE { + vars = []ast.Expr{s.Key, s.Value} + } + outside = []ast.Expr{s.X} + case *ast.ForStmt: + if init, ok := s.Init.(*ast.AssignStmt); ok && init.Tok == token.DEFINE { + vars, outside = init.Lhs, init.Rhs + } + } + for _, v := range vars { + id, ok := v.(*ast.Ident) + if !ok || id.Name == "_" { + continue + } + if slices.Contains(emittedNames, id.Name) { + return true + } + for _, e := range outside { + if mentions(e, id.Name) { + return true + } + } + } + return false +} + +// mentions reports whether e uses an identifier named name. +func mentions(e ast.Expr, name string) bool { + found := false + ast.Inspect(e, func(n ast.Node) bool { + if id, ok := n.(*ast.Ident); ok && id.Name == name { + found = true + } + return !found + }) + return found +} + +// helperNames are the predeclared names that _loopvecOverlap, which loopvec +// declares at package level, uses besides emittedNames. +var helperNames = []string{"any", "bool", "false", "int", "uintptr"} + +// ShadowsEmittedName reports whether, at pos in scope, a name the rewritten code +// uses means something else than the code needs: a predeclared name declared +// again in an enclosing scope, the package or an import, or simd or unsafe naming +// something other than those packages. scope is the innermost scope containing +// pos, or nil when the type information has no scopes, which counts as shadowed. +// The names only the overlap helper uses matter at package level, where it is +// declared. +func ShadowsEmittedName(scope *types.Scope, pos token.Pos) bool { + if scope == nil { + return true + } + pkg := scope + for pkg.Parent() != nil && pkg.Parent() != types.Universe { + pkg = pkg.Parent() + } + for _, name := range helperNames { + if pkg.Lookup(name) != nil { + return true + } + } + for _, name := range emittedNames { + _, obj := scope.LookupParent(name, pos) + switch name { + case "simd", "unsafe": + if pkg, ok := obj.(*types.PkgName); obj != nil && (!ok || pkg.Imported().Path() != name) { + return true + } + default: + if obj != types.Universe.Lookup(name) { + return true + } + } + } + return false +} + // NamedResults returns the named result variables of fn and of every function // literal inside it. func NamedResults(fn *ast.FuncDecl, info *types.Info) map[types.Object]bool { @@ -532,6 +654,25 @@ func (l *lowerer) body(node ast.Stmt, block *ast.BlockStmt, trip Trip, step int6 } } + // A constant offset that puts an index below zero makes the scalar loop panic + // when it runs: on its first iteration when the start is a constant and the + // first index is negative, or on every iteration when the limit is a constant + // and even the last index is. It is left alone, since the vector loop would + // index or slice at a negative constant, which does not compile. + start, errStart := strconv.ParseInt(trip.Start, 10, 64) + limit, errLimit := strconv.ParseInt(trip.Limit, 10, 64) + negative := func(offset string) bool { + off, err := strconv.ParseInt(offset, 10, 64) + return err == nil && (errStart == nil && start+off < 0 || errLimit == nil && limit+off <= 0) + } + for _, stmt := range body { + for _, leaf := range Leaves(stmtValue(stmt)) { + if load, ok := leaf.(*Load); ok && negative(load.Off) { + return nil, ReasonNegativeIndex + } + } + } + loop := &Loop{ Node: node, Ind: Induction{Var: l.iv, Step: step, Trip: trip}, diff --git a/internal/loopir/reason.go b/internal/loopir/reason.go index aaeca53..a9616d9 100644 --- a/internal/loopir/reason.go +++ b/internal/loopir/reason.go @@ -42,6 +42,12 @@ const ( // iteration writes (a[i] = a[i-1] + x): iteration i reads what iteration i-1 // wrote, so the iterations cannot run as a vector. ReasonCarriedDependence Reason = "an iteration reads an element an earlier iteration wrote" + // ReasonNegativeIndex covers a slice read at a constant offset that puts an + // index below zero: the first index, with a constant start (a[i-1] in a loop + // from 0), or every index, with a constant limit (a[i-20] in a loop that stops + // before 16). The scalar loop panics when it runs, and the vector loop would + // index or slice at a negative constant, which does not compile. + ReasonNegativeIndex Reason = "a constant offset puts an index below zero" // ReasonUnsupportedIf covers an if that is not a select: a condition that is // not a comparison of loop values (joined by && and ||), an init statement, or // a branch that is not one assignment to an element of the same slice. @@ -96,6 +102,18 @@ const ( // propagate the NaN and order the zeros. arm64 matches Go, so the result // would depend on the machine. ReasonFloatMinMax Reason = "float min and max differ from Go's on some architectures (NaN and signed zero)" + // ReasonReservedName covers a loop in a function that uses an identifier + // beginning with an underscore, other than _ itself. Every name loopvec adds + // begins with one (_i, _count, _w1, _v1, _vcFloat32s0, _sx_f, ...), and code + // it adds before or inside the loop could shadow the function's own and change + // what it refers to. + ReasonReservedName Reason = "the function uses a name that begins with an underscore, as loopvec's own names do" + // ReasonShadowedName covers a loop where a predeclared name the rewritten + // code uses (len, min, max, copy, uint64, an element type) is declared again + // in a scope around it, its package or an import, or where simd or unsafe + // names something other than those packages: the code would call or convert + // with the wrong thing. + ReasonShadowedName Reason = "a name the rewritten code relies on (len, min, simd, ...) means something else here" // ReasonMethodSkipped covers a loop inside a method, skipped unless // -methods is passed (see analysis.Analyze's doc comment). ReasonMethodSkipped Reason = "loops inside methods are skipped without -methods" @@ -113,7 +131,7 @@ const ( // and "lower" for everything about the loop's own shape. func (r Reason) Stage() string { switch r { - case ReasonMethodSkipped: + case ReasonMethodSkipped, ReasonReservedName, ReasonShadowedName: return "analysis" case ReasonUnsupportedOp, ReasonFloatMinMax, ReasonFloatReduction, ReasonMixedTypes, ReasonTooManySlices, ReasonShortLoop: return "plan" diff --git a/internal/rewrite/rewriter.go b/internal/rewrite/rewriter.go index 558e835..25fb69c 100644 --- a/internal/rewrite/rewriter.go +++ b/internal/rewrite/rewriter.go @@ -74,6 +74,10 @@ func File(fset *token.FileSet, file *ast.File, info *types.Info, src []byte, opt } needsOverlapHelper := false needsSimd := false + // rewritten holds the functions given a rewritten loop, by their index + // among the file's function declarations. Rewriting changes only loop + // bodies and appends the overlap helper, so the indexes stay valid. + rewritten := map[int]bool{} for _, plan := range plans { stmtNode := plan.Loop.Node @@ -106,6 +110,11 @@ func File(fset *token.FileSet, file *ast.File, info *types.Info, src []byte, opt end: stmtNode.End(), text: repl, }) + for i, fd := range funcDecls(file) { + if fd.Pos() <= stmtNode.Pos() && stmtNode.End() <= fd.End() { + rewritten[i] = true + } + } if pre != "" { extraStmts = append(extraStmts, struct { pos token.Pos @@ -179,7 +188,7 @@ func File(fset *token.FileSet, file *ast.File, info *types.Info, src []byte, opt // Rename blank identifier parameters to avoid a gotip GOEXPERIMENT=simd // compiler bug where _ params in simd-using functions produce // "cannot use _ as value or type". - formatted = fixBlankParams(fset, formatted) + formatted = fixBlankParams(fset, formatted, rewritten) return Result{Src: formatted, Rewrites: len(replacements), DefinesHelper: definesHelper}, nil } @@ -307,9 +316,12 @@ func ensureOverlapHelper(src []byte) []byte { // fixBlankParams renames blank identifier (_) function parameters in src to // _p0, _p1, … to work around a gotip GOEXPERIMENT=simd compiler bug: // blank params in functions that contain simd code cause the SIMD lowering -// pass to emit "cannot use _ as value or type". The rename is safe because -// the parameters are never referenced by name in the body. -func fixBlankParams(fset *token.FileSet, src []byte) []byte { +// pass to emit "cannot use _ as value or type". Only the functions loopvec +// rewrote, given by their funcDecls index in rewritten, are renamed: they +// mention no name that begins with an underscore, so the new names cannot +// capture anything, while in another function a _p0 parameter could hide a +// package-level _p0. +func fixBlankParams(fset *token.FileSet, src []byte, rewritten map[int]bool) []byte { file, err := parser.ParseFile(fset, "", src, parser.ParseComments) if err != nil { return src @@ -317,13 +329,9 @@ func fixBlankParams(fset *token.FileSet, src []byte) []byte { counter := 0 changed := false - ast.Inspect(file, func(n ast.Node) bool { - fd, ok := n.(*ast.FuncDecl) - if !ok { - return true - } - if fd.Type.Params == nil { - return true + for i, fd := range funcDecls(file) { + if fd.Type.Params == nil || !rewritten[i] { + continue } for _, field := range fd.Type.Params.List { for _, name := range field.Names { @@ -334,8 +342,7 @@ func fixBlankParams(fset *token.FileSet, src []byte) []byte { } } } - return true - }) + } if !changed { return src @@ -348,6 +355,17 @@ func fixBlankParams(fset *token.FileSet, src []byte) []byte { return buf.Bytes() } +// funcDecls returns the function declarations of file in source order. +func funcDecls(file *ast.File) []*ast.FuncDecl { + var fds []*ast.FuncDecl + for _, decl := range file.Decls { + if fd, ok := decl.(*ast.FuncDecl); ok { + fds = append(fds, fd) + } + } + return fds +} + // ensureImport adds an import of path to the source if not already present. func ensureImport(src []byte, path string) ([]byte, error) { fset := token.NewFileSet() diff --git a/testdata/blank_params.txt b/testdata/blank_params.txt index 1413dbf..a2c424c 100644 --- a/testdata/blank_params.txt +++ b/testdata/blank_params.txt @@ -1,6 +1,9 @@ # Blank identifier parameters are renamed to _p0, _p1, ... to avoid a gotip # GOEXPERIMENT=simd compiler bug where _ params in simd-using functions produce -# "cannot use _ as value or type". +# "cannot use _ as value or type". Only the functions loopvec rewrites are +# renamed: Offset reads the package-level _p0, which a parameter of that name +# would hide, and so do Local, which also has a variable named simd, and the +# first blank function. cp ops.go ops.go.orig loopvec -split . @@ -16,11 +19,28 @@ go 1.27.1 -- ops.go -- package test +var _p0 = 5 + +func Offset(_ int) int { return _p0 } + +func Local(_ int) int { + simd := struct{ x int }{1} + return _p0 + simd.x +} + +func _(_ int) int { return _p0 } + func FillBlankParams(_ int, _ byte, dst []float32) { for i := range dst { dst[i] = 1 } } + +func _(_ int, dst []float32) { + for i := range dst { + dst[i] = 2 + } +} -- ops_simd.go.want -- //go:build goexperiment.simd @@ -28,6 +48,17 @@ package test import "simd" +var _p0 = 5 + +func Offset(_ int) int { return _p0 } + +func Local(_ int) int { + simd := struct{ x int }{1} + return _p0 + simd.x +} + +func _(_ int) int { return _p0 } + func FillBlankParams(_p0 int, _p1 byte, dst []float32) { _vcFloat32s0 := simd.BroadcastFloat32s(1) for _i := 0; _i < len(dst); { @@ -35,15 +66,40 @@ func FillBlankParams(_p0 int, _p1 byte, dst []float32) { _i += _n } } + +func _(_p2 int, dst []float32) { + _vcFloat32s1 := simd.BroadcastFloat32s(2) + for _i := 0; _i < len(dst); { + _n := _vcFloat32s1.StorePart(dst[_i:]) + _i += _n + } +} -- stderr.want -- -loopvec: rewrote 1 loop(s) in $WORK/ops.go +loopvec: rewrote 2 loop(s) in $WORK/ops.go -- ops.go.guarded -- //go:build !goexperiment.simd package test +var _p0 = 5 + +func Offset(_ int) int { return _p0 } + +func Local(_ int) int { + simd := struct{ x int }{1} + return _p0 + simd.x +} + +func _(_ int) int { return _p0 } + func FillBlankParams(_ int, _ byte, dst []float32) { for i := range dst { dst[i] = 1 } } + +func _(_ int, dst []float32) { + for i := range dst { + dst[i] = 2 + } +} diff --git a/testdata/offset_negative_constant.txt b/testdata/offset_negative_constant.txt new file mode 100644 index 0000000..d23cc26 --- /dev/null +++ b/testdata/offset_negative_constant.txt @@ -0,0 +1,90 @@ +# A read at a negative constant offset is rewritten while every index it reads can +# be zero or more. When an index is a negative constant, the first one in a loop +# with a constant start (a[i-1] from 0) or every one in a loop with a constant limit +# (a[i-16] in a loop that stops before 16), the scalar loop panics when it runs and +# the vector loop would index at a negative constant, which does not compile, so the +# loop is left alone, whether the start and limit are written as literals, named +# constants or constant expressions. The rewritten package must build. + +loopvec -json . +cmpenv stdout stdout.want + +loopvec -split . +cmpenv stderr stderr.want +env GOEXPERIMENT=simd +exec go build . + +-- go.mod -- +module example.com/test + +go 1.27.1 +-- ops.go -- +package test + +func FirstIndexZeroConstant(dst, a []int32) { + for i := 15; i < 31; i++ { + dst[i] = a[i-15] + 1 + } +} + +func EveryIndexNegative(dst, a []int32) { + for i := 0; i < 16; i++ { + dst[i] = a[i-16] + 1 + } +} + +func FarBelow(dst, a []float32) { + for i := 0; i < 16; i++ { + dst[i] = a[i-20] * a[i] + } +} + +func FirstIndexNegative(dst, a []int32, n int) { + for i := 0; i < n; i++ { + dst[i] = a[i-1] * 3 + } +} + +func FirstIndexZero(dst, a []int32, n int) { + for i := 1; i < n; i++ { + dst[i] = a[i-1] * 3 + } +} + +const s0, lim = 0, 16 + +func NamedLimit(dst, a []int32, k int) { + for i := k; i < lim; i++ { + dst[i] = a[i-20] + 1 + } +} + +func NamedStart(dst, a []int32, n int) { + for i := s0; i < n; i++ { + dst[i] = a[i-1] * 3 + } +} + +func ExpressionStart(dst, a []int32, n int) { + for i := 1 - 1; i < n; i++ { + dst[i] = a[i-1] * 3 + } +} + +func NamedStartCopy(dst, a []int32, n int) { + for i := s0; i < n; i++ { + dst[i] = a[i-1] + } +} +-- stdout.want -- +{"file":"$WORK/ops.go","line":4,"func":"FirstIndexZeroConstant","vectorized":true} +{"file":"$WORK/ops.go","line":10,"func":"EveryIndexNegative","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":16,"func":"FarBelow","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":22,"func":"FirstIndexNegative","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":28,"func":"FirstIndexZero","vectorized":true} +{"file":"$WORK/ops.go","line":36,"func":"NamedLimit","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":42,"func":"NamedStart","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":48,"func":"ExpressionStart","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":54,"func":"NamedStartCopy","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +-- stderr.want -- +loopvec: rewrote 2 loop(s) in $WORK/ops.go diff --git a/testdata/reserved_names.txt b/testdata/reserved_names.txt new file mode 100644 index 0000000..2a98eaa --- /dev/null +++ b/testdata/reserved_names.txt @@ -0,0 +1,175 @@ +# Every name loopvec adds begins with an underscore, and some are declared before +# the loop or inside it, where they would shadow the function's own: a parameter +# named _w1 or _count, a limit named _i, or a local named like a broadcast vector +# would then refer to loopvec's variable, so a function that uses such a name is +# left alone; the blank identifier is not such a name. The code also relies on +# predeclared names (len, min, copy, uint64, ...) and on the packages simd and +# unsafe: a loop where one of them, looked up from the loop, is a local, a +# package-level declaration or an import of something else is left alone too, +# while a declaration the loop cannot see (max after it) does not matter. The +# scalar loop that finishes a reduction redeclares the loop's variables around +# its bound, so a loop variable must not take one of those names either, nor a +# name used by the range operand or the start of a loop that counts down +# (for n := range n, for n := n - 1; n >= 0; n--). The overlap helper loopvec +# declares in the package relies on a few more predeclared names (uintptr, +# ...), which the package must not redeclare. The rewritten package must build. + +loopvec -json ./... +cmpenv stdout stdout.want + +loopvec -split . +cmpenv stderr stderr.want +env GOEXPERIMENT=simd +exec go build . + +-- go.mod -- +module example.com/test + +go 1.27.1 +-- ops.go -- +package test + +func WindowName(dst, _w1 []int32) { + for i := range dst { + dst[i] = _w1[i] + 1 + } +} + +func CountName(dst, a []int32, _count int) { + for i := 0; i < len(dst); i++ { + dst[i] = a[i+_count] + 1 + } +} + +func IndexName(dst, a []int32, _i int) { + for i := 0; i < _i; i++ { + dst[i] = a[i] + 1 + } +} + +func BroadcastName(dst []float32, c float32) float32 { + _vcFloat32s0 := c * 2 + for i := range dst { + dst[i] *= c + } + return _vcFloat32s0 +} + +func Blank(dst, a []int32, _ int) { + for i := range dst { + dst[i] = a[i] + 1 + } +} + +func LocalLen(dst, a []int32) { + len := func(s []int32) int { return 2 } + _ = len + for i := range dst { + dst[i] = a[i] + 1 + } +} + +func LocalMin(dst, a []int32, k int) { + min := func(x, y int) int { return 0 } + _ = min + for i := 0; i < 8; i++ { + dst[i] = a[i+k] + 1 + } +} + +func MaxAfterLoop(dst, a []int32) int32 { + for i := range dst { + dst[i] = a[i] + 1 + } + max := int32(0) + for _, v := range dst { + if v > max { + max = v + } + } + return max +} +func SumCount(a []int32, n int) int32 { + var s int32 + for n := range n { + s += a[n] + } + return s +} + +func SumIndexLen(a []int32) int32 { + var s int32 + for len := range a { + s += a[len] + } + return s +} + +func SumDown(a []int32, n int) int32 { + var s int32 + for n := n - 1; n >= 0; n-- { + s += a[n] + } + return s +} + +func SumByte(a []byte) byte { + var s byte + { + type byte = int8 + var _ byte + for _, x := range a { + s += x + } + } + return s +} +-- pkglen/ops.go -- +package pkglen + +func len(s []int32) int { return cap(s) / 4 } + +func Double(dst, a []int32, n int) { + for i := 0; i < n; i++ { + dst[i] = a[i] * 2 + } +} +-- pkguintptr/ops.go -- +package pkguintptr + +type uintptr = int32 + +func Inc(dst, a []int32) { + for i := range dst { + dst[i] = a[i] + 1 + } +} +-- alias/ops.go -- +package alias + +import simd "strings" + +var _ = simd.ToUpper + +func Double(dst, a []int32) { + for i := range dst { + dst[i] = a[i] * 2 + } +} +-- stdout.want -- +{"file":"$WORK/ops.go","line":4,"func":"WindowName","vectorized":false,"reason":"the function uses a name that begins with an underscore, as loopvec's own names do","stage":"analysis"} +{"file":"$WORK/ops.go","line":10,"func":"CountName","vectorized":false,"reason":"the function uses a name that begins with an underscore, as loopvec's own names do","stage":"analysis"} +{"file":"$WORK/ops.go","line":16,"func":"IndexName","vectorized":false,"reason":"the function uses a name that begins with an underscore, as loopvec's own names do","stage":"analysis"} +{"file":"$WORK/ops.go","line":23,"func":"BroadcastName","vectorized":false,"reason":"the function uses a name that begins with an underscore, as loopvec's own names do","stage":"analysis"} +{"file":"$WORK/ops.go","line":30,"func":"Blank","vectorized":true} +{"file":"$WORK/ops.go","line":38,"func":"LocalLen","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +{"file":"$WORK/ops.go","line":46,"func":"LocalMin","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +{"file":"$WORK/ops.go","line":52,"func":"MaxAfterLoop","vectorized":true} +{"file":"$WORK/ops.go","line":65,"func":"SumCount","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +{"file":"$WORK/ops.go","line":73,"func":"SumIndexLen","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +{"file":"$WORK/ops.go","line":81,"func":"SumDown","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +{"file":"$WORK/alias/ops.go","line":8,"func":"Double","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +{"file":"$WORK/pkglen/ops.go","line":6,"func":"Double","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +{"file":"$WORK/pkguintptr/ops.go","line":6,"func":"Inc","vectorized":false,"reason":"a name the rewritten code relies on (len, min, simd, ...) means something else here","stage":"analysis"} +-- stderr.want -- +loopvec: rewrote 2 loop(s) in $WORK/ops.go diff --git a/testdata/toolexec_falls_back_when_rewrite_does_not_compile.txt b/testdata/toolexec_falls_back_when_rewrite_does_not_compile.txt index e8555d5..f87bc51 100644 --- a/testdata/toolexec_falls_back_when_rewrite_does_not_compile.txt +++ b/testdata/toolexec_falls_back_when_rewrite_does_not_compile.txt @@ -1,6 +1,6 @@ # A rewrite that does not type-check is discarded: the package is compiled from # its original source, the build succeeds, and the log says why. Here the -# parameter named simd shadows the package the rewrite imports. +# package already declares _loopvecOverlap, the helper the rewrite adds. env GOEXPERIMENT=simd env GOCACHE=$WORK/gocache @@ -9,7 +9,7 @@ env LOOPVEC_TOOLEXEC_LOG_PKGS=example.com/app/vec exec go build -toolexec=$LOOPVEC_TOOLEXEC -o app . ! stdout . ! stderr . -cmpenv log log.want +grep -count=1 '^kept example\.com/app/vec: rewritten source rejected: .*/vec/vec\.go:[0-9]+:[0-9]+: _loopvecOverlap redeclared in this block$' log exec ./app cmp stdout want.out @@ -18,17 +18,15 @@ cmp stdout want.out module example.com/app go 1.27.1 --- vec/vec.go -- +-- vec/helper.go -- package vec -// The parameter named simd shadows the package the rewrite would import. -func AddInt32s(simd int, dst, a, b []int32) { - for i := range dst { - dst[i] = a[i] + b[i] + int32(simd) - } -} +// _loopvecOverlap has the name of the helper the rewrite would add. +func _loopvecOverlap() {} +-- vec/vec.go -- +package vec -func Add(simd int, dst, a, b []int32) { +func Add(dst, a, b []int32) { for i := range dst { dst[i] = a[i] + b[i] } @@ -44,10 +42,8 @@ import ( func main() { d := make([]int32, 3) - vec.Add(0, d, []int32{1, 2, 3}, []int32{10, 20, 30}) + vec.Add(d, []int32{1, 2, 3}, []int32{10, 20, 30}) fmt.Println(d) } -- want.out -- [11 22 33] --- log.want -- -kept example.com/app/vec: rewritten source rejected: $WORK/vec/vec.go:10:22: simd.BroadcastInt32s undefined (type int has no field or method BroadcastInt32s)