cmd/compile: don't fuse branches with side effects
Count Values with side effects but no use as live, and don't fuse
branches that contain such Values. (This can happen e.g. when it
is followed by an infinite loop.) Otherwise this may lead to
miscompilation (side effect fired at wrong condition) or ICE (two
stores live simultaneously).
Fixes #36005.
Change-Id: If202eae4b37cb7f0311d6ca120ffa46609925157
Reviewed-on: https://go-review.googlesource.com/c/go/+/210179
Reviewed-by: Keith Randall <khr@golang.org>
diff --git a/src/cmd/compile/internal/ssa/fuse.go b/src/cmd/compile/internal/ssa/fuse.go
index a530874..c2d4051 100644
--- a/src/cmd/compile/internal/ssa/fuse.go
+++ b/src/cmd/compile/internal/ssa/fuse.go
@@ -145,7 +145,7 @@
// There may be false positives.
func isEmpty(b *Block) bool {
for _, v := range b.Values {
- if v.Uses > 0 || v.Type.IsVoid() {
+ if v.Uses > 0 || v.Op.IsCall() || v.Op.HasSideEffects() || v.Type.IsVoid() {
return false
}
}
diff --git a/src/cmd/compile/internal/ssa/fuse_test.go b/src/cmd/compile/internal/ssa/fuse_test.go
index c3e25a8..77d2aad 100644
--- a/src/cmd/compile/internal/ssa/fuse_test.go
+++ b/src/cmd/compile/internal/ssa/fuse_test.go
@@ -63,7 +63,7 @@
t.Errorf("then was not eliminated, but should have")
}
if b == fun.blocks["else"] && b.Kind != BlockInvalid {
- t.Errorf("then was not eliminated, but should have")
+ t.Errorf("else was not eliminated, but should have")
}
}
}
@@ -97,7 +97,7 @@
t.Errorf("then was not eliminated, but should have")
}
if b == fun.blocks["else"] && b.Kind != BlockInvalid {
- t.Errorf("then was not eliminated, but should have")
+ t.Errorf("else was not eliminated, but should have")
}
}
}
@@ -131,6 +131,40 @@
}
}
+func TestFuseSideEffects(t *testing.T) {
+ // Test that we don't fuse branches that have side effects but
+ // have no use (e.g. followed by infinite loop).
+ // See issue #36005.
+ c := testConfig(t)
+ fun := c.Fun("entry",
+ Bloc("entry",
+ Valu("mem", OpInitMem, types.TypeMem, 0, nil),
+ Valu("b", OpArg, c.config.Types.Bool, 0, nil),
+ If("b", "then", "else")),
+ Bloc("then",
+ Valu("call1", OpStaticCall, types.TypeMem, 0, nil, "mem"),
+ Goto("empty")),
+ Bloc("else",
+ Valu("call2", OpStaticCall, types.TypeMem, 0, nil, "mem"),
+ Goto("empty")),
+ Bloc("empty",
+ Goto("loop")),
+ Bloc("loop",
+ Goto("loop")))
+
+ CheckFunc(fun.f)
+ fuseAll(fun.f)
+
+ for _, b := range fun.f.Blocks {
+ if b == fun.blocks["then"] && b.Kind == BlockInvalid {
+ t.Errorf("then is eliminated, but should not")
+ }
+ if b == fun.blocks["else"] && b.Kind == BlockInvalid {
+ t.Errorf("else is eliminated, but should not")
+ }
+ }
+}
+
func BenchmarkFuse(b *testing.B) {
for _, n := range [...]int{1, 10, 100, 1000, 10000} {
b.Run(strconv.Itoa(n), func(b *testing.B) {
diff --git a/src/cmd/compile/internal/ssa/gen/main.go b/src/cmd/compile/internal/ssa/gen/main.go
index 2107da4..8520c68 100644
--- a/src/cmd/compile/internal/ssa/gen/main.go
+++ b/src/cmd/compile/internal/ssa/gen/main.go
@@ -405,6 +405,7 @@
fmt.Fprintln(w, "func (o Op) SymEffect() SymEffect { return opcodeTable[o].symEffect }")
fmt.Fprintln(w, "func (o Op) IsCall() bool { return opcodeTable[o].call }")
+ fmt.Fprintln(w, "func (o Op) HasSideEffects() bool { return opcodeTable[o].hasSideEffects }")
fmt.Fprintln(w, "func (o Op) UnsafePoint() bool { return opcodeTable[o].unsafePoint }")
// generate registers
diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go
index a5951dd..99dc606 100644
--- a/src/cmd/compile/internal/ssa/opGen.go
+++ b/src/cmd/compile/internal/ssa/opGen.go
@@ -31636,6 +31636,7 @@
func (o Op) UsesScratch() bool { return opcodeTable[o].usesScratch }
func (o Op) SymEffect() SymEffect { return opcodeTable[o].symEffect }
func (o Op) IsCall() bool { return opcodeTable[o].call }
+func (o Op) HasSideEffects() bool { return opcodeTable[o].hasSideEffects }
func (o Op) UnsafePoint() bool { return opcodeTable[o].unsafePoint }
var registers386 = [...]Register{
