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

Andrew Trick atrick at apple.com
Thu Sep 25 11:01:55 PDT 2014


> 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.

-Andy





More information about the llvm-commits mailing list