Skip to content

Commit

Permalink
[release-branch.go1.21] cmd/compile: fix findIndVar so it does not ma…
Browse files Browse the repository at this point in the history
…tch disjointed loop headers

Fix #63984

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 8b4e125)
Reviewed-on: https://go-review.googlesource.com/c/go/+/540535
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
  • Loading branch information
Jorropo authored and gopherbot committed Nov 28, 2023
1 parent 3684d19 commit f7a79cb
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/cmd/compile/internal/ssa/loopbce.go
Expand Up @@ -127,6 +127,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.isGenericIntConst() {
continue
Expand Down
22 changes: 22 additions & 0 deletions 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
}
}
}

0 comments on commit f7a79cb

Please sign in to comment.