diff options
author | Keith Randall <khr@golang.org> | 2021-03-23 14:48:47 -0700 |
---|---|---|
committer | Keith Randall <khr@golang.org> | 2021-03-24 17:54:03 +0000 |
commit | 771c57e68ed5ef2bbb0eafc0d48419f59d143932 (patch) | |
tree | ae4c6e8519345dd9a4c89761bd5509eedfcf00b0 /src/cmd/compile/internal/ssa/shortcircuit.go | |
parent | fd5e0bd385660275a8049a2499e0bbbffd465bc8 (diff) | |
download | go-771c57e68ed5ef2bbb0eafc0d48419f59d143932.tar.gz go-771c57e68ed5ef2bbb0eafc0d48419f59d143932.zip |
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 #45175
Change-Id: I2411dca03948653c053291f6829a76bec0c32330
Reviewed-on: https://go-review.googlesource.com/c/go/+/304251
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Diffstat (limited to 'src/cmd/compile/internal/ssa/shortcircuit.go')
-rw-r--r-- | src/cmd/compile/internal/ssa/shortcircuit.go | 18 |
1 files changed, 18 insertions, 0 deletions
diff --git a/src/cmd/compile/internal/ssa/shortcircuit.go b/src/cmd/compile/internal/ssa/shortcircuit.go index c72c86a67e..29abf3c591 100644 --- a/src/cmd/compile/internal/ssa/shortcircuit.go +++ b/src/cmd/compile/internal/ssa/shortcircuit.go @@ -138,6 +138,24 @@ func shortcircuitBlock(b *Block) bool { if len(b.Values) != nval+nOtherPhi { return false } + if nOtherPhi > 0 { + // Check for any phi which is the argument of another phi. + // These cases are tricky, as substitutions done by replaceUses + // are no longer trivial to do in any ordering. See issue 45175. + m := make(map[*Value]bool, 1+nOtherPhi) + for _, v := range b.Values { + if v.Op == OpPhi { + m[v] = true + } + } + for v := range m { + for _, a := range v.Args { + if a != v && m[a] { + return false + } + } + } + } // Locate index of first const phi arg. cidx := -1 |