PR24071 Fix an assert in SelectionDAGBuilder

[rovka]

Summary

When processing inline asm that contains errors, make sure we can recover gracefully by creating an UNDEF SDValue for the inline asm statement before returning from SelectionDAGBuilder::visitInlineAsm. This is necessary for consumers that don't exit on the first error that is emitted (e.g. clang) and that would assert later on.

Fixes PR24071.

[rovka]

The test is here: http://reviews.llvm.org/D20157

[t.p.northover]

It seems like it would be better to modify llc as you suggest. Adding an explicitly llc test to Clang is almost certainly the wrong thing to do.

Tim.

[rengolin]

In D20156#427155, @t.p.northover wrote:

It seems like it would be better to modify llc as you suggest. Adding an explicitly llc test to Clang is almost certainly the wrong thing to do.

Hi Tim,

That's a big change though. I agree llc should have diagnostics, too, but this bug shouldn't remain broken until it does, which can take a while and involve a lot of people.

I don't think having the Clang test is wrong, either. Just that it's better if the test is in the same repo and directly tested. I mean, even having an llc test, doesn't stop us from having a Clang one.

cheers,
--renato

[rovka]

In D20156#427155, @t.p.northover wrote:

It seems like it would be better to modify llc as you suggest. Adding an explicitly llc test to Clang is almost certainly the wrong thing to do.

Tim.

Thanks, Tim. I'll try to work on that.

[rnk]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

6792–6796

Can you wrap up most of these calls into a helper like SelectionDAGBuilder::emitInlineAsmError(const Twine &) so that we don't forget to call setValue in the future?

[rovka]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

6792–6796

Good idea. I'll post an updated version of this next week, after we get the new llc diagnostic handler (so I can add the tests too). Thanks.

[rovka]

Addressed previous comments.

Removed the exit-on-error flag on some of the CodeGen tests, that needed it in order to avoid this assert.

[rnk]

lgtm

[rengolin]

Committed in r269811.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

void visitUserOp2(const Instruction &I) {
     llvm_unreachable("UserOp2 should not exist at instruction selection time!");
   }
 
   void processIntegerCallValue(const Instruction &I,
                                SDValue Value, bool IsSigned);
 
   void HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB);
 
+  void emitInlineAsmError(ImmutableCallSite CS, const Twine &Message);
+
   /// EmitFuncArgumentDbgValue - If V is an function argument then create
   /// corresponding DBG_VALUE machine instruction for it now. At the end of
   /// instruction selection, they will be inserted to the entry BB.
   bool EmitFuncArgumentDbgValue(const Value *V, DILocalVariable *Variable,
                                 DIExpression *Expr, DILocation *DL,
                                 int64_t Offset, bool IsIndirect,
                                 const SDValue &N);
 

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

