[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>

diff --git a/src/cmd/compile/internal/ssa/deadcode.go b/src/cmd/compile/internal/ssa/deadcode.go
index 87244a6248818f6f485e05ca8d3ec6f13d054a6a..a33de438e2f760bfa5f96d2806e715e5f48ca40a 100644 (file)
--- 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.
        }
 }

diff --git a/src/cmd/compile/internal/ssa/phielim.go b/src/cmd/compile/internal/ssa/phielim.go
index 20ce5920308505761a71cc20727495ed10645d20..d69449ee21fa1c0e05cf59c19a0fd661650d1c58 100644 (file)
--- a/src/cmd/compile/internal/ssa/phielim.go
+++ b/src/cmd/compile/internal/ssa/phielim.go
@@ -40,7 +40,11 @@ func phielimValue(v *Value) bool {
        // are not v itself, then the phi must remain.
        // Otherwise, we can replace it with a copy.
        var w *Value
-       for _, x := range v.Args {
+       for i, x := range v.Args {
+               if b := v.Block.Preds[i]; b.Kind == BlockFirst && b.Succs[1] == v.Block {
+                       // This branch is never taken so we can just eliminate it.
+                       continue
+               }
                if x == v {
                        continue
                }