[dev.ssa] cmd/compile/internal/ssa: handle phis in fuse.

Change-Id: Idd880cc6c1e5dc34dddbdea0841a7a718d2fa836
Reviewed-on: https://go-review.googlesource.com/19544
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/cmd/compile/internal/ssa/fuse.go b/src/cmd/compile/internal/ssa/fuse.go
index 2647b841d7b90dcd60f00e1bf6fdc3140a487e4f..3f81e452b656ceee41766694e605714db5dd723e 100644 (file)
--- a/src/cmd/compile/internal/ssa/fuse.go
+++ b/src/cmd/compile/internal/ssa/fuse.go
@@ -17,15 +17,15 @@ func fuse(f *Func) {
 
 // fuseBlockIf handles the following cases where s0 and s1 are empty blocks.
 //
-//   b       b          b
-//  / \      | \      / |
-// s0  s1    |  s1   s0 |
-//  \ /      | /      \ |
-//   ss      ss        ss
+//   b        b        b      b
+//  / \      | \      / |    | |
+// s0  s1    |  s1   s0 |    | |
+//  \ /      | /      \ |    | |
+//   ss      ss        ss     ss
 //
-// If ss doesn't contain any Phi ops and s0 & s1 are empty then the branch
-// can be dropped.
-// TODO: If ss doesn't contain any Phi ops, are s0 and s1 dead code anyway?
+// If all Phi ops in ss have identical variables for slots corresponding to
+// s0, s1 and b then the branch can be dropped.
+// TODO: If ss doesn't contain any OpPhis, are s0 and s1 dead code anyway.
 func fuseBlockIf(b *Block) bool {
        if b.Kind != BlockIf {
                return false
@@ -34,13 +34,13 @@ func fuseBlockIf(b *Block) bool {
        var ss0, ss1 *Block
        s0 := b.Succs[0]
        if s0.Kind != BlockPlain || len(s0.Preds) != 1 || len(s0.Values) != 0 {
-               s0, ss0 = nil, s0
+               s0, ss0 = b, s0
        } else {
                ss0 = s0.Succs[0]
        }
        s1 := b.Succs[1]
        if s1.Kind != BlockPlain || len(s1.Preds) != 1 || len(s1.Values) != 0 {
-               s1, ss1 = nil, s1
+               s1, ss1 = b, s1
        } else {
                ss1 = s1.Succs[0]
        }
@@ -50,41 +50,63 @@ func fuseBlockIf(b *Block) bool {
        }
        ss := ss0
 
-       // TODO: Handle OpPhi operations. We can still replace OpPhi if the
-       // slots corresponding to b, s0 and s1 point to the same variable.
+       // s0 and s1 are equal with b if the corresponding block is missing
+       // (2nd, 3rd and 4th case in the figure).
+       i0, i1 := -1, -1
+       for i, p := range ss.Preds {
+               if p == s0 {
+                       i0 = i
+               }
+               if p == s1 {
+                       i1 = i
+               }
+       }
+       if i0 == -1 || i1 == -1 {
+               b.Fatalf("invalid predecessors")
+       }
        for _, v := range ss.Values {
-               if v.Op == OpPhi {
+               if v.Op == OpPhi && v.Args[i0] != v.Args[i1] {
                        return false
                }
        }
 
-       // Now we have two following b->ss, b->s0->ss and b->s1->ss,
+       // Now we have two of following b->ss, b->s0->ss and b->s1->ss,
        // with s0 and s1 empty if exist.
-       // We can replace it with b->ss without if ss has no phis
-       // which is checked above.
+       // We can replace it with b->ss without if all OpPhis in ss
+       // have identical predecessors (verified above).
        // No critical edge is introduced because b will have one successor.
-       if s0 != nil {
+       if s0 != b && s1 != b {
                ss.removePred(s0)
-       }
-       if s1 != nil {
+
+               // Replace edge b->s1->ss with b->ss.
+               // We need to keep a slot for Phis corresponding to b.
+               for i := range b.Succs {
+                       if b.Succs[i] == s1 {
+                               b.Succs[i] = ss
+                       }
+               }
+               for i := range ss.Preds {
+                       if ss.Preds[i] == s1 {
+                               ss.Preds[i] = b
+                       }
+               }
+       } else if s0 != b {
+               ss.removePred(s0)
+       } else if s1 != b {
                ss.removePred(s1)
        }
-       if s0 != nil && s1 != nil {
-               // Add an edge if both edges are removed, otherwise b is no longer connected to ss.
-               ss.Preds = append(ss.Preds, b)
-       }
        b.Kind = BlockPlain
        b.Control = nil
        b.Succs = append(b.Succs[:0], ss)
 
        // Trash the empty blocks s0 & s1.
-       if s0 != nil {
+       if s0 != b {
                s0.Kind = BlockInvalid
                s0.Values = nil
                s0.Succs = nil
                s0.Preds = nil
        }
-       if s1 != nil {
+       if s1 != b {
                s1.Kind = BlockInvalid
                s1.Values = nil
                s1.Succs = nil

diff --git a/src/cmd/compile/internal/ssa/fuse_test.go b/src/cmd/compile/internal/ssa/fuse_test.go
index b6f6b82c3556105602eab6cd13572ec4fc520e52..3ce8ea54b3915a2da2f824282b24f638a3f6ea13 100644 (file)
--- a/src/cmd/compile/internal/ssa/fuse_test.go
+++ b/src/cmd/compile/internal/ssa/fuse_test.go
@@ -65,6 +65,40 @@ func TestFuseEliminatesBothBranches(t *testing.T) {
        }
 }
 
+func TestFuseHandlesPhis(t *testing.T) {
+       ptrType := &TypeImpl{Size_: 8, Ptr: true, Name: "testptr"} // dummy for testing
+       c := NewConfig("amd64", DummyFrontend{t}, nil, true)
+       fun := Fun(c, "entry",
+               Bloc("entry",
+                       Valu("mem", OpInitMem, TypeMem, 0, nil),
+                       Valu("sb", OpSB, TypeInvalid, 0, nil),
+                       Goto("checkPtr")),
+               Bloc("checkPtr",
+                       Valu("ptr1", OpLoad, ptrType, 0, nil, "sb", "mem"),
+                       Valu("nilptr", OpConstNil, ptrType, 0, nil, "sb"),
+                       Valu("bool1", OpNeqPtr, TypeBool, 0, nil, "ptr1", "nilptr"),
+                       If("bool1", "then", "else")),
+               Bloc("then",
+                       Goto("exit")),
+               Bloc("else",
+                       Goto("exit")),
+               Bloc("exit",
+                       Valu("phi", OpPhi, ptrType, 0, nil, "ptr1", "ptr1"),
+                       Exit("mem")))
+
+       CheckFunc(fun.f)
+       fuse(fun.f)
+
+       for _, b := range fun.f.Blocks {
+               if b == fun.blocks["then"] && b.Kind != BlockInvalid {
+                       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")
+               }
+       }
+}
+
 func TestFuseEliminatesEmptyBlocks(t *testing.T) {
        c := NewConfig("amd64", DummyFrontend{t}, nil, true)
        fun := Fun(c, "entry",