internal/analysis: don't rewrite loops whose names the new code would capture

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

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
 

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

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

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
 )

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=

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
 }

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)

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