cmd/compile: remove loop in shortcircuit

shortcircuitBlock contained a loop to handle blocks like

b: <- p q
  v = Phi true false
If v -> t u

in a single execution.
This change makes shortcircuitBlock do it in two instead,
one for each constant phi arg.

Motivation: Upcoming changes will expand the range of
blocks that the shortcircuit pass can handle.
Those changes need to understand what the CFG
will look like after the rewrite in shortcircuitBlock.
Making shortcircuitBlock do only a single CFG
modification at a time significantly simplifies that code.

In theory, this is less efficient, but not measurably so.
There is minor, unimportant churn in the generated code.

Updates #37608

Change-Id: Ia6dce7011e3e19b546ed1e176bd407575a0ab837
Reviewed-on: https://go-review.googlesource.com/c/go/+/222918
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/cmd/compile/internal/ssa/shortcircuit.go b/src/cmd/compile/internal/ssa/shortcircuit.go
index 42b0639caea9358277aaf3fe939d1280fec3f466..274ef9a1283d05c6744f441a4fb0f79b4a267856 100644 (file)
--- a/src/cmd/compile/internal/ssa/shortcircuit.go
+++ b/src/cmd/compile/internal/ssa/shortcircuit.go
@@ -117,65 +117,63 @@ func shortcircuitBlock(b *Block) bool {
                return false
        }
 
-       // Check for const phi args.
-       var changed bool
-       for i := 0; i < len(ctl.Args); i++ {
-               a := ctl.Args[i]
-               if a.Op != OpConstBool {
-                       continue
+       // Locate index of first const phi arg.
+       cidx := -1
+       for i, a := range ctl.Args {
+               if a.Op == OpConstBool {
+                       cidx = i
+                       break
                }
-               // The predecessor we come in from.
-               e1 := b.Preds[i]
-               p := e1.b
-               pi := e1.i
+       }
+       if cidx == -1 {
+               return false
+       }
 
-               // The successor we always go to when coming in
-               // from that predecessor.
-               si := 1 - a.AuxInt
-               if swap {
-                       si = 1 - si
-               }
-               e2 := b.Succs[si]
-               t := e2.b
-               if p == b || t == b {
-                       // This is an infinite loop; we can't remove it. See issue 33903.
-                       continue
-               }
-               ti := e2.i
+       a := ctl.Args[cidx]
+       // The predecessor we come in from.
+       e1 := b.Preds[cidx]
+       p := e1.b
+       pi := e1.i
 
-               // Update CFG and Phis.
-               changed = true
+       // The successor we always go to when coming in
+       // from that predecessor.
+       si := 1 - a.AuxInt
+       if swap {
+               si = 1 - si
+       }
+       e2 := b.Succs[si]
+       t := e2.b
+       if p == b || t == b {
+               // This is an infinite loop; we can't remove it. See issue 33903.
+               return false
+       }
+       ti := e2.i
 
-               // Remove b's incoming edge from p.
-               b.removePred(i)
-               n := len(b.Preds)
-               ctl.Args[i].Uses--
-               ctl.Args[i] = ctl.Args[n]
-               ctl.Args[n] = nil
-               ctl.Args = ctl.Args[:n]
+       // We're committed. Update CFG and Phis.
 
-               // Redirect p's outgoing edge to t.
-               p.Succs[pi] = Edge{t, len(t.Preds)}
+       // Remove b's incoming edge from p.
+       b.removePred(cidx)
+       n := len(b.Preds)
+       ctl.Args[cidx].Uses--
+       ctl.Args[cidx] = ctl.Args[n]
+       ctl.Args[n] = nil
+       ctl.Args = ctl.Args[:n]
 
-               // Fix up t to have one more predecessor.
-               t.Preds = append(t.Preds, Edge{p, pi})
-               for _, w := range t.Values {
-                       if w.Op != OpPhi {
-                               continue
-                       }
-                       w.AddArg(w.Args[ti])
-               }
-               i--
-       }
+       // Redirect p's outgoing edge to t.
+       p.Succs[pi] = Edge{t, len(t.Preds)}
 
-       if !changed {
-               return false
+       // Fix up t to have one more predecessor.
+       t.Preds = append(t.Preds, Edge{p, pi})
+       for _, w := range t.Values {
+               if w.Op != OpPhi {
+                       continue
+               }
+               w.AddArg(w.Args[ti])
        }
 
        if len(b.Preds) == 0 {
                // Block is now dead.
                b.Kind = BlockInvalid
-               return true
        }
 
        phielimValue(ctl)