[release-branch.go1.8] cmd/compile: don't move spills to loop exits where the spill...

We shouldn't move a spill to a loop exit where the spill itself
is dead.  The stack location assigned to the spill might already
be reused by another spill at this point.

The case we previously handled incorrectly is the one where the value
being spilled is still live, but the spill itself is dead.

Fixes #20472

Patching directly on the release branch because the spill moving code has
already been rewritten for 1.9. (And it doesn't have this bug.)

Change-Id: I26c5273dafd98d66ec448750073c2b354ef89ad6
Reviewed-on: https://go-review.googlesource.com/44033
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/cmd/compile/internal/ssa/export_test.go b/src/cmd/compile/internal/ssa/export_test.go
index ee6ed51d730ebce5d4f8957704c58a8c5be1cbf3..80fa92d94becb18380efdc3bf5ac7c5c17d1f890 100644 (file)
--- a/src/cmd/compile/internal/ssa/export_test.go
+++ b/src/cmd/compile/internal/ssa/export_test.go
@@ -35,8 +35,20 @@ type DummyFrontend struct {
 func (DummyFrontend) StringData(s string) interface{} {
        return nil
 }
-func (DummyFrontend) Auto(t Type) GCNode {
-       return nil
+
+type dummyGCNode struct {
+       typ  Type
+       name string
+}
+
+func (d *dummyGCNode) Typ() Type {
+       return d.typ
+}
+func (d *dummyGCNode) String() string {
+       return d.name
+}
+func (d DummyFrontend) Auto(t Type) GCNode {
+       return &dummyGCNode{typ: t, name: "dummy"}
 }
 func (d DummyFrontend) SplitString(s LocalSlot) (LocalSlot, LocalSlot) {
        return LocalSlot{s.N, d.TypeBytePtr(), s.Off}, LocalSlot{s.N, d.TypeInt(), s.Off + 8}

diff --git a/src/cmd/compile/internal/ssa/regalloc.go b/src/cmd/compile/internal/ssa/regalloc.go
index 7bf778609ee487ccbd5d475ecd0580f28b8e3ed3..90b5947f1cb01adf94199157e76674e969e064d9 100644 (file)
--- a/src/cmd/compile/internal/ssa/regalloc.go
+++ b/src/cmd/compile/internal/ssa/regalloc.go
@@ -1699,6 +1699,24 @@ sinking:
                        }
                        p := d.Preds[0].b // block in loop exiting to d.
 
+                       // Check that the spill value is still live at the start of d.
+                       // If it isn't, we can't move the spill here because some other value
+                       // may be using the same stack slot.  See issue 20472.
+                       // The spill value can't be defined in d, so that makes our lives easier.
+                       for _, x := range stacklive[d.ID] {
+                               if x == vsp.ID {
+                                       goto stillLive
+                               }
+                       }
+                       for _, v := range d.Values {
+                               if v.Op == OpLoadReg && v.Args[0] == vsp {
+                                       goto stillLive
+                               }
+                       }
+                       // Spill is not live - abort sinking this spill.
+                       continue sinking
+               stillLive:
+
                        endregs := s.endRegs[p.ID]
                        for _, regrec := range endregs {
                                if regrec.v == e && regrec.r != noRegister && regrec.c == e { // TODO: regrec.c != e implies different spill possible.

diff --git a/src/cmd/compile/internal/ssa/regalloc_test.go b/src/cmd/compile/internal/ssa/regalloc_test.go
index cf8f452d120542ffc0c61a8c04dd33354fc910f3..d3d1891bcda0248dda74619231ec20f67fbfeb54 100644 (file)
--- a/src/cmd/compile/internal/ssa/regalloc_test.go
+++ b/src/cmd/compile/internal/ssa/regalloc_test.go
@@ -31,3 +31,68 @@ func TestLiveControlOps(t *testing.T) {
        regalloc(f.f)
        checkFunc(f.f)
 }
+
+func TestSpillMove(t *testing.T) {
+       // Test for issue 20472. We shouldn't move a spill out to exit blocks
+       // if there is an exit block where the spill is dead but the pre-spill
+       // value is live.
+       c := testConfig(t)
+       ptrType := &TypeImpl{Size_: 8, Ptr: true, Name: "testptr"} // dummy for testing
+       arg1Aux := c.fe.Auto(TypeInt64)
+       arg2Aux := c.fe.Auto(ptrType)
+       f := Fun(c, "entry",
+               Bloc("entry",
+                       Valu("mem", OpInitMem, TypeMem, 0, nil),
+                       Valu("x", OpArg, TypeInt64, 0, arg1Aux),
+                       Valu("p", OpArg, ptrType, 0, arg2Aux),
+                       Valu("a", OpAMD64TESTQ, TypeFlags, 0, nil, "x", "x"),
+                       Goto("loop1"),
+               ),
+               Bloc("loop1",
+                       Valu("y", OpAMD64MULQ, TypeInt64, 0, nil, "x", "x"),
+                       Eq("a", "loop2", "exit1"),
+               ),
+               Bloc("loop2",
+                       Eq("a", "loop1", "exit2"),
+               ),
+               Bloc("exit1",
+                       // store before call, y is available in a register
+                       Valu("mem2", OpAMD64MOVQstore, TypeMem, 0, nil, "p", "y", "mem"),
+                       Valu("mem3", OpAMD64CALLstatic, TypeMem, 0, nil, "mem2"),
+                       Exit("mem3"),
+               ),
+               Bloc("exit2",
+                       // store after call, y must be loaded from a spill location
+                       Valu("mem4", OpAMD64CALLstatic, TypeMem, 0, nil, "mem"),
+                       Valu("mem5", OpAMD64MOVQstore, TypeMem, 0, nil, "p", "y", "mem4"),
+                       Exit("mem5"),
+               ),
+       )
+       flagalloc(f.f)
+       regalloc(f.f)
+       checkFunc(f.f)
+       // There should still be a spill in Loop1, and nowhere else.
+       if numSpills(f.blocks["loop1"]) != 1 {
+               t.Errorf("spill missing from loop1")
+       }
+       if numSpills(f.blocks["loop2"]) != 0 {
+               t.Errorf("spill present in loop2")
+       }
+       if numSpills(f.blocks["exit1"]) != 0 {
+               t.Errorf("spill present in exit1")
+       }
+       if numSpills(f.blocks["exit2"]) != 0 {
+               t.Errorf("spill present in exit2")
+       }
+
+}
+
+func numSpills(b *Block) int {
+       n := 0
+       for _, v := range b.Values {
+               if v.Op == OpStoreReg {
+                       n++
+               }
+       }
+       return n
+}