This CL updates several frontend passes to stop relying on
ir.CurFunc (at least directly).
Change-Id: I3c3529e81e27fb05d54a828f081f7c7efc31af67
Reviewed-on: https://go-review.googlesource.com/c/go/+/520606
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Steps towards eliminating implicit dependencies on base.Pos and
ir.CurFunc. Mechanical CL produced with gofmt -r.
Change-Id: I070015513cb955cbe87f9a148d81db8c0d4b0dc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/520605
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
This includes version-dependent support for GOEXPERIMENT and
-d=loopvar, -d=loopvarhash, to allow testing/porting of old code.
Includes tests of downgrade (1.22 -> 1.21) and upgrade (1.21 -> 1.22)
based on //go:build lines (while running a 1.22 build/compiler).
Change-Id: Idd3be61a2b46acec33c7e7edac0924158cc726b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/508819
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This adds the loop type to the json/LSP logging, to help with
studies of how many loops of which kind were modified.
Change-Id: I637a630cd275b413259601c0070b963f3c6d2185
Reviewed-on: https://go-review.googlesource.com/c/go/+/496515
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
running on cmd/compile/internal/testdata/inlines now shows:
```
--- change set #1 (enabling changes causes failure)
b/b.go:16:6: loop variable i now per-iteration (loop inlined into b/b.go:10)
b/b.go:16:6: loop variable i now per-iteration
./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into a/a.go:18)
./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into ./main.go:37)
./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into ./main.go:38)
---
```
and
```
--- change set #2 (enabling changes causes failure)
./main.go:27:6: loop variable i now per-iteration
./main.go:27:6: loop variable i now per-iteration (loop inlined into ./main.go:35)
---
```
Still unsure about the utility of mentioning the inlined occurrence, but better
than mysteriously repeating the line over and over again.
Change-Id: I357f5d419ab4928fa316f4612eec3b75e7f8ac34
Reviewed-on: https://go-review.googlesource.com/c/go/+/494296
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: David Chase <drchase@google.com>
Fix comments, including duplicate is, wrong phrases and articles, misspellings, etc.
Change-Id: I8bfea53b9b275e649757cc4bee6a8a026ed9c7a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/493035
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: shuang cui <imcusg@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Using more of internal/bisect gives us more that will be deleted
from base/hashdebug.go when we have updated the tools that
need the old protocol. It is also cheaper: there is no allocation to
make a decision about whether to enable, and no locking unless
printing is needed.
Change-Id: I43ec398461205a1a9e988512a134ed6b3a3b1587
Reviewed-on: https://go-review.googlesource.com/c/go/+/493736
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Michael Stapelberg thought the former messages had
upside potential, Russ and I agreed.
Also slightly tweaked the json logging, not sure if
anyone will use it but it should at least be okay.
Change-Id: Iaab75114dd5f5d8f011fab22d32b57abc0272815
Reviewed-on: https://go-review.googlesource.com/c/go/+/493135
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
This is intended to support automated pairing of performance
regressions with transformed loops; there is already a POC
for doing this in the general missed-optimization case; the
difference here is the ability to describe an entire range,
which required some extra plumbing to acquire and publish
the ending line+column.
Change-Id: Ibe606786f6be917b5a9a69d773560ed716a0754d
Reviewed-on: https://go-review.googlesource.com/c/go/+/492717
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
HashDebugPos function/method included a parameter that was always
the same, and a variable in the same package as the hashdebug code.
So remove it.
(I wrote that code, there was no reason for it to be that way).
Also corrects a stale comment in the loopvar code.
Change-Id: Id3da69cfe6dadeb31d5de62fb76d15103a5d6152
Reviewed-on: https://go-review.googlesource.com/c/go/+/482816
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
but this time, correctly.
children of Returns can have For/Range loops in them,
and those must be visited.
Includes test to verify that the optimization occurs,
and also that the problematic case that broke the original
optimization is now correctly handled.
Change-Id: If5a94fd51c862d4bfb318fec78456b7b202f3fcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/472355
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This modifies the loopvar change to be tied to the
package if it is specified that way, and preserves
the change across inlining.
Down the road, this will be triggered (and flow correctly)
if the changed semantics are tied to Go version specified
in go.mod (or rather, for the compiler, by the specified
version for compilation).
Includes tests.
Change-Id: If54e8b6dd23273b86be5ba47838c90d38af9bd1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/463595
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Adds:
GOEXPERIMENT=loopvar (expected way of invoking)
-d=loopvar={-1,0,1,2,11,12} (for per-package control and/or logging)
-d=loopvarhash=... (for hash debugging)
loopvar=11,12 are for testing, benchmarking, and debugging.
If enabled,for loops of the form `for x,y := range thing`, if x and/or
y are addressed or captured by a closure, are transformed by renaming
x/y to a temporary and prepending an assignment to the body of the
loop x := tmp_x. This changes the loop semantics by making each
iteration's instance of x be distinct from the others (currently they
are all aliased, and when this matters, it is almost always a bug).
3-range with captured iteration variables are also transformed,
though it is a more complex transformation.
"Optimized" to do a simpler transformation for
3-clause for where the increment is empty.
(Prior optimization of address-taking under Return disabled, because
it was incorrect; returns can have loops for children. Restored in
a later CL.)
Includes support for -d=loopvarhash=<binary string> intended for use
with hash search and GOCOMPILEDEBUG=loopvarhash=<binary string>
(use `gossahash -e loopvarhash command-that-fails`).
Minor feature upgrades to hash-triggered features; clients can specify
that file-position hashes use only the most-inline position, and/or that
they use only the basenames of source files (not the full directory path).
Most-inlined is the right choice for debugging loop-iteration change
once the semantics are linked to the package across inlining; basename-only
makes it tractable to write tests (which, otherwise, depend on the full
pathname of the source file and thus vary).
Updates #57969.
Change-Id: I180a51a3f8d4173f6210c861f10de23de8a1b1db
Reviewed-on: https://go-review.googlesource.com/c/go/+/411904
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
