[llvm] r217260 - Set the parent pointer of cloned DBG_VALUE instructions correctly.

[Adrian Prantl]

> On Sep 5, 2014, at 10:31 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> On Sep 5, 2014 10:22 AM, "Adrian Prantl" <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> >
> > Author: adrian
> > Date: Fri Sep  5 12:10:10 2014
> > New Revision: 217260
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=217260&view=rev <http://llvm.org/viewvc/llvm-project?rev=217260&view=rev>
> > Log:
> > Set the parent pointer of cloned DBG_VALUE instructions correctly.
> > Fixes PR20523.
> >
> > When spilling variables onto the stack, spillVirtReg() is setting the
> > parent pointer of the cloned DBG_VALUE intrinsic for the stack location
> > to the parent pointer of the original intrinsic. MachineInstr parent
> > pointers should however always point to the parent basic block.
> >
> > MBB is shadowing the MBB member variable. The instruction still ends up
> > being inserted into the right basic block, because it's inserted after MI
> > which serves as the iterator.
> 
> Perhaps we should assert that the iterator within the basic block?
> 
> 
That makes sense on its own, but it wouldn’t help with catching the bug in the PR. I’ll add this as an additional assertion.
> > I failed at constructing a reliable testcase for this, see
> > http://llvm.org/bugs/show_bug.cgi?id=20523 <http://llvm.org/bugs/show_bug.cgi?id=20523> for a large testcases.
> >
> > Modified:
> >     llvm/trunk/lib/CodeGen/RegAllocFast.cpp
> >
> > Modified: llvm/trunk/lib/CodeGen/RegAllocFast.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocFast.cpp?rev=217260&r1=217259&r2=217260&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocFast.cpp?rev=217260&r1=217259&r2=217260&view=diff>
> > ==============================================================================
> > --- llvm/trunk/lib/CodeGen/RegAllocFast.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/RegAllocFast.cpp Fri Sep  5 12:10:10 2014
> > @@ -309,10 +309,10 @@ void RAFast::spillVirtReg(MachineBasicBl
> >          DL = (--EI)->getDebugLoc();
> >        } else
> >          DL = MI->getDebugLoc();
> > -      MachineBasicBlock *MBB = DBG->getParent();
> >        MachineInstr *NewDV =
> >            BuildMI(*MBB, MI, DL, TII->get(TargetOpcode::DBG_VALUE))
> >                .addFrameIndex(FI).addImm(Offset).addMetadata(MDPtr);
> > +      assert(NewDV->getParent() == MBB && "dangling parent pointer");
> 
> Maybe this assertion (in some form) could be moved into a more central location so this invariant (that BuildMI functions are passed an iterator into the same MBB - but maybe even below the build function, in the insertion itself)
> 
> 

I experimented with the following patch and it seems to lead into a world of pain... Apparently we violate this invariant quite often at the moment.

diff --git a/include/llvm/CodeGen/MachineBasicBlock.h b/include/llvm/CodeGen/MachineBasicBlock.h
index a08cc2e..edd228a 100644
--- a/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/include/llvm/CodeGen/MachineBasicBlock.h
@@ -486,11 +486,13 @@ public:
   /// Insert a range of instructions into the instruction list before I.
   template<typename IT>
   void insert(iterator I, IT S, IT E) {
+    assert(I->getParent() == this && "iterator points outside of basic block");
     Insts.insert(I.getInstrIterator(), S, E);
   }
 
   /// Insert MI into the instruction list before I.
   iterator insert(iterator I, MachineInstr *MI) {
+    assert(I->getParent() == this && "iterator points outside of basic block");
     assert(!MI->isBundledWithPred() && !MI->isBundledWithSucc() &&
            "Cannot insert instruction with bundle flags");
     return Insts.insert(I.getInstrIterator(), MI);
@@ -498,6 +500,7 @@ public:
 
   /// Insert MI into the instruction list after I.
   iterator insertAfter(iterator I, MachineInstr *MI) {
+    assert(I->getParent() == this && "iterator points outside of basic block");
     assert(!MI->isBundledWithPred() && !MI->isBundledWithSucc() &&
            "Cannot insert instruction with bundle flags");
     return Insts.insertAfter(I.getInstrIterator(), MI);



> >        (void)NewDV;
> >        DEBUG(dbgs() << "Inserting debug info due to spill:" << "\n" << *NewDV);
> >      }
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140905/049f2fe0/attachment.html>