diff --git a/AGENTS.md b/AGENTS.md index cbf99b5..f0c876e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -89,10 +89,14 @@ safety argument stays structural: down, need one added to a bound, so they are accepted only where that cannot overflow (`plusOne`): a constant, or something minus a positive constant. A constant limit of zero or less, or a negative constant start, would put a negative - constant index in the emitted code and is rejected. `Trip` states every loop as - the forward `Start <= i < Limit`; a limit that is not exactly `len` of the stored - slice makes every operand slice carry an explicit cap (`Plan.Bound`), or the last - vector would write past it. + constant index in the emitted code and is rejected. A constant written with + literals alone (`16`, `1 - 1`) is spelled as its value; one that names a constant + keeps the name (`spell`), since the emitted file is compiled for every build and + the name can stand for another value in another (a file for each GOARCH). Checks + that need a constant read `Trip.StartValue`/`LimitValue`, never the text. `Trip` + states every loop as the forward `Start <= i < Limit`; a limit that is not exactly + `len` of the stored slice makes every operand slice carry an explicit cap + (`Plan.Bound`), or the last vector would write past it. - A slice operand is a variable or a chain of field selections ending in a slice (`x.f`, `x.f.g`) whose root variable the body does not assign (`sliceRef`). The body has no call and stores only elements, so nothing in it changes the header a field holds. The @@ -107,9 +111,13 @@ safety argument stays structural: iteration writes, so the scalar loop sees the old value and the emitter loads it before any store of the chunk (`statements` hoists those loads). A negative offset is a recurrence (`ReasonCarriedDependence`); a variable distance or a - loop that counts down is `ReasonOffsetOfStored`. The length check is - against the last index read (`lastIndex`), and offset loads are not capped at the - limit. Every other store and load is `ident[i]` (`index`) — `dst[i+1]`, `dst[2*i]` are rejected outright, regardless of + loop that counts down is `ReasonOffsetOfStored`. A constant offset that puts an + index below zero, the lowest one read with a constant start or every one with a + constant limit, is `ReasonNegativeIndex`: the scalar loop panics and the emitted + code would index at a negative constant. A positive one that pushes a constant + start or limit past the largest int is rejected as that bound, since the emitted + code adds them. The length check is against the last index read (`lastIndex`), + and offset loads are not capped at the limit. Every other store and load is `ident[i]` (`index`) — `dst[i+1]`, `dst[2*i]` are rejected outright, regardless of whether they're actually dependent. That is what guarantees no two iterations touch the same element. - A non-slice operand becomes an `Invariant` only if it is a variable, a diff --git a/BUGS.md b/BUGS.md index 843087e..deb82fc 100644 --- a/BUGS.md +++ b/BUGS.md @@ -2,12 +2,12 @@ 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 | -| [#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` | +| [#80657](https://github.com/golang/go/issues/80657) simd in a method: internal compiler error | closed, 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 yet; 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` | | [#81846](https://github.com/golang/go/issues/81846) `Float32s.Neg` rebuilds its sign constant every iteration | open | speed of negation loops | none; the loop is correct, only slower than it should be | @@ -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). @@ -42,8 +42,21 @@ vector loop still wins and one that is mostly scalar float code around it can lo `simd/archsimd` (amd64) has `ClearAVXUpperBits`, not the portable `simd`, so loopvec has no portable way to clear the state. When comparing a function before and after a rewrite, benchmark the whole function, not the loop. This is separate from #81847: it reproduces with -async preemption off. One machine and no performance counters, so the size of the effect on -Intel, where the penalty works differently, is unknown. +async preemption off. + +Two more things from loopvec's runs, also on the 9950X3D. `GOAMD64=v3` does not remove the +penalty: the compiler still emits legacy SSE for scalar float code, except where it fuses a +multiply-add into a VEX `VFMADD231SD`. And `ClearAVXUpperBits()` costs nothing measurable +after a loop that stays in vectors (4096 `float32` adds, 255.3 against 254.0 ns). On real +code, one rewritten `work[i] = 1` fill in gonum's `Dlantb` made the scalar loops after it 7x +slower (420 to 2917 us), and the clear brought it back to 409 us. + +Others have measured Intel in the issue thread: Skylake-X pays 2x to 3x on scalar code after +an AVX-512 kernel, and on Sapphire Rapids and Granite Rapids a kernel that reloads with +legacy SSE loses about 15 points of peak until the clear is added. The same thread reports +`ClearAVXUpperBits()` at the return of every short leaf function costing about 0.8 ns per +call, so inserting it unconditionally is not free. loopvec does not emit it yet; it is +amd64-only and would need an arch-tagged helper. ## #81844: blank identifier parameters diff --git a/Makefile b/Makefile index 7b6f652..ac89073 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -export GOTOOLCHAIN ?= go1.27.1 +export GOTOOLCHAIN ?= go1.27.3 .DEFAULT_GOAL := build @@ -8,9 +8,10 @@ test: build go test ./... -count=1 -race # The scripts run the arm64 binaries they build, which needs an aarch64 binfmt -# handler as well as qemu-aarch64-static. +# handler as well as qemu-aarch64-static. Under emulation the script suite takes +# several times as long, near go test's default 10m timeout on a CI runner. test-qemu-arm64: build - GOARCH=arm64 GOOS=linux CGO_ENABLED=0 go test ./... -count=1 -exec=qemu-aarch64-static + GOARCH=arm64 GOOS=linux CGO_ENABLED=0 go test ./... -count=1 -timeout=30m -exec=qemu-aarch64-static bench-fuzz: GOEXPERIMENT=simd go test ./bench/ -count=1 -run Fuzz diff --git a/cmd/loopvec-toolexec/compile.go b/cmd/loopvec-toolexec/compile.go index adf3ea4..82a6962 100644 --- a/cmd/loopvec-toolexec/compile.go +++ b/cmd/loopvec-toolexec/compile.go @@ -328,6 +328,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 80c0f21..2025be4 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,10 @@ module github.com/mauri870/loopvec go 1.27.1 +toolchain go1.27.3 + require ( + github.com/pmezard/go-difflib v1.0.0 golang.org/x/tools v0.52.1-0.20261103172851-aba6dfd3c19d rsc.io/script v0.0.2 ) diff --git a/go.sum b/go.sum index 457ea15..73d56f3 100644 --- a/go.sum +++ b/go.sum @@ -478,6 +478,7 @@ github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo= 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/ir.go b/internal/loopir/ir.go index 981fe77..9b596b0 100644 --- a/internal/loopir/ir.go +++ b/internal/loopir/ir.go @@ -136,6 +136,12 @@ type Trip struct { // Start and Limit are loop-invariant integer expressions, as source text. // Start is "0" for the common case. Start, Limit string + // StartConst and LimitConst are set when Start and Limit are constants, whose + // values are StartValue and LimitValue. The text of a constant that names + // another constant keeps the name (see spell), so the checks that need a + // constant read these. + StartConst, LimitConst bool + StartValue, LimitValue int64 // Slice is the slice whose length is Limit, when Limit is exactly len(Slice). Slice *Ref // NonEmpty is set when both are constants and Start < Limit, so the loop runs diff --git a/internal/loopir/lower.go b/internal/loopir/lower.go index c4a7719..cf9e6d9 100644 --- a/internal/loopir/lower.go +++ b/internal/loopir/lower.go @@ -7,6 +7,7 @@ import ( "go/token" "go/types" "math" + "slices" "strconv" "strings" ) @@ -100,7 +101,7 @@ func (l *lowerer) rangeLoop(s *ast.RangeStmt) (*Loop, Reason) { l.valueVar = l.info.Defs[valueIdent] l.valueRef = ref } - return l.body(s, s.Body, Trip{Start: "0", Limit: "len(" + ref.Text() + ")", Slice: ref}, 1) + return l.body(s, s.Body, Trip{Start: "0", StartConst: true, Limit: "len(" + ref.Text() + ")", Slice: ref}, 1) } if s.Value != nil { return nil, ReasonUnsupportedRange @@ -215,7 +216,7 @@ func (l *lowerer) reverse(s *ast.ForStmt, start, limit ast.Expr, strict bool) (* if !isConst || low == math.MaxInt64 { return nil, ReasonUnsupportedBound } - from = bound{text: strconv.FormatInt(low+1, 10), value: low + 1, isConst: true} + from = bound{text: spell(limit, low, 1), value: low + 1, isConst: true} } else if from, ok = l.bound(limit); !ok { return nil, ReasonUnsupportedBound } @@ -269,10 +270,14 @@ type bound struct { // trip states a loop that runs from to to (exclusive) as a Trip. func (l *lowerer) trip(from, to bound) Trip { return Trip{ - Start: from.text, - Limit: to.text, - Slice: to.slice, - NonEmpty: from.isConst && to.isConst && from.value < to.value, + Start: from.text, + Limit: to.text, + StartConst: from.isConst, + LimitConst: to.isConst, + StartValue: from.value, + LimitValue: to.value, + Slice: to.slice, + NonEmpty: from.isConst && to.isConst && from.value < to.value, } } @@ -286,6 +291,9 @@ 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 { + b.text = spell(e, b.value, 0) + } if call, ok := ast.Unparen(e).(*ast.CallExpr); ok { b.slice, _ = l.lenOf(call) } @@ -296,29 +304,57 @@ func (l *lowerer) bound(e ast.Expr) (bound, bool) { // need. It is only accepted where adding one cannot overflow: a constant below the // largest int, or something minus a positive constant. func (l *lowerer) plusOne(e ast.Expr) (bound, bool) { - if v, isConst := l.constant(e); isConst { - if v == math.MaxInt64 { - return bound{}, false - } - return bound{text: strconv.FormatInt(v+1, 10), value: v + 1, isConst: true}, true - } - sub, ok := ast.Unparen(e).(*ast.BinaryExpr) - if !ok || sub.Op != token.SUB { - return bound{}, false - } - k, isConst := l.constant(sub.Y) - if !isConst || k < 1 || !l.pureInt(sub.X) { + v, isConst := l.constant(e) + if isConst && v == math.MaxInt64 { return bound{}, false } - // x - k + 1 is x - (k-1); with k == 1 that is x itself. - x, ok := l.bound(sub.X) - if !ok { - return bound{}, false + // x - k + 1 is x - (k-1); with k == 1 that is x itself. A constant that names + // another is written this way too, so width - 1 + 1 is width. One written with + // literals alone is spelled as its value. + if sub, ok := ast.Unparen(e).(*ast.BinaryExpr); ok && sub.Op == token.SUB && !literal(e) { + if k, kConst := l.constant(sub.Y); kConst && k >= 1 { + if x, ok := l.bound(sub.X); ok { + if k == 1 { + return x, true + } + return bound{text: x.text + " - " + strconv.FormatInt(k-1, 10), value: v + 1, isConst: isConst}, true + } + } } - if k == 1 { - return x, true + if isConst { + return bound{text: spell(e, v, 1), value: v + 1, isConst: true}, true } - return bound{text: x.text + " - " + strconv.FormatInt(k-1, 10)}, true + return bound{}, false +} + +// spell is the source text of the constant e, whose value is value, plus delta. A +// constant written with literals alone (16, 1 - 1) is spelled as its value. One +// that names a constant keeps the name, since the emitted file is compiled for +// every build and the name can stand for another value in another one (a file for +// each GOARCH). +func spell(e ast.Expr, value, delta int64) string { + if literal(e) { + return strconv.FormatInt(value+delta, 10) + } + // Every integer operator binds at least as tightly as +, and operators that bind + // as tightly group from the left, so the sum needs no parentheses. + text := types.ExprString(e) + if delta != 0 { + text += " + " + strconv.FormatInt(delta, 10) + } + return text +} + +// literal reports whether e is written with literals and operators alone, no names. +func literal(e ast.Expr) bool { + found := false + ast.Inspect(e, func(n ast.Node) bool { + if _, ok := n.(*ast.Ident); ok { + found = true + } + return !found + }) + return !found } // constant returns the value of e when it is an integer constant that fits an int64. @@ -370,6 +406,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 { @@ -510,6 +661,17 @@ func (l *lowerer) body(node ast.Stmt, block *ast.BlockStmt, trip Trip, step int6 // the chunk. A read of an earlier iteration's element (d < 0) is a recurrence, // and a distance that is not a constant, or a loop that counts down, has no // order loopvec can check. + // + // A constant offset that puts an index below zero makes the scalar loop panic + // when it runs. Start is the lowest index the loop visits, whichever way it + // counts, so a constant start puts the lowest index read at Start+off, and a + // constant limit puts the highest at Limit-1+off. Such a loop is left alone, + // since the vector loop would index or slice at that negative constant, which + // does not compile. Start is never negative and Limit never below one (see + // rangeReason), so only a negative offset can do this, and it is at least + // -maxOffset, so negating it cannot overflow. A positive offset that pushes a + // constant start or limit past the largest int would not compile either, as the + // emitted code adds them. stored := map[types.Object]bool{} for _, stmt := range body { if store, ok := stmt.(Store); ok { @@ -519,15 +681,29 @@ func (l *lowerer) body(node ast.Stmt, block *ast.BlockStmt, trip Trip, step int6 for _, stmt := range body { for _, leaf := range Leaves(stmtValue(stmt)) { load, ok := leaf.(*Load) - if !ok || load.Off == "" || !stored[load.Ref.Obj] { + if !ok || load.Off == "" { continue } distance, err := strconv.ParseInt(load.Off, 10, 64) - switch { - case err != nil || step != 1: - return nil, ReasonOffsetOfStored - case distance < 0: - return nil, ReasonCarriedDependence + if stored[load.Ref.Obj] { + switch { + case err != nil || step != 1: + return nil, ReasonOffsetOfStored + case distance < 0: + return nil, ReasonCarriedDependence + } + } + if err == nil && distance < 0 && + (trip.StartConst && trip.StartValue < -distance || trip.LimitConst && trip.LimitValue <= -distance) { + return nil, ReasonNegativeIndex + } + if err == nil && distance > 0 { + switch { + case trip.StartConst && trip.StartValue > math.MaxInt64-distance: + return nil, ReasonUnsupportedStart + case trip.LimitConst && trip.LimitValue > math.MaxInt64-distance: + return nil, ReasonUnsupportedBound + } } } } diff --git a/internal/loopir/plan.go b/internal/loopir/plan.go index d4bf63b..bbe9e61 100644 --- a/internal/loopir/plan.go +++ b/internal/loopir/plan.go @@ -1,9 +1,6 @@ package loopir -import ( - "go/types" - "strconv" -) +import "go/types" // maxSlices is the most distinct slices a loop may touch. Each stored slice is // checked at run time against every other slice in the loop, so the number of @@ -166,22 +163,18 @@ func NewPlan(l *Loop, opts Options) (*Plan, Reason) { p.Stmts = append(p.Stmts, planned) } if len(p.Stmts) == 1 && p.Stmts[0].Dst != nil { - if load, ok := p.Stmts[0].Root.(*Load); ok { - p.Copy = !negativeSource(l.Ind.Trip.Start, load.Off) - } + _, p.Copy = p.Stmts[0].Root.(*Load) } trip := l.Ind.Trip // A loop that runs a few times, known at compile time, is faster as it is. - if from, errFrom := strconv.ParseInt(trip.Start, 10, 64); errFrom == nil { - if to, errTo := strconv.ParseInt(trip.Limit, 10, 64); errTo == nil { - least := int64(minTrip) - if p.Full { - least = int64(MinReduceVectors * MaxLanes(elem)) - } - if to-from < least { - return nil, ReasonShortLoop - } + if trip.StartConst && trip.LimitConst { + least := int64(minTrip) + if p.Full { + least = int64(MinReduceVectors * MaxLanes(elem)) + } + if trip.LimitValue-trip.StartValue < least { + return nil, ReasonShortLoop } } p.Start, p.Limit, p.NonEmpty, p.TripRef = trip.Start, trip.Limit, trip.NonEmpty, trip.Slice @@ -228,16 +221,6 @@ func NewPlan(l *Loop, opts Options) (*Plan, Reason) { return p, "" } -// negativeSource reports whether a copy would slice its source at a negative -// constant, which does not compile: the loop starts at start and reads at offset -// off, and both are constants that add up to less than zero. Such a loop panics -// when it runs, which the ordinary vector loop does as well. -func negativeSource(start, off string) bool { - s, errStart := strconv.ParseInt(start, 10, 64) - o, errOff := strconv.ParseInt(off, 10, 64) - return errStart == nil && errOff == nil && s+o < 0 -} - // checkOps returns why simd cannot express an operation in v on the element // type, or "". func checkOps(elem types.Type, v Value) Reason { diff --git a/internal/loopir/reason.go b/internal/loopir/reason.go index aaeca53..20dfb66 100644 --- a/internal/loopir/reason.go +++ b/internal/loopir/reason.go @@ -17,7 +17,9 @@ const ( ReasonUnsupportedRange Reason = "range clause is not i := range s, i, v := range s, or i := range n" // ReasonUnsupportedStart covers a counting loop whose start is not a // loop-invariant integer expression (a constant, an int variable, len of a - // slice, and sums and products of those), or is a negative constant. + // slice, and sums and products of those), is a negative constant, or is a + // constant that a read at a positive constant offset pushes past the largest + // int. ReasonUnsupportedStart Reason = "loop start is not a loop-invariant integer expression" // ReasonUnsupportedCondition covers a loop condition that is not // i < limit or i <= limit, or i >= limit or i > limit when counting down. @@ -28,7 +30,9 @@ const ( // ReasonUnsupportedBound covers a limit that is not a loop-invariant integer // expression, is a constant that is not positive, or is written so that // adding one to it could overflow (a <= limit, or the start of a loop that - // counts down, that is not a constant or something minus a constant). + // counts down, that is not a constant or something minus a constant), or is + // a constant that a read at a positive constant offset pushes past the + // largest int. ReasonUnsupportedBound Reason = "loop limit is not a loop-invariant integer expression" // ReasonUnsupportedDestination covers a destination that isn't a plain // slice[i] element: an offset (dst[i+1]), a stride (dst[2*i]), or a @@ -42,6 +46,13 @@ 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 one read at the lowest index the loop visits, with a + // constant start (a[i-1] in a loop over 0..n-1, whichever way it counts), or + // every one, with a constant limit (a[i-20] in a loop that stops before 16). The + // scalar loop panics when it reaches that index, 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 +107,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 +136,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/main.go b/main.go index 61dd22f..adb0b26 100644 --- a/main.go +++ b/main.go @@ -21,11 +21,11 @@ import ( "go/format" "go/token" "os" - "os/exec" "runtime" "runtime/pprof" "strings" + "github.com/pmezard/go-difflib/difflib" "golang.org/x/tools/go/packages" "github.com/mauri870/loopvec/internal/analysis" @@ -319,34 +319,25 @@ func addBuildTag(src []byte, tag string) []byte { return formatted } -// printDiff prints a unified diff between orig and rewritten for path, -// using the system diff command. Both sides are labeled with path, so the -// header does not depend on the temporary files or on which diff is installed -// (GNU and BSD diff print their timestamps differently). +// printDiff prints a unified diff between orig and rewritten for path, with +// both sides labeled with path. It needs no diff command, so the output is the +// same everywhere. func printDiff(path string, orig, rewritten []byte) error { - origFile, err := os.CreateTemp("", "loopvec-orig-*.go") - if err != nil { - return err - } - defer func() { _ = os.Remove(origFile.Name()) }() - if _, err := origFile.Write(orig); err != nil { - _ = origFile.Close() - return err - } - _ = origFile.Close() + return difflib.WriteUnifiedDiff(os.Stdout, difflib.UnifiedDiff{ + A: diffLines(orig), + B: diffLines(rewritten), + FromFile: path, + ToFile: path, + Context: 3, + }) +} - newFile, err := os.CreateTemp("", "loopvec-new-*.go") - if err != nil { - return err - } - defer func() { _ = os.Remove(newFile.Name()) }() - if _, err := newFile.Write(rewritten); err != nil { - _ = newFile.Close() - return err +// diffLines splits src into lines that keep their newline. difflib.SplitLines +// would add an empty line after a final newline. +func diffLines(src []byte) []string { + lines := strings.SplitAfter(string(src), "\n") + if lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] } - _ = newFile.Close() - - out, _ := exec.Command("diff", "-u", "-L", path, "-L", path, origFile.Name(), newFile.Name()).Output() - _, err = os.Stdout.Write(out) - return err + return lines } diff --git a/testdata/abs_sqrt.txt b/testdata/abs_sqrt.txt index d6368e2..5a44e70 100644 --- a/testdata/abs_sqrt.txt +++ b/testdata/abs_sqrt.txt @@ -5,9 +5,9 @@ # output is compared bit for bit. # testmain/output was recorded on amd64: a NaN that an operation creates (the square -# root of a negative number, 0 times infinity) has the sign bit set there and clear -# on arm64, so the recording is compared only on amd64, and on every target the -# simd build is compared with the scalar build of the same machine. +# root of a negative number) has the sign bit set there and clear on arm64, so only +# the scalar build is compared with the recording, and only on amd64. On every +# target the simd build is compared with the scalar build of the same machine. # The scalar program is the reference behavior. exec go run ./testmain @@ -24,7 +24,6 @@ cmp ops.go ops.go.guarded # The simd build must behave identically. env GOEXPERIMENT=simd exec go run ./testmain -[GOARCH:amd64] cmp stdout testmain/output cmp stdout scalar.out -- go.mod -- diff --git a/testdata/activation_functions.txt b/testdata/activation_functions.txt index d9cbac6..621bf43 100644 --- a/testdata/activation_functions.txt +++ b/testdata/activation_functions.txt @@ -3,9 +3,16 @@ # Both branches are computed for every lane, and the results must still match the # scalar loop for NaN, infinities and signed zeros, which compare the way Go does. +# testmain/output was recorded on amd64. These loops only pass the input NaN +# through, but a NaN that an operation creates (an alpha of 0 times an infinity) +# has the sign bit set there and clear on arm64, so only the scalar build is +# compared with the recording, and only on amd64. On every target the simd build +# is compared with the scalar build of the same machine. + # The scalar program is the reference behavior. exec go run ./testmain -cmp stdout testmain/output +[GOARCH:amd64] cmp stdout testmain/output +cp stdout scalar.out cp ops.go ops.go.orig loopvec -split . @@ -17,7 +24,7 @@ cmp ops.go ops.go.guarded # The simd build must behave identically. env GOEXPERIMENT=simd exec go run ./testmain -cmp stdout testmain/output +cmp stdout scalar.out -- go.mod -- module example.com/test diff --git a/testdata/activation_temp.txt b/testdata/activation_temp.txt index 5526f8d..6b65c05 100644 --- a/testdata/activation_temp.txt +++ b/testdata/activation_temp.txt @@ -4,10 +4,10 @@ # if/else if, then use it. Results must match the scalar loop bit for bit, NaN, # infinities and signed zeros included. -# testmain/output was recorded on amd64: a NaN that an operation creates (the square -# root of a negative number, 0 times infinity) has the sign bit set there and clear -# on arm64, so the recording is compared only on amd64, and on every target the -# simd build is compared with the scalar build of the same machine. +# testmain/output was recorded on amd64: a NaN that an operation creates (0 times +# infinity) has the sign bit set there and clear on arm64, so only the scalar build +# is compared with the recording, and only on amd64. On every target the simd build +# is compared with the scalar build of the same machine. exec go run ./testmain [GOARCH:amd64] cmp stdout testmain/output @@ -22,7 +22,6 @@ cmp ops.go ops.go.guarded env GOEXPERIMENT=simd exec go run ./testmain -[GOARCH:amd64] cmp stdout testmain/output cmp stdout scalar.out -- go.mod -- 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/bound_arch_constant.txt b/testdata/bound_arch_constant.txt new file mode 100644 index 0000000..649af02 --- /dev/null +++ b/testdata/bound_arch_constant.txt @@ -0,0 +1,236 @@ +# A start or limit that names a constant keeps the name in the emitted code, since +# the file is compiled for every build and the constant can have another value in +# another one. Here width differs by GOARCH: written out as its value on one +# architecture, the vector loop would stop short, or run past the slices, on the +# others. A constant written with literals alone is spelled as its value. + +exec go run ./testmain +cmp stdout testmain/output + +loopvec -split . +! stdout . +cmpenv stderr stderr.want +cmp ops_simd.go ops_simd.go.want + +env GOEXPERIMENT=simd +exec go run ./testmain +cmp stdout testmain/output + +-- go.mod -- +module example.com/test + +go 1.27.1 +-- width_amd64.go -- +package test + +const width = 32 +-- width_arm64.go -- +package test + +const width = 64 +-- width_other.go -- +//go:build !amd64 && !arm64 + +package test + +const width = 48 +-- width.go -- +package test + +// Width is width, for testmain to check against. +func Width() int { return width } +-- ops.go -- +package test + +func Add(dst, a, b []float32) { + for i := 0; i < width; i++ { + dst[i] = a[i] + b[i] + } +} + +func AddInclusive(dst, a, b []float32) { + for i := 0; i <= width-1; i++ { + dst[i] = a[i] + b[i] + } +} + +func AddDown(dst, a, b []float32) { + for i := width - 1; i >= 0; i-- { + dst[i] = a[i] + b[i] + } +} + +func AddAbove(dst, a, b []float32) { + for i := len(dst) - 1; i > width; i-- { + dst[i] = a[i] + b[i] + } +} + +func AddLiteral(dst, a, b []float32) { + for i := 0; i < 2*8; i++ { + dst[i] = a[i] + b[i] + } +} +-- ops_simd.go.want -- +//go:build goexperiment.simd + +package test + +import ( + "simd" + "unsafe" +) + +func Add(dst, a, b []float32) { + if _loopvecOverlap(dst, a, 0, width, 0, width) || _loopvecOverlap(dst, b, 0, width, 0, width) { + for i := 0; i < width; i++ { + dst[i] = a[i] + b[i] + } + } else { + _ = dst[width-1] + _ = a[width-1] + _ = b[width-1] + for _i := 0; _i < width; { + _v1, _n := simd.LoadFloat32sPart(a[_i:width]) + _v2, _ := simd.LoadFloat32sPart(b[_i:width]) + _v1.Add(_v2).StorePart(dst[_i:width]) + _i += _n + } + } +} + +func AddInclusive(dst, a, b []float32) { + if _loopvecOverlap(dst, a, 0, width, 0, width) || _loopvecOverlap(dst, b, 0, width, 0, width) { + for i := 0; i <= width-1; i++ { + dst[i] = a[i] + b[i] + } + } else { + _ = dst[width-1] + _ = a[width-1] + _ = b[width-1] + for _i := 0; _i < width; { + _v1, _n := simd.LoadFloat32sPart(a[_i:width]) + _v2, _ := simd.LoadFloat32sPart(b[_i:width]) + _v1.Add(_v2).StorePart(dst[_i:width]) + _i += _n + } + } +} + +func AddDown(dst, a, b []float32) { + if _loopvecOverlap(dst, a, 0, width, 0, width) || _loopvecOverlap(dst, b, 0, width, 0, width) { + for i := width - 1; i >= 0; i-- { + dst[i] = a[i] + b[i] + } + } else { + _ = dst[width-1] + _ = a[width-1] + _ = b[width-1] + for _i := 0; _i < width; { + _v1, _n := simd.LoadFloat32sPart(a[_i:width]) + _v2, _ := simd.LoadFloat32sPart(b[_i:width]) + _v1.Add(_v2).StorePart(dst[_i:width]) + _i += _n + } + } +} + +func AddAbove(dst, a, b []float32) { + if _loopvecOverlap(dst, a, width+1, len(dst), width+1, len(dst)) || _loopvecOverlap(dst, b, width+1, len(dst), width+1, len(dst)) { + for i := len(dst) - 1; i > width; i-- { + dst[i] = a[i] + b[i] + } + } else { + if len(dst) > width+1 { + _ = a[len(dst)-1] + _ = b[len(dst)-1] + for _i := width + 1; _i < len(dst); { + _v1, _n := simd.LoadFloat32sPart(a[_i:]) + _v2, _ := simd.LoadFloat32sPart(b[_i:]) + _v1.Add(_v2).StorePart(dst[_i:]) + _i += _n + } + } + } +} + +func AddLiteral(dst, a, b []float32) { + if _loopvecOverlap(dst, a, 0, 16, 0, 16) || _loopvecOverlap(dst, b, 0, 16, 0, 16) { + for i := 0; i < 2*8; i++ { + dst[i] = a[i] + b[i] + } + } else { + _ = dst[16-1] + _ = a[16-1] + _ = b[16-1] + for _i := 0; _i < 16; { + _v1, _n := simd.LoadFloat32sPart(a[_i:16]) + _v2, _ := simd.LoadFloat32sPart(b[_i:16]) + _v1.Add(_v2).StorePart(dst[_i:16]) + _i += _n + } + } +} + +func _loopvecOverlap[T any](a, b []T, window ...int) bool { + aLo, aHi, bLo, bHi := 0, len(a), 0, len(b) + if len(window) == 4 { + aLo, aHi, bLo, bHi = window[0], window[1], window[2], window[3] + } + if aLo >= aHi || bLo >= bHi { + return false + } + size := unsafe.Sizeof(a[0]) + aBase := uintptr(unsafe.Pointer(unsafe.SliceData(a))) + bBase := uintptr(unsafe.Pointer(unsafe.SliceData(b))) + aStart, aEnd := aBase+uintptr(aLo)*size, aBase+uintptr(aHi)*size + bStart, bEnd := bBase+uintptr(bLo)*size, bBase+uintptr(bHi)*size + return aStart < bEnd && bStart < aEnd +} +-- stderr.want -- +loopvec: rewrote 5 loop(s) in $WORK/ops.go +-- testmain/main.go -- +package main + +import ( + "fmt" + + "example.com/test" +) + +const n = 100 + +// check reports whether f stored a[i] + b[i] at exactly the indexes lo..hi-1 of +// dst and left every other element alone. +func check(f func(dst, a, b []float32), lo, hi int) bool { + dst, a, b := make([]float32, n), make([]float32, n), make([]float32, n) + for i := range a { + a[i], b[i], dst[i] = float32(i), float32(2*i), -1 + } + f(dst, a, b) + for i := range dst { + want := float32(-1) + if lo <= i && i < hi { + want = a[i] + b[i] + } + if dst[i] != want { + return false + } + } + return true +} + +func main() { + w := test.Width() + fmt.Println(check(test.Add, 0, w)) + fmt.Println(check(test.AddInclusive, 0, w)) + fmt.Println(check(test.AddDown, 0, w)) + fmt.Println(check(test.AddAbove, w+1, n)) + fmt.Println(check(test.AddLiteral, 0, 16)) +} +-- testmain/output -- +true +true +true +true +true diff --git a/testdata/flag_diff.txt b/testdata/flag_diff.txt index 5d7bf4a..0bd5f39 100644 --- a/testdata/flag_diff.txt +++ b/testdata/flag_diff.txt @@ -1,16 +1,11 @@ -# -d prints a unified diff, labeled with the file's path on both sides so the -# headers are the same with GNU and BSD diff, and leaves the source untouched. +# -d prints a unified diff, labeled with the file's path on both sides, and +# leaves the source untouched. loopvec computes the diff itself, so the output +# does not depend on a diff command and is checked whole. cp ops.go ops.go.orig loopvec -d . cmpenv stderr stderr.want -stdout -count=1 '^--- \S+/ops\.go$' -stdout -count=1 '^\+\+\+ \S+/ops\.go$' -stdout -count=1 '^@@ -1,7 \+1,43 @@$' -stdout -count=1 '^\+//go:build goexperiment.simd$' -stdout -count=1 '^\+import \($' -stdout -count=1 '^- for i := range dst \{$' -stdout -count=1 '^\+ _v1.Add\(_v2\).StorePart\(dst\[_i:\]\)$' +cmpenv stdout stdout.want cmp ops.go ops.go.orig ! exists ops_simd.go @@ -28,3 +23,52 @@ func AddInt32s(dst, a, b []int32) { } -- stderr.want -- loopvec: rewrote 1 loop(s) in $WORK/ops.go +-- stdout.want -- +--- $WORK/ops.go ++++ $WORK/ops.go +@@ -1,7 +1,43 @@ ++//go:build goexperiment.simd ++ + package test + ++import ( ++ "simd" ++ "unsafe" ++) ++ + func AddInt32s(dst, a, b []int32) { +- for i := range dst { +- dst[i] = a[i] + b[i] ++ if _loopvecOverlap(dst, a) || _loopvecOverlap(dst, b) { ++ for i := range dst { ++ dst[i] = a[i] + b[i] ++ } ++ } else { ++ if len(dst) > 0 { ++ _ = a[len(dst)-1] ++ _ = b[len(dst)-1] ++ for _i := 0; _i < len(dst); { ++ _v1, _n := simd.LoadInt32sPart(a[_i:]) ++ _v2, _ := simd.LoadInt32sPart(b[_i:]) ++ _v1.Add(_v2).StorePart(dst[_i:]) ++ _i += _n ++ } ++ } + } + } ++ ++func _loopvecOverlap[T any](a, b []T, window ...int) bool { ++ aLo, aHi, bLo, bHi := 0, len(a), 0, len(b) ++ if len(window) == 4 { ++ aLo, aHi, bLo, bHi = window[0], window[1], window[2], window[3] ++ } ++ if aLo >= aHi || bLo >= bHi { ++ return false ++ } ++ size := unsafe.Sizeof(a[0]) ++ aBase := uintptr(unsafe.Pointer(unsafe.SliceData(a))) ++ bBase := uintptr(unsafe.Pointer(unsafe.SliceData(b))) ++ aStart, aEnd := aBase+uintptr(aLo)*size, aBase+uintptr(aHi)*size ++ bStart, bEnd := bBase+uintptr(bLo)*size, bBase+uintptr(bHi)*size ++ return aStart < bEnd && bStart < aEnd ++} diff --git a/testdata/flag_methods_ice.txt b/testdata/flag_methods_ice.txt index 0948c91..c2ec597 100644 --- a/testdata/flag_methods_ice.txt +++ b/testdata/flag_methods_ice.txt @@ -1,4 +1,4 @@ -# -methods rewrites method bodies, but go1.27.1 cannot compile them under +# -methods rewrites method bodies, but go1.27.3 cannot compile them under # GOEXPERIMENT=simd: the compiler crashes (golang/go#80657, fixed in later # toolchains). This pins the crash so the README limitation stays accurate. # When this script starts failing on a newer toolchain, the limitation is gone: diff --git a/testdata/invariant_call.txt b/testdata/invariant_call.txt index 3f1fcf5..eafaef1 100644 --- a/testdata/invariant_call.txt +++ b/testdata/invariant_call.txt @@ -3,10 +3,10 @@ # the call is evaluated once before the loop. The results must match the scalar loop # bit for bit, which is the only way to compare NaN and negative zero. -# testmain/output was recorded on amd64: a NaN that an operation creates (the square -# root of a negative number, 0 times infinity) has the sign bit set there and clear -# on arm64, so the recording is compared only on amd64, and on every target the -# simd build is compared with the scalar build of the same machine. +# testmain/output was recorded on amd64: a NaN that an operation creates (0 times +# infinity) has the sign bit set there and clear on arm64, so only the scalar build +# is compared with the recording, and only on amd64. On every target the simd build +# is compared with the scalar build of the same machine. exec go run ./testmain [GOARCH:amd64] cmp stdout testmain/output @@ -21,7 +21,6 @@ cmp ops.go ops.go.guarded env GOEXPERIMENT=simd exec go run ./testmain -[GOARCH:amd64] cmp stdout testmain/output cmp stdout scalar.out -- go.mod -- diff --git a/testdata/offset_negative_constant.txt b/testdata/offset_negative_constant.txt new file mode 100644 index 0000000..6927e61 --- /dev/null +++ b/testdata/offset_negative_constant.txt @@ -0,0 +1,223 @@ +# A read at a negative constant offset is rewritten while every index it reads can +# be zero or more: from a constant start at least as large as the offset, a named +# one included, which the emitted code keeps by name, and up to a constant limit +# that leaves the highest index read at zero or more. Loops whose index is a +# negative constant are left alone (see offset_negative_constant_no_rewrite.txt). + +exec go run ./testmain +cmp stdout testmain/output + +loopvec -split . +! stdout . +cmpenv stderr stderr.want +cmp ops_simd.go ops_simd.go.want + +env GOEXPERIMENT=simd +exec go run ./testmain +cmp stdout testmain/output + +-- 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 FirstIndexZero(dst, a []int32, n int) { + for i := 1; i < n; i++ { + dst[i] = a[i-1] * 3 + } +} + +const s1 = 1 + +func NamedStart(dst, a []int32, n int) { + for i := s1; i < n; i++ { + dst[i] = a[i-1] * 3 + } +} + +func NamedStartCopy(dst, a []int32, n int) { + for i := s1; i < n; i++ { + dst[i] = a[i-1] + } +} + +func LastIndexZero(dst, a []int32, k int) { + for i := k; i < 16; i++ { + dst[i] = a[i-15] + 1 + } +} +-- ops_simd.go.want -- +//go:build goexperiment.simd + +package test + +import ( + "simd" + "unsafe" +) + +func FirstIndexZeroConstant(dst, a []int32) { + _vcInt32s0 := simd.BroadcastInt32s(1) + if _loopvecOverlap(dst, a, 15, 31, 0, 31) { + for i := 15; i < 31; i++ { + dst[i] = a[i-15] + 1 + } + } else { + _ = dst[31-1] + _ = a[31-16] + for _i := 15; _i < 31; { + _v1, _n := simd.LoadInt32sPart(a[_i-15:]) + _v1.Add(_vcInt32s0).StorePart(dst[_i:31]) + _i += _n + } + } +} + +func FirstIndexZero(dst, a []int32, n int) { + _vcInt32s1 := simd.BroadcastInt32s(3) + if _loopvecOverlap(dst, a, 1, n, 0, n) { + for i := 1; i < n; i++ { + dst[i] = a[i-1] * 3 + } + } else { + if n > 1 { + _ = dst[n-1] + _ = a[n-2] + for _i := 1; _i < n; { + _v1, _n := simd.LoadInt32sPart(a[_i-1:]) + _v1.Mul(_vcInt32s1).StorePart(dst[_i:n]) + _i += _n + } + } + } +} + +const s1 = 1 + +func NamedStart(dst, a []int32, n int) { + _vcInt32s2 := simd.BroadcastInt32s(3) + if _loopvecOverlap(dst, a, s1, n, s1-1, n) { + for i := s1; i < n; i++ { + dst[i] = a[i-1] * 3 + } + } else { + if n > s1 { + _ = dst[n-1] + _ = a[n-2] + for _i := s1; _i < n; { + _v1, _n := simd.LoadInt32sPart(a[_i-1:]) + _v1.Mul(_vcInt32s2).StorePart(dst[_i:n]) + _i += _n + } + } + } +} + +func NamedStartCopy(dst, a []int32, n int) { + if _loopvecOverlap(dst, a, s1, n, s1-1, n) { + for i := s1; i < n; i++ { + dst[i] = a[i-1] + } + } else { + if n > s1 { + _ = dst[n-1] + _ = a[n-2] + copy(dst[s1:n], a[s1-1:]) + } + } +} + +func LastIndexZero(dst, a []int32, k int) { + _vcInt32s4 := simd.BroadcastInt32s(1) + if _loopvecOverlap(dst, a, k, 16, k-15, 16) { + for i := k; i < 16; i++ { + dst[i] = a[i-15] + 1 + } + } else { + if 16 > k { + _ = dst[16-1] + _ = a[16-16] + for _i := k; _i < 16; { + _v1, _n := simd.LoadInt32sPart(a[_i-15:]) + _v1.Add(_vcInt32s4).StorePart(dst[_i:16]) + _i += _n + } + } + } +} + +func _loopvecOverlap[T any](a, b []T, window ...int) bool { + aLo, aHi, bLo, bHi := 0, len(a), 0, len(b) + if len(window) == 4 { + aLo, aHi, bLo, bHi = window[0], window[1], window[2], window[3] + } + if aLo >= aHi || bLo >= bHi { + return false + } + size := unsafe.Sizeof(a[0]) + aBase := uintptr(unsafe.Pointer(unsafe.SliceData(a))) + bBase := uintptr(unsafe.Pointer(unsafe.SliceData(b))) + aStart, aEnd := aBase+uintptr(aLo)*size, aBase+uintptr(aHi)*size + bStart, bEnd := bBase+uintptr(bLo)*size, bBase+uintptr(bHi)*size + return aStart < bEnd && bStart < aEnd +} +-- stderr.want -- +loopvec: rewrote 5 loop(s) in $WORK/ops.go +-- testmain/main.go -- +package main + +import ( + "fmt" + + "example.com/test" +) + +func try(f func()) (panicked bool) { + defer func() { panicked = recover() != nil }() + f() + return +} + +func ramp(n int) []int32 { + s := make([]int32, n) + for i := range s { + s[i] = int32(i + 1) + } + return s +} + +func main() { + dst := make([]int32, 31) + test.FirstIndexZeroConstant(dst, ramp(16)) + fmt.Println(dst) + + for _, f := range []func(dst, a []int32, n int){test.FirstIndexZero, test.NamedStart, test.NamedStartCopy} { + dst := make([]int32, 21) + f(dst, ramp(21), 21) + fmt.Println(dst) + fmt.Println(try(func() { f(make([]int32, 21), ramp(19), 21) })) + } + + dst = make([]int32, 16) + test.LastIndexZero(dst, ramp(1), 15) + fmt.Println(dst) + fmt.Println(try(func() { test.LastIndexZero(make([]int32, 16), ramp(1), 14) })) +} +-- testmain/output -- +[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17] +[0 3 6 9 12 15 18 21 24 27 30 33 36 39 42 45 48 51 54 57 60] +true +[0 3 6 9 12 15 18 21 24 27 30 33 36 39 42 45 48 51 54 57 60] +true +[0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20] +true +[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2] +true diff --git a/testdata/offset_negative_constant_no_rewrite.txt b/testdata/offset_negative_constant_no_rewrite.txt new file mode 100644 index 0000000..abad41e --- /dev/null +++ b/testdata/offset_negative_constant_no_rewrite.txt @@ -0,0 +1,109 @@ +# A read at a negative constant offset is left alone when an index it reads is a +# negative constant: the one at the lowest index the loop visits, with a constant +# start (a[i-1] over 0..n-1, whichever way the loop counts, and over a range of a +# slice), or every one, with a constant limit (a[i-16] in a loop that stops before +# 16). The scalar loop panics when it reaches that index, and the vector loop would +# index at a negative constant, which does not compile. The start and limit may be +# literals, named constants or constant expressions (see offset_negative_constant.txt +# for the loops that are rewritten). A read at a positive constant offset that +# pushes a constant start or limit past the largest int is left alone too, since the +# emitted code would add them. + +cp ops.go ops.go.orig +loopvec -split . +! stdout . +! stderr . +! exists ops_simd.go +cmp ops.go ops.go.orig + +loopvec -json . +cmpenv stdout stdout.want + +-- go.mod -- +module example.com/test + +go 1.27.1 +-- ops.go -- +package test + +import "math" + +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 + } +} + +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] + } +} + +func CountingDownToZero(dst, a []int32) { + for i := 15; i >= 0; i-- { + dst[i] = a[i-1] + 1 + } +} + +func RangeOverSlice(dst, a []int32) { + for i := range dst { + dst[i] = a[i-1] + 1 + } +} + +func LimitPastLargest(dst, a []int32) { + for i := 0; i < math.MaxInt64; i++ { + dst[i] = a[i+1] + 1 + } +} + +func StartPastLargest(dst, a []int32, n int) { + for i := math.MaxInt64 - 1; i < n; i++ { + dst[i] = a[i+2] + 1 + } +} +-- stdout.want -- +{"file":"$WORK/ops.go","line":6,"func":"EveryIndexNegative","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":12,"func":"FarBelow","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":18,"func":"FirstIndexNegative","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":26,"func":"NamedLimit","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":32,"func":"NamedStart","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":38,"func":"ExpressionStart","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":44,"func":"NamedStartCopy","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":50,"func":"CountingDownToZero","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":56,"func":"RangeOverSlice","vectorized":false,"reason":"a constant offset puts an index below zero","stage":"lower"} +{"file":"$WORK/ops.go","line":62,"func":"LimitPastLargest","vectorized":false,"reason":"loop limit is not a loop-invariant integer expression","stage":"lower"} +{"file":"$WORK/ops.go","line":68,"func":"StartPastLargest","vectorized":false,"reason":"loop start is not a loop-invariant integer expression","stage":"lower"} 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) diff --git a/testdata/toolexec_pgo.txt b/testdata/toolexec_pgo.txt new file mode 100644 index 0000000..974d0a6 --- /dev/null +++ b/testdata/toolexec_pgo.txt @@ -0,0 +1,103 @@ +# With PGO, go test compiles a package once for a main package's binary and +# again for its test binary, with the same inputs, and can compile the test +# against one copy and link the other. The linker rejects the build unless both +# copies of a rewritten package are the same archive. A profile with no samples +# is enough to turn PGO on. + +cd mkprof +exec go run . $WORK/cmd/bench/default.pgo +! stdout . +! stderr . +cd $WORK + +env GOEXPERIMENT=simd +env GOCACHE=$WORK/gocache +env LOOPVEC_TOOLEXEC_LOG=$WORK/log +env LOOPVEC_TOOLEXEC_LOG_PKGS=example.com/app/vec +exec go test -toolexec=$LOOPVEC_TOOLEXEC ./... +stdout -count=1 '^ok example\.com/app/vec ' +stdout -count=1 '^ok example\.com/app/cmd/bench ' +! stderr . +cmp log log.want + +-- go.mod -- +module example.com/app + +go 1.27.1 +-- vec/vec.go -- +package vec + +func Add(dst, a, b []float32) { + for i := range dst { + dst[i] = a[i] + b[i] + } +} +-- vec/vec_test.go -- +package vec + +import "testing" + +func TestAdd(t *testing.T) { + dst := make([]float32, 9) + Add(dst, []float32{1, 2, 3, 4, 5, 6, 7, 8, 9}, []float32{1, 1, 1, 1, 1, 1, 1, 1, 1}) + if dst[8] != 10 { + t.Fatal(dst) + } +} +-- cmd/bench/main.go -- +package main + +import "example.com/app/vec" + +func main() { + dst := make([]float32, 4) + vec.Add(dst, dst, dst) +} +-- cmd/bench/main_test.go -- +package main + +import ( + "testing" + + "example.com/app/vec" +) + +// The test imports vec itself, so the test binary gets a PGO copy of vec of its +// own. +func TestAdd(t *testing.T) { + dst := make([]float32, 4) + vec.Add(dst, []float32{1, 2, 3, 4}, []float32{1, 1, 1, 1}) + if dst[3] != 5 { + t.Fatal(dst) + } +} +-- mkprof/go.mod -- +module mkprof + +go 1.27.1 +-- mkprof/main.go -- +// mkprof writes a CPU profile with no samples to the file named by its argument. +package main + +import ( + "os" + "runtime/pprof" +) + +func main() { + f, err := os.Create(os.Args[1]) + if err != nil { + panic(err) + } + if err := pprof.StartCPUProfile(f); err != nil { + panic(err) + } + pprof.StopCPUProfile() + if err := f.Close(); err != nil { + panic(err) + } +} +-- log.want -- +rewrote 1 loop(s) in example.com/app/vec +rewrote 1 loop(s) in example.com/app/vec +rewrote 1 loop(s) in example.com/app/vec diff --git a/testdata/toolexec_reproducible.txt b/testdata/toolexec_reproducible.txt index ad9e082..6b4d65d 100644 --- a/testdata/toolexec_reproducible.txt +++ b/testdata/toolexec_reproducible.txt @@ -7,11 +7,16 @@ env GOEXPERIMENT=simd env LOOPVEC_TOOLEXEC_LOG=$WORK/log +env LOOPVEC_TOOLEXEC_LOG_PKGS=example.com/lib env GOCACHE=$WORK/gocache1 exec go build -buildmode=archive -toolexec=$LOOPVEC_TOOLEXEC -o a1.a . +! stdout . +! stderr . env GOCACHE=$WORK/gocache2 exec go build -buildmode=archive -toolexec=$LOOPVEC_TOOLEXEC -o a2.a . -grep -count=2 '^rewrote 1 loop\(s\) in example\.com/lib$' log +! stdout . +! stderr . +cmp log log.want cmp a1.a a2.a -- go.mod -- @@ -26,3 +31,6 @@ func Add(dst, a, b []float32) { dst[i] = a[i] + b[i] } } +-- log.want -- +rewrote 1 loop(s) in example.com/lib +rewrote 1 loop(s) in example.com/lib