[dev.ssa] cmd/compile/internal/ssa: eliminate phis during deadcode removal

While investigating the differences between 19710 (remove
tautological controls) and 12960 (bounds and nil propagation)
I observed that part of the wins of 19710 come from missed
opportunities for deadcode elimination due to phis.
See for example runtime.stackcacherelease. 19710 happens much
later than 12960 and has more chances to eliminate bounds.

Size of pkg/tool/linux_amd64/* excluding compile:

-this -12960 95882248
+this -12960 95880120
-this +12960 95581512
+this +12960 95555224

This change saves about 25k.

Change-Id: Id2f4e55fc92b71595842ce493c3ed527d424fe0e
Reviewed-on: https://go-review.googlesource.com/19728
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
TryBot-Result: Gobot Gobot <gobot@golang.org>

1 files changed, 32 insertions, 34 deletions

diff --git a/src/cmd/compile/internal/ssa/deadcode.go b/src/cmd/compile/internal/ssa/deadcode.go
index 87244a6248..a33de438e2 100644
--- a/src/cmd/compile/internal/ssa/deadcode.go
+++ b/src/cmd/compile/internal/ssa/deadcode.go
@@ -234,39 +234,37 @@ func (b *Block) removePred(p *Block) {
 		v.Args[i] = v.Args[n]
 		v.Args[n] = nil // aid GC
 		v.Args = v.Args[:n]
-		if n == 1 {
-			v.Op = OpCopy
-			// Note: this is trickier than it looks.  Replacing
-			// a Phi with a Copy can in general cause problems because
-			// Phi and Copy don't have exactly the same semantics.
-			// Phi arguments always come from a predecessor block,
-			// whereas copies don't.  This matters in loops like:
-			// 1: x = (Phi y)
-			//    y = (Add x 1)
-			//    goto 1
-			// If we replace Phi->Copy, we get
-			// 1: x = (Copy y)
-			//    y = (Add x 1)
-			//    goto 1
-			// (Phi y) refers to the *previous* value of y, whereas
-			// (Copy y) refers to the *current* value of y.
-			// The modified code has a cycle and the scheduler
-			// will barf on it.
-			//
-			// Fortunately, this situation can only happen for dead
-			// code loops.  We know the code we're working with is
-			// not dead, so we're ok.
-			// Proof: If we have a potential bad cycle, we have a
-			// situation like this:
-			//   x = (Phi z)
-			//   y = (op1 x ...)
-			//   z = (op2 y ...)
-			// Where opX are not Phi ops.  But such a situation
-			// implies a cycle in the dominator graph.  In the
-			// example, x.Block dominates y.Block, y.Block dominates
-			// z.Block, and z.Block dominates x.Block (treating
-			// "dominates" as reflexive).  Cycles in the dominator
-			// graph can only happen in an unreachable cycle.
-		}
+		phielimValue(v)
+		// Note: this is trickier than it looks.  Replacing
+		// a Phi with a Copy can in general cause problems because
+		// Phi and Copy don't have exactly the same semantics.
+		// Phi arguments always come from a predecessor block,
+		// whereas copies don't.  This matters in loops like:
+		// 1: x = (Phi y)
+		//    y = (Add x 1)
+		//    goto 1
+		// If we replace Phi->Copy, we get
+		// 1: x = (Copy y)
+		//    y = (Add x 1)
+		//    goto 1
+		// (Phi y) refers to the *previous* value of y, whereas
+		// (Copy y) refers to the *current* value of y.
+		// The modified code has a cycle and the scheduler
+		// will barf on it.
+		//
+		// Fortunately, this situation can only happen for dead
+		// code loops.  We know the code we're working with is
+		// not dead, so we're ok.
+		// Proof: If we have a potential bad cycle, we have a
+		// situation like this:
+		//   x = (Phi z)
+		//   y = (op1 x ...)
+		//   z = (op2 y ...)
+		// Where opX are not Phi ops.  But such a situation
+		// implies a cycle in the dominator graph.  In the
+		// example, x.Block dominates y.Block, y.Block dominates
+		// z.Block, and z.Block dominates x.Block (treating
+		// "dominates" as reflexive).  Cycles in the dominator
+		// graph can only happen in an unreachable cycle.
 	}
 }