Skip to content

Commit 4ad643d

Browse files
josharianjosharian
committed
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 golang#37608 Change-Id: Ia6dce7011e3e19b546ed1e176bd407575a0ab837 Reviewed-on: https://go-review.googlesource.com/c/go/+/222918 Run-TryBot: Josh Bleecher Snyder <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 74bc90a commit 4ad643d

1 file changed

Lines changed: 45 additions & 47 deletions

File tree

‎src/cmd/compile/internal/ssa/shortcircuit.go‎

Lines changed: 45 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -117,65 +117,63 @@ func shortcircuitBlock(b *Block) bool {
117117
return false
118118
}
119119

120-
// Check for const phi args.
121-
var changed bool
122-
for i := 0; i < len(ctl.Args); i++ {
123-
a := ctl.Args[i]
124-
if a.Op != OpConstBool {
125-
continue
120+
// Locate index of first const phi arg.
121+
cidx := -1
122+
for i, a := range ctl.Args {
123+
if a.Op == OpConstBool {
124+
cidx = i
125+
break
126126
}
127-
// The predecessor we come in from.
128-
e1 := b.Preds[i]
129-
p := e1.b
130-
pi := e1.i
127+
}
128+
if cidx == -1 {
129+
return false
130+
}
131131

132-
// The successor we always go to when coming in
133-
// from that predecessor.
134-
si := 1 - a.AuxInt
135-
if swap {
136-
si = 1 - si
137-
}
138-
e2 := b.Succs[si]
139-
t := e2.b
140-
if p == b || t == b {
141-
// This is an infinite loop; we can't remove it. See issue 33903.
142-
continue
143-
}
144-
ti := e2.i
132+
a := ctl.Args[cidx]
133+
// The predecessor we come in from.
134+
e1 := b.Preds[cidx]
135+
p := e1.b
136+
pi := e1.i
145137

146-
// Update CFG and Phis.
147-
changed = true
138+
// The successor we always go to when coming in
139+
// from that predecessor.
140+
si := 1 - a.AuxInt
141+
if swap {
142+
si = 1 - si
143+
}
144+
e2 := b.Succs[si]
145+
t := e2.b
146+
if p == b || t == b {
147+
// This is an infinite loop; we can't remove it. See issue 33903.
148+
return false
149+
}
150+
ti := e2.i
148151

149-
// Remove b's incoming edge from p.
150-
b.removePred(i)
151-
n := len(b.Preds)
152-
ctl.Args[i].Uses--
153-
ctl.Args[i] = ctl.Args[n]
154-
ctl.Args[n] = nil
155-
ctl.Args = ctl.Args[:n]
152+
// We're committed. Update CFG and Phis.
156153

157-
// Redirect p's outgoing edge to t.
158-
p.Succs[pi] = Edge{t, len(t.Preds)}
154+
// Remove b's incoming edge from p.
155+
b.removePred(cidx)
156+
n := len(b.Preds)
157+
ctl.Args[cidx].Uses--
158+
ctl.Args[cidx] = ctl.Args[n]
159+
ctl.Args[n] = nil
160+
ctl.Args = ctl.Args[:n]
159161

160-
// Fix up t to have one more predecessor.
161-
t.Preds = append(t.Preds, Edge{p, pi})
162-
for _, w := range t.Values {
163-
if w.Op != OpPhi {
164-
continue
165-
}
166-
w.AddArg(w.Args[ti])
167-
}
168-
i--
169-
}
162+
// Redirect p's outgoing edge to t.
163+
p.Succs[pi] = Edge{t, len(t.Preds)}
170164

171-
if !changed {
172-
return false
165+
// Fix up t to have one more predecessor.
166+
t.Preds = append(t.Preds, Edge{p, pi})
167+
for _, w := range t.Values {
168+
if w.Op != OpPhi {
169+
continue
170+
}
171+
w.AddArg(w.Args[ti])
173172
}
174173

175174
if len(b.Preds) == 0 {
176175
// Block is now dead.
177176
b.Kind = BlockInvalid
178-
return true
179177
}
180178

181179
phielimValue(ctl)

0 commit comments

Comments
 (0)