From 661e836afb5bf18826a30bbb4a0217d3d7818de8 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Thu, 10 Jul 2014 06:27:25 -0400 Subject: [PATCH] go.tools/go/ssa: introduce a sanity check for dead referrers This extends the sanity checker to identify and report referrers which do not appear in the function's instruction lists, and fixes two bugs in the lifting algorithm which were caught by the sanity check. LGTM=adonovan R=adonovan CC=axwalk, golang-codereviews https://golang.org/cl/110210045 --- go/ssa/lift.go | 27 ++++++++++++++++++++++++++- go/ssa/sanity.go | 27 ++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/go/ssa/lift.go b/go/ssa/lift.go index f285f2d5..1f190fe4 100644 --- a/go/ssa/lift.go +++ b/go/ssa/lift.go @@ -108,6 +108,21 @@ func buildDomFrontier(fn *Function) domFrontier { return df } +func removeInstr(refs []Instruction, instr Instruction) []Instruction { + i := 0 + for _, ref := range refs { + if ref == instr { + continue + } + refs[i] = ref + i++ + } + for j := i; j != len(refs); j++ { + refs[j] = nil // aid GC + } + return refs[:i] +} + // lift attempts to replace local and new Allocs accessed only with // load/store by SSA registers, inserting φ-nodes where necessary. // The result is a program in classical pruned SSA form. @@ -208,7 +223,13 @@ func lift(fn *Function) { j := 0 for _, np := range nps { if !phiIsLive(np.phi) { - continue // discard it + // discard it, first removing it from referrers + for _, newval := range np.phi.Edges { + if refs := newval.Referrers(); refs != nil { + *refs = removeInstr(*refs, np.phi) + } + } + continue } nps[j] = np j++ @@ -495,6 +516,10 @@ func rename(u *BasicBlock, renaming []Value, newPhis newPhiMap) { fmt.Fprintf(os.Stderr, "\tkill store %s; new value: %s\n", instr, instr.Val.Name()) } + // Remove the store from the referrer list of the stored value. + if refs := instr.Val.Referrers(); refs != nil { + *refs = removeInstr(*refs, instr) + } // Delete the Store. u.Instrs[i] = nil u.gaps++ diff --git a/go/ssa/sanity.go b/go/ssa/sanity.go index 33b5b109..69d80e27 100644 --- a/go/ssa/sanity.go +++ b/go/ssa/sanity.go @@ -20,6 +20,7 @@ type sanity struct { reporter io.Writer fn *Function block *BasicBlock + instrs map[Instruction]struct{} insane bool } @@ -185,7 +186,8 @@ func (s *sanity) checkInstr(idx int, instr Instruction) { } } - // Check that value-defining instructions have valid types. + // Check that value-defining instructions have valid types + // and a valid referrer list. if v, ok := instr.(Value); ok { t := v.Type() if t == nil { @@ -195,6 +197,7 @@ func (s *sanity) checkInstr(idx int, instr Instruction) { } else if b, ok := t.Underlying().(*types.Basic); ok && b.Info()&types.IsUntyped != 0 { s.errorf("instruction has 'untyped' result: %s = %s : %s", v.Name(), v, t) } + s.checkReferrerList(v) } // Untyped constants are legal as instruction Operands(), @@ -381,6 +384,19 @@ func (s *sanity) checkBlock(b *BasicBlock, index int) { } } +func (s *sanity) checkReferrerList(v Value) { + refs := v.Referrers() + if refs == nil { + s.errorf("%s has missing referrer list", v.Name()) + return + } + for i, ref := range *refs { + if _, ok := s.instrs[ref]; !ok { + s.errorf("%s.Referrers()[%d] = %s is not an instruction belonging to this function", v.Name(), i, ref) + } + } +} + func (s *sanity) checkFunction(fn *Function) bool { // TODO(adonovan): check Function invariants: // - check params match signature @@ -418,15 +434,24 @@ func (s *sanity) checkFunction(fn *Function) bool { s.errorf("Local %s at index %d has Heap flag set", l.Name(), i) } } + // Build the set of valid referrers. + s.instrs = make(map[Instruction]struct{}) + for _, b := range fn.Blocks { + for _, instr := range b.Instrs { + s.instrs[instr] = struct{}{} + } + } for i, p := range fn.Params { if p.Parent() != fn { s.errorf("Param %s at index %d has wrong parent", p.Name(), i) } + s.checkReferrerList(p) } for i, fv := range fn.FreeVars { if fv.Parent() != fn { s.errorf("FreeVar %s at index %d has wrong parent", fv.Name(), i) } + s.checkReferrerList(fv) } if fn.Blocks != nil && len(fn.Blocks) == 0 {