[thesyncim]

The rewritten code adds names that start with an underscore, some of them declared before or inside the loop. It also calls len, min, max and copy, converts with uint64 and the element types, and refers to the packages simd and unsafe. The function, its package or an import can give any of those names another meaning, and then the vector loop silently computes something else. These all go wrong on master:

func F(dst, a []int32, _i int) {
	for i := 0; i < _i; i++ {
		dst[i] = a[i] + 1
	}
}

The vector loop's condition reads loopvec's _i instead of the parameter, so it never runs. In a package that declares func len(s []int32) int { return cap(s) / 4 }, the vector loop takes its length from that len and stops a quarter of the way. And a sum written as for n := range n { s += a[n] }, or as for n := n - 1; n >= 0; n--, finishes its last elements with for n := _i; n < n; n++, which never runs, so the result is short.

A loop that would otherwise be rewritten is now left alone, with its own reason, when:

• its function uses an identifier that starts with an underscore, other than _. Rather than track which names each part of the emitter uses, this covers the names loopvec adds now and any it adds later;
• one of the names the code relies on, looked up from the loop, resolves to something other than the predeclared one or the imported package. The lookup starts at the loop's position, so a max declared after the loop doesn't count. The overlap helper's own names (uintptr, int, bool, any, false) are checked at package level, where it is declared. The toolexec wrapper now records scopes for this;
• the loop itself declares one of those names, or a name its range operand or initial values use (for n := range n, for n := n - 1; ..., for len := range a).

These checks run last, so a loop that can't be vectorized anyway keeps the reason that matters.

The second commit fixes the same kind of bug in fixBlankParams. It renames _ parameters to _p0, _p1, ... to work around a compiler bug with simd code, but it did that in every function of a rewritten file, so func Offset(_ int) int { return _p0 } started returning its argument instead of a package-level _p0. Now it only renames in the functions loopvec rewrote, which can't mention such names.

No loop in std, gonum or goav1 stops being vectorized. reserved_names.txt and blank_params.txt cover each case. A rewrite that doesn't type-check still falls back in toolexec; toolexec_falls_back_when_rewrite_does_not_compile.txt used a parameter named simd to get one, which is now rejected earlier, so it uses a package that declares _loopvecOverlap itself instead.

This is #3 again, now that #2 is in.

[mauri870]

Thanks, this is a real fix: each case in the description computes the wrong result silently on master. I checked that the two name lists cover everything the emitter writes. That's len, min, max, copy, the uint64(...) shift counts, the var _bufN [N]T element types, and simd and unsafe, plus any, int, bool, false and uintptr in the overlap helper. Both callers also fill info.Scopes. loopvec -json output is byte-identical to master's over std (1663 loops) and gonum (3251), reasons included, and make test passes. The fixBlankParams commit looks right to me.

Two things to change:

1. The header check rejects loops that master rewrites correctly. The problem only exists in the scalar loop that finishes a reduction, which redeclares the loop variable around its bound. HeaderShadowsName runs on every loop, though. Master rewrites these two correctly, because the vector loop is for _i := 0; _i < n; outside the loop's scope and the fallback is the original loop:

   func RangeN(dst, a []int32, n int) {
   	for n := range n {
   		dst[n] = a[n] + 1
   	}
   }
   
   func DownN(dst, a []int32, n int) {
   	for n := n - 1; n >= 0; n-- {
   		dst[n] = a[n] + 1
   	}
   }

   With this PR both are rejected with "a name the rewritten code relies on ... means something else here", which isn't true for them. Could the check apply only when the plan has a reduction (a statement with Acc != nil)? A case like the ones above in reserved_names.txt would cover it.

2. toolexec_falls_back_when_rewrite_does_not_compile.txt can keep its want-section. The log is a single deterministic line, and this passes:

   cmpenv log log.want
   ...
   -- log.want --
   kept example.com/app/vec: rewritten source rejected: $WORK/vec/vec.go:27:6: _loopvecOverlap redeclared in this block
   

   We prefer want-sections over grep in the scripts where possible.

Nits, take them or leave them:

• UsesReservedNames, HeaderShadowsName and ShadowsEmittedName are in internal/loopir/lower.go, but Lower never calls them. They're analysis-stage checks (Stage() reports "analysis"), so internal/analysis or a separate loopir/names.go would fit better.
• AGENTS.md lists what is rejected and why, so one sentence on the underscore rule and the shadowing rule would help the next reader.
• In reserved_names.txt, a blank line is missing before func SumCount. The two accepted cases (Blank, MaxAfterLoop) are only built, never compared with expected output or run. Either move them out and name the rest reserved_names_no_rewrite.txt, or add an ops_simd.go.want section.