[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