aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJorropo <jorropo.pgm@gmail.com>2023-11-05 22:40:01 +0100
committerGopher Robot <gobot@golang.org>2023-11-28 20:11:58 +0000
commitd77307f855b1aab9899d85798614af9f1b8dc735 (patch)
tree9914f8dbe06aad1b0af5d54591c740ce32dbb3f0
parent1bd76576fec9c23a2de3bd282ff9b1c0ba117c50 (diff)
downloadgo-d77307f855b1aab9899d85798614af9f1b8dc735.tar.gz
go-d77307f855b1aab9899d85798614af9f1b8dc735.zip
[release-branch.go1.20] cmd/compile: fix findIndVar so it does not match disjointed loop headers
Fix #63983 parseIndVar, prove and maybe more are on the assumption that the loop header is a single block. This can be wrong, ensure we don't match theses cases we don't know how to handle. In the future we could update them so that they know how to handle such cases but theses cases seems rare so I don't think the value would be really high. We could also run a loop canonicalization pass first which could handle this. The repro case looks weird because I massaged it so it would crash with the previous compiler. Change-Id: I4aa8afae9e90a17fa1085832250fc1139c97faa6 Reviewed-on: https://go-review.googlesource.com/c/go/+/539977 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> (cherry picked from commit 8b4e1259d0e82c8fe38a1456f997a4e9d63573a2) Reviewed-on: https://go-review.googlesource.com/c/go/+/539936 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Jorropo <jorropo.pgm@gmail.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
-rw-r--r--src/cmd/compile/internal/ssa/loopbce.go7
-rw-r--r--test/fixedbugs/issue63955.go22
2 files changed, 29 insertions, 0 deletions
diff --git a/src/cmd/compile/internal/ssa/loopbce.go b/src/cmd/compile/internal/ssa/loopbce.go
index d92566f2d3..696930fed8 100644
--- a/src/cmd/compile/internal/ssa/loopbce.go
+++ b/src/cmd/compile/internal/ssa/loopbce.go
@@ -130,6 +130,13 @@ func findIndVar(f *Func) []indVar {
less = false
}
+ if ind.Block != b {
+ // TODO: Could be extended to include disjointed loop headers.
+ // I don't think this is causing missed optimizations in real world code often.
+ // See https://go.dev/issue/63955
+ continue
+ }
+
// Expect the increment to be a nonzero constant.
if inc.Op != OpConst64 {
continue
diff --git a/test/fixedbugs/issue63955.go b/test/fixedbugs/issue63955.go
new file mode 100644
index 0000000000..258e874220
--- /dev/null
+++ b/test/fixedbugs/issue63955.go
@@ -0,0 +1,22 @@
+// compile
+
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package j
+
+func f(try func() int, shouldInc func() bool, N func(int) int) {
+ var n int
+loop: // we want to have 3 preds here, the function entry and both gotos
+ if v := try(); v == 42 || v == 1337 { // the two || are to trick findIndVar
+ if n < 30 { // this aims to be the matched block
+ if shouldInc() {
+ n++
+ goto loop
+ }
+ n = N(n) // try to prevent some block joining
+ goto loop
+ }
+ }
+}