diff --git a/go/ssa/lift.go b/go/ssa/lift.go index 00c8a723..048e9b03 100644 --- a/go/ssa/lift.go +++ b/go/ssa/lift.go @@ -214,7 +214,7 @@ func lift(fn *Function) { rename(fn.Blocks[0], renaming, newPhis) // Eliminate dead φ-nodes. - removeDeadPhis(newPhis) + removeDeadPhis(fn.Blocks, newPhis) // Prepend remaining live φ-nodes to each block. for _, b := range fn.Blocks { @@ -269,8 +269,14 @@ func lift(fn *Function) { // removeDeadPhis removes φ-nodes not transitively needed by a // non-Phi, non-DebugRef instruction. -func removeDeadPhis(newPhis newPhiMap) { - // First pass: compute reachability from non-Phi/DebugRef instructions. +func removeDeadPhis(blocks []*BasicBlock, newPhis newPhiMap) { + // First pass: find the set of "live" φ-nodes: those reachable + // from some non-Phi instruction. + // + // We compute reachability in reverse, starting from each φ, + // rather than forwards, starting from each live non-Phi + // instruction, because this way visits much less of the + // Value graph. livePhis := make(map[*Phi]bool) for _, npList := range newPhis { for _, np := range npList { @@ -281,6 +287,14 @@ func removeDeadPhis(newPhis newPhiMap) { } } + // Existing φ-nodes due to && and || operators + // are all considered live (see Go issue 19622). + for _, b := range blocks { + for _, phi := range b.phis() { + markLivePhi(livePhis, phi.(*Phi)) + } + } + // Second pass: eliminate unused phis from newPhis. for block, npList := range newPhis { j := 0 @@ -295,9 +309,7 @@ func removeDeadPhis(newPhis newPhiMap) { *refs = removeInstr(*refs, np.phi) } } - // This may leave DebugRef instructions referring to - // Phis that aren't in the control flow graph. - // TODO(adonovan): we should delete them. + np.phi.block = nil } } newPhis[block] = npList[:j] @@ -318,14 +330,11 @@ func markLivePhi(livePhis map[*Phi]bool, phi *Phi) { } // phiHasDirectReferrer reports whether phi is directly referred to by -// a non-Phi, non-DebugRef instruction. Such instructions are the +// a non-Phi instruction. Such instructions are the // roots of the liveness traversal. func phiHasDirectReferrer(phi *Phi) bool { for _, instr := range *phi.Referrers() { - switch instr.(type) { - case *Phi, *DebugRef: - // ignore - default: + if _, ok := instr.(*Phi); !ok { return true } } diff --git a/go/ssa/print.go b/go/ssa/print.go index 0c1b270b..3333ba41 100644 --- a/go/ssa/print.go +++ b/go/ssa/print.go @@ -89,8 +89,12 @@ func (v *Phi) String() string { b.WriteString(", ") } // Be robust against malformed CFG. + if v.block == nil { + b.WriteString("??") + continue + } block := -1 - if v.block != nil && i < len(v.block.Preds) { + if i < len(v.block.Preds) { block = v.block.Preds[i].Index } fmt.Fprintf(&b, "%d: ", block) diff --git a/go/ssa/sanity.go b/go/ssa/sanity.go index 638a2c42..9e49e3b4 100644 --- a/go/ssa/sanity.go +++ b/go/ssa/sanity.go @@ -351,7 +351,10 @@ func (s *sanity) checkBlock(b *BasicBlock, index int) { // Check that Operands that are also Instructions belong to same function. // TODO(adonovan): also check their block dominates block b. if val, ok := val.(Instruction); ok { - if val.Parent() != s.fn { + if val.Block() == nil { + val.String() + s.errorf("operand %d of %s is an instruction (%s) that belongs to no block", i, instr, val) + } else if val.Parent() != s.fn { s.errorf("operand %d of %s is an instruction (%s) from function %s", i, instr, val, val.Parent()) } }