case InlineAsm::isOutput: {
         AsmNodeOperands.push_back(OpInfo.CallOperand);
         break;
       }
 
       // Otherwise, this is a register or register class output.
 
       // Copy the output from the appropriate register.  Find a register that
       // we can use.
       if (OpInfo.AssignedRegs.Regs.empty()) {
-        LLVMContext &Ctx = *DAG.getContext();
-        Ctx.emitError(CS.getInstruction(),
-                      "couldn't allocate output register for constraint '" +
-                          Twine(OpInfo.ConstraintCode) + "'");
+        emitInlineAsmError(
+            CS, "couldn't allocate output register for constraint '" +
         return;
       }
 
       // If this is an indirect operand, store through the pointer after the
       // asm.
       if (OpInfo.isIndirect) {
         IndirectStoresToEmit.push_back(std::make_pair(OpInfo.AssignedRegs,
                                                       OpInfo.CallOperandVal));
       } else {
case InlineAsm::isInput: {
 
         unsigned OpFlag =
           cast<ConstantSDNode>(AsmNodeOperands[CurOp])->getZExtValue();
         if (InlineAsm::isRegDefKind(OpFlag) ||
             InlineAsm::isRegDefEarlyClobberKind(OpFlag)) {
           // Add (OpFlag&0xffff)>>3 registers to MatchedRegs.
           if (OpInfo.isIndirect) {
             // This happens on gcc/testsuite/gcc.dg/pr8788-1.c
-            LLVMContext &Ctx = *DAG.getContext();
-            Ctx.emitError(CS.getInstruction(), "inline asm not supported yet:"
-                                               " don't know how to handle tied "
-                                               "indirect register inputs");
+            emitInlineAsmError(CS, "inline asm not supported yet:"
             return;
           }
 
           RegsForValue MatchedRegs;
           MatchedRegs.ValueVTs.push_back(InOperandVal.getValueType());
           MVT RegVT = AsmNodeOperands[CurOp+1].getSimpleValueType();
           MatchedRegs.RegVTs.push_back(RegVT);
           MachineRegisterInfo &RegInfo = DAG.getMachineFunction().getRegInfo();
           for (unsigned i = 0, e = InlineAsm::getNumOperandRegisters(OpFlag);
                i != e; ++i) {
             if (const TargetRegisterClass *RC = TLI.getRegClassFor(RegVT))
               MatchedRegs.Regs.push_back(RegInfo.createVirtualRegister(RC));
             else {
-              LLVMContext &Ctx = *DAG.getContext();
-              Ctx.emitError(CS.getInstruction(),
-                            "inline asm error: This value"
-                            " type register class is not natively supported!");
+              emitInlineAsmError(
+                  CS, "inline asm error: This value"
               return;
             }
           }
           SDLoc dl = getCurSDLoc();
           // Use the produced MatchedRegs object to
           MatchedRegs.getCopyToRegs(InOperandVal, DAG, dl,
                                     Chain, &Flag, CS.getInstruction());
           MatchedRegs.AddInlineAsmOperands(InlineAsm::Kind_RegUse,
case InlineAsm::isInput: {
           OpInfo.isIndirect)
         OpInfo.ConstraintType = TargetLowering::C_Memory;
 
       if (OpInfo.ConstraintType == TargetLowering::C_Other) {
         std::vector<SDValue> Ops;
         TLI.LowerAsmOperandForConstraint(InOperandVal, OpInfo.ConstraintCode,
                                           Ops, DAG);
         if (Ops.empty()) {
-          LLVMContext &Ctx = *DAG.getContext();
-          Ctx.emitError(CS.getInstruction(),
-                        "invalid operand for inline asm constraint '" +
+          emitInlineAsmError(CS, "invalid operand for inline asm constraint '" +
                                      Twine(OpInfo.ConstraintCode) + "'");
           return;
         }
 
         // Add information to the INLINEASM node to know about this input.
         unsigned ResOpType =
           InlineAsm::getFlagWord(InlineAsm::Kind_Imm, Ops.size());
         AsmNodeOperands.push_back(DAG.getTargetConstant(
             ResOpType, getCurSDLoc(), TLI.getPointerTy(DAG.getDataLayout())));
case InlineAsm::isInput: {
       }
 
       assert((OpInfo.ConstraintType == TargetLowering::C_RegisterClass ||
               OpInfo.ConstraintType == TargetLowering::C_Register) &&
              "Unknown constraint type!");
 
       // TODO: Support this.
       if (OpInfo.isIndirect) {
-        LLVMContext &Ctx = *DAG.getContext();
-        Ctx.emitError(CS.getInstruction(),
-                      "Don't know how to handle indirect register inputs yet "
-                      "for constraint '" +
-                          Twine(OpInfo.ConstraintCode) + "'");
+        emitInlineAsmError(
+            CS, "Don't know how to handle indirect register inputs yet "
         return;
       }
 
       // Copy the input into the appropriate registers.
       if (OpInfo.AssignedRegs.Regs.empty()) {
-        LLVMContext &Ctx = *DAG.getContext();
-        Ctx.emitError(CS.getInstruction(),
-                      "couldn't allocate input reg for constraint '" +
+        emitInlineAsmError(CS, "couldn't allocate input reg for constraint '" +
                                    Twine(OpInfo.ConstraintCode) + "'");
         return;
       }
 
       SDLoc dl = getCurSDLoc();
 
       OpInfo.AssignedRegs.getCopyToRegs(InOperandVal, DAG, dl,
                                         Chain, &Flag, CS.getInstruction());
 
void SelectionDAGBuilder::visitInlineAsm(ImmutableCallSite CS) {
   }
 
   if (!OutChains.empty())
     Chain = DAG.getNode(ISD::TokenFactor, getCurSDLoc(), MVT::Other, OutChains);
 
   DAG.setRoot(Chain);
 }
 
+void SelectionDAGBuilder::emitInlineAsmError(ImmutableCallSite CS,
+                                             const Twine &Message) {
+  LLVMContext &Ctx = *DAG.getContext();
+  Ctx.emitError(CS.getInstruction(), Message);
+
+  // Make sure we leave the DAG in a valid state
+  const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+  auto VT = TLI.getValueType(DAG.getDataLayout(), CS.getType());
+  setValue(CS.getInstruction(), DAG.getUNDEF(VT));
+}
+
 void SelectionDAGBuilder::visitVAStart(const CallInst &I) {
   DAG.setRoot(DAG.getNode(ISD::VASTART, getCurSDLoc(),
                           MVT::Other, getRoot(),
                           getValue(I.getArgOperand(0)),
                           DAG.getSrcValue(I.getArgOperand(0))));
 }
 
 void SelectionDAGBuilder::visitVAArg(const VAArgInst &I) {

test/CodeGen/AArch64/arm64-inline-asm-error-I.ll

-; RUN: not llc -march=arm64 -exit-on-error < %s  2> %t
+; RUN: not llc -march=arm64 < %s  2> %t
 ; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
 
 ; Check for at least one invalid constant.
 ; CHECK-ERRORS: error: invalid operand for inline asm constraint 'I'
 
 define i32 @constraint_I(i32 %i, i32 %j) nounwind ssp {
 entry:
   %0 = tail call i32 asm sideeffect "add $0, $1, $2", "=r,r,I"(i32 %i, i32 4097) nounwind
   ret i32 %0
 }

test/CodeGen/AArch64/arm64-inline-asm-error-J.ll

-; RUN: not llc -march=arm64 -exit-on-error < %s  2> %t
+; RUN: not llc -march=arm64 < %s  2> %t
 ; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
 
 ; Check for at least one invalid constant.
 ; CHECK-ERRORS: error: invalid operand for inline asm constraint 'J'
 
 define i32 @constraint_J(i32 %i, i32 %j) nounwind ssp {
 entry:
   %0 = tail call i32 asm sideeffect "sub $0, $1, $2", "=r,r,J"(i32 %i, i32 2) nounwind
   ret i32 %0
 }

test/CodeGen/AArch64/arm64-inline-asm-error-K.ll

-; RUN: not llc -march=arm64 -exit-on-error < %s  2> %t
+; RUN: not llc -march=arm64 < %s  2> %t
 ; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
 
 ; Check for at least one invalid constant.
 ; CHECK-ERRORS: error: invalid operand for inline asm constraint 'K'
 
 define i32 @constraint_K(i32 %i, i32 %j) nounwind {
 entry:
   %0 = tail call i32 asm sideeffect "eor $0, $1, $2", "=r,r,K"(i32 %i, i32 -1) nounwind
   ret i32 %0
 }

test/CodeGen/AArch64/arm64-inline-asm-error-L.ll

-; RUN: not llc -march=arm64 -exit-on-error < %s  2> %t
+; RUN: not llc -march=arm64 < %s  2> %t
 ; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
 
 ; Check for at least one invalid constant.
 ; CHECK-ERRORS: error: invalid operand for inline asm constraint 'L'
 
 define i32 @constraint_L(i32 %i, i32 %j) nounwind {
 entry:
   %0 = tail call i32 asm sideeffect "eor $0, $1, $2", "=r,r,L"(i32 %i, i64 -1) nounwind
   ret i32 %0
 }

test/CodeGen/AArch64/arm64-inline-asm-error-M.ll

-; RUN: not llc -march=arm64 -exit-on-error < %s  2> %t
+; RUN: not llc -march=arm64 < %s  2> %t
 ; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
 
 ; Check for at least one invalid constant.
 ; CHECK-ERRORS: error: invalid operand for inline asm constraint 'M'
 
 define i32 @constraint_M(i32 %i, i32 %j) nounwind {
 entry:
   %0 = tail call i32 asm sideeffect "movk $0, $1", "=r,M"(i32 305418240) nounwind
   ret i32 %0
 }

test/CodeGen/AArch64/arm64-inline-asm-error-N.ll

-; RUN: not llc -march=arm64 -exit-on-error < %s  2> %t
+; RUN: not llc -march=arm64 < %s  2> %t
 ; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
 
 ; Check for at least one invalid constant.
 ; CHECK-ERRORS: error: invalid operand for inline asm constraint 'N'
 
 define i32 @constraint_N(i32 %i, i32 %j) nounwind {
 entry:
   %0 = tail call i32 asm sideeffect "movk $0, $1", "=r,N"(i64 1311761352401879040) nounwind
   ret i32 %0
 }

test/CodeGen/PowerPC/crbit-asm-disabled.ll

-; RUN: not llc -mcpu=pwr7 -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s
+; RUN: not llc -mcpu=pwr7 -o /dev/null %s 2>&1 | FileCheck %s
 target datalayout = "E-m:e-i64:64-n32:64"
 target triple = "powerpc64-unknown-linux-gnu"
 
 define zeroext i1 @testi1(i1 zeroext %b1, i1 zeroext %b2) #0 {
 entry:
   %0 = tail call i8 asm "crand $0, $1, $2", "=^wc,^wc,^wc"(i1 %b1, i1 %b2) #0
   %1 = and i8 %0, 1
   %tobool3 = icmp ne i8 %1, 0
   ret i1 %tobool3
 
 ; CHECK: error: couldn't allocate output register for constraint 'wc'
 }
 
 attributes #0 = { nounwind "target-features"="-crbits" }
 

test/CodeGen/PowerPC/vec-asm-disabled.ll

-; RUN: not llc -mcpu=pwr7 -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s
+; RUN: not llc -mcpu=pwr7 -o /dev/null %s 2>&1 | FileCheck %s
 target datalayout = "E-m:e-i64:64-n32:64"
 target triple = "powerpc64-unknown-linux-gnu"
 
 define <4 x i32> @testi1(<4 x i32> %b1, <4 x i32> %b2) #0 {
 entry:
   %0 = call <4 x i32> asm "xxland $0, $1, $2", "=^wd,^wd,^wd"(<4 x i32> %b1, <4 x i32> %b2) #0
   ret <4 x i32> %0
 
 ; CHECK: error: couldn't allocate output register for constraint 'wd'
 }
 
 attributes #0 = { nounwind "target-features"="-vsx" }
 

test/CodeGen/X86/asm-reject-reg-type-mismatch.ll

-; RUN: not llc -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s
+; RUN: not llc -o /dev/null %s 2>&1 | FileCheck %s
 target triple = "x86_64--"
 
 ; CHECK: error: couldn't allocate output register for constraint '{ax}'
 define i128 @blup() {
   %v = tail call i128 asm "", "={ax},0"(i128 0)
   ret i128 %v
 }