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/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/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)