Skip to content

Commit 40f2b57

Browse files
Alexandru Moșoibrtzsnr
Alexandru Moșoi
authored andcommitted
[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 <[email protected]> Run-TryBot: Alexandru Moșoi <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent c17b6b4 commit 40f2b57

File tree

2 files changed

+37
-35
lines changed

2 files changed

+37
-35
lines changed

src/cmd/compile/internal/ssa/deadcode.go

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -234,39 +234,37 @@ func (b *Block) removePred(p *Block) {
234234
v.Args[i] = v.Args[n]
235235
v.Args[n] = nil // aid GC
236236
v.Args = v.Args[:n]
237-
if n == 1 {
238-
v.Op = OpCopy
239-
// Note: this is trickier than it looks. Replacing
240-
// a Phi with a Copy can in general cause problems because
241-
// Phi and Copy don't have exactly the same semantics.
242-
// Phi arguments always come from a predecessor block,
243-
// whereas copies don't. This matters in loops like:
244-
// 1: x = (Phi y)
245-
// y = (Add x 1)
246-
// goto 1
247-
// If we replace Phi->Copy, we get
248-
// 1: x = (Copy y)
249-
// y = (Add x 1)
250-
// goto 1
251-
// (Phi y) refers to the *previous* value of y, whereas
252-
// (Copy y) refers to the *current* value of y.
253-
// The modified code has a cycle and the scheduler
254-
// will barf on it.
255-
//
256-
// Fortunately, this situation can only happen for dead
257-
// code loops. We know the code we're working with is
258-
// not dead, so we're ok.
259-
// Proof: If we have a potential bad cycle, we have a
260-
// situation like this:
261-
// x = (Phi z)
262-
// y = (op1 x ...)
263-
// z = (op2 y ...)
264-
// Where opX are not Phi ops. But such a situation
265-
// implies a cycle in the dominator graph. In the
266-
// example, x.Block dominates y.Block, y.Block dominates
267-
// z.Block, and z.Block dominates x.Block (treating
268-
// "dominates" as reflexive). Cycles in the dominator
269-
// graph can only happen in an unreachable cycle.
270-
}
237+
phielimValue(v)
238+
// Note: this is trickier than it looks. Replacing
239+
// a Phi with a Copy can in general cause problems because
240+
// Phi and Copy don't have exactly the same semantics.
241+
// Phi arguments always come from a predecessor block,
242+
// whereas copies don't. This matters in loops like:
243+
// 1: x = (Phi y)
244+
// y = (Add x 1)
245+
// goto 1
246+
// If we replace Phi->Copy, we get
247+
// 1: x = (Copy y)
248+
// y = (Add x 1)
249+
// goto 1
250+
// (Phi y) refers to the *previous* value of y, whereas
251+
// (Copy y) refers to the *current* value of y.
252+
// The modified code has a cycle and the scheduler
253+
// will barf on it.
254+
//
255+
// Fortunately, this situation can only happen for dead
256+
// code loops. We know the code we're working with is
257+
// not dead, so we're ok.
258+
// Proof: If we have a potential bad cycle, we have a
259+
// situation like this:
260+
// x = (Phi z)
261+
// y = (op1 x ...)
262+
// z = (op2 y ...)
263+
// Where opX are not Phi ops. But such a situation
264+
// implies a cycle in the dominator graph. In the
265+
// example, x.Block dominates y.Block, y.Block dominates
266+
// z.Block, and z.Block dominates x.Block (treating
267+
// "dominates" as reflexive). Cycles in the dominator
268+
// graph can only happen in an unreachable cycle.
271269
}
272270
}

src/cmd/compile/internal/ssa/phielim.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ func phielimValue(v *Value) bool {
4040
// are not v itself, then the phi must remain.
4141
// Otherwise, we can replace it with a copy.
4242
var w *Value
43-
for _, x := range v.Args {
43+
for i, x := range v.Args {
44+
if b := v.Block.Preds[i]; b.Kind == BlockFirst && b.Succs[1] == v.Block {
45+
// This branch is never taken so we can just eliminate it.
46+
continue
47+
}
4448
if x == v {
4549
continue
4650
}

0 commit comments

Comments
 (0)