Skip to content

Commit f17b659

Browse files
randall77dr2chase
authored andcommitted
[release-branch.go1.15] cmd/compile: disable shortcircuit optimization for intertwined phi values
We need to be careful that when doing value graph surgery, we not re-substitute a value that has already been substituted. That can lead to confusing a previous iteration's value with the current iteration's value. The simple fix in this CL just aborts the optimization if it detects intertwined phis (a phi which is the argument to another phi). It might be possible to keep the optimization with a more complicated CL, but: 1) This CL is clearly safe to backport. 2) There were no instances of this abort triggering in all.bash, prior to the test introduced in this CL. Fixes #45187 Change-Id: I2411dca03948653c053291f6829a76bec0c32330 Reviewed-on: https://go-review.googlesource.com/c/go/+/304251 Trust: Keith Randall <[email protected]> Trust: Josh Bleecher Snyder <[email protected]> Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Josh Bleecher Snyder <[email protected]> (cherry picked from commit 771c57e) Reviewed-on: https://go-review.googlesource.com/c/go/+/304529
1 parent c77418f commit f17b659

File tree

2 files changed

+47
-0
lines changed

2 files changed

+47
-0
lines changed

src/cmd/compile/internal/ssa/shortcircuit.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,24 @@ func shortcircuitBlock(b *Block) bool {
138138
if len(b.Values) != nval+nOtherPhi {
139139
return false
140140
}
141+
if nOtherPhi > 0 {
142+
// Check for any phi which is the argument of another phi.
143+
// These cases are tricky, as substitutions done by replaceUses
144+
// are no longer trivial to do in any ordering. See issue 45175.
145+
m := make(map[*Value]bool, 1+nOtherPhi)
146+
for _, v := range b.Values {
147+
if v.Op == OpPhi {
148+
m[v] = true
149+
}
150+
}
151+
for v := range m {
152+
for _, a := range v.Args {
153+
if a != v && m[a] {
154+
return false
155+
}
156+
}
157+
}
158+
}
141159

142160
// Locate index of first const phi arg.
143161
cidx := -1

test/fixedbugs/issue45175.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// run
2+
3+
// Copyright 2021 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
//go:noinline
10+
func f(c bool) int {
11+
b := true
12+
x := 0
13+
y := 1
14+
for b {
15+
b = false
16+
y = x
17+
x = 2
18+
if c {
19+
return 3
20+
}
21+
}
22+
return y
23+
}
24+
25+
func main() {
26+
if got := f(false); got != 0 {
27+
panic(got)
28+
}
29+
}

0 commit comments

Comments
 (0)