[PATCH] SelectionDAG: Remove #if NDEBUG from check for a post-isel hook

Tom Stellard tom at stellard.net
Fri Sep 26 07:28:11 PDT 2014


On Thu, Sep 25, 2014 at 11:01:55AM -0700, Andrew Trick wrote:
> 
> > On Sep 25, 2014, at 8:52 AM, Tom Stellard <thomas.stellard at amd.com> wrote:
> > 
> > The InstrEmitter will skip the check of MI.hasPostISelHook()
> > before calling AdjustInstrPostInstrSelection() when NDEBUG
> > is not defined.
> > 
> > This was added in r140228, and I'm not sure if it is intentional or not,
> > but it is a likely source for bugs, because it means with
> > Release+Asserts builds you can forget to set the hasPostISelHook
> > flag on TableGen definitions and AdjustInstrPostInstrSelection() will
> > still be called.
> > ---
> > lib/CodeGen/SelectionDAG/InstrEmitter.cpp | 2 --
> > 1 file changed, 2 deletions(-)
> > 
> > diff --git a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
> > index 6661aca..4d6de12 100644
> > --- a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
> > +++ b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
> > @@ -865,9 +865,7 @@ EmitMachineNode(SDNode *Node, bool IsClone, bool IsCloned,
> >     MIB->setPhysRegsDeadExcept(UsedRegs, *TRI);
> > 
> >   // Run post-isel target hook to adjust this instruction if needed.
> > -#ifdef NDEBUG
> >   if (II.hasPostISelHook())
> > -#endif
> >     TLI->AdjustInstrPostInstrSelection(MIB, Node);
> > }
> 
> Yes, please remove this. I’ve been meaning to do it myself. The #ifdef was originally there to enable the verification at the top of ARMTargetLowering::AdjustInstrPostInstrSelection. Please just remove that verification too:
> 
> -  if (!MI->hasPostISelHook()) {
> -    assert(!convertAddSubFlagsOpcode(MI->getOpcode()) &&
> -           "Pseudo flag-setting opcodes must be marked with 'hasPostISelHook'");
> -    return;
> -  }
> 
> The correct place to check this is in ARMBaseInstrInfo::verifyInstruction, which is already doing it.
> 
>

r218458-218459

-Tom
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list