From: Alexandru Moșoi <mosoi@google.com>
Date: Wed, 17 Feb 2016 16:21:53 +0000 (+0100)
Subject: [dev.ssa] cmd/compile/internal/ssa: handle phis in fuse.
X-Git-Tag: go1.7beta1~1623^2^2~37
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5949524fc48aa514154cfb939ae28af58aaf6540;p=gostls13.git

[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 2647b841d7..3f81e452b6 100644
--- 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 b6f6b82c35..3ce8ea54b3 100644
--- 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",