[SelectionDAG] treat X constrained labels as i for asm

Completely rework how we handle X constrained labels for inline asm.

X should really be treated as i. Then existing tests can be moved to use
i D115410 and clang can just emit i D115311. (D115410 and D115311 are
callbr, but this can be done for label inputs, too).

Coincidentally, this simplification solves an ICE uncovered by D87279
based on assumptions made during D69868.

This is the third approach considered. See also discussions v1 (D114895)
and v2 (D115409).

Reported-by: kernel test robot <lkp@intel.com>
Fixes: ClangBuiltLinux/linux#1512

Reviewed By: void, jyknight

Differential Revision: https://reviews.llvm.org/D115688

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

@@ -1683,6 +1683,8 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) {
   if (const MetadataAsValue *MD = dyn_cast<MetadataAsValue>(V)) {
     return DAG.getMDNode(cast<MDNode>(MD->getMetadata()));
   }
+  if (const auto *BB = dyn_cast<BasicBlock>(V))
+    return DAG.getBasicBlock(FuncInfo.MBBMap[BB]);
   llvm_unreachable("Can't get register for value!");
 }
 
@@ -8558,32 +8560,14 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
 
   unsigned ArgNo = 0;   // ArgNo - The argument of the CallInst.
   unsigned ResNo = 0;   // ResNo - The result number of the next output.
-  unsigned NumMatchingOps = 0;
   for (auto &T : TargetConstraints) {
     ConstraintOperands.push_back(SDISelAsmOperandInfo(T));
     SDISelAsmOperandInfo &OpInfo = ConstraintOperands.back();
 
     // Compute the value type for each operand.
     if (OpInfo.hasArg()) {
       OpInfo.CallOperandVal = Call.getArgOperand(ArgNo);
-
-      // Process the call argument. BasicBlocks are labels, currently appearing
-      // only in asm's.
-      if (isa<CallBrInst>(Call) &&
-          ArgNo >= (cast<CallBrInst>(&Call)->arg_size() -
-                        cast<CallBrInst>(&Call)->getNumIndirectDests() -
-                        NumMatchingOps) &&
-          (NumMatchingOps == 0 ||
-           ArgNo < (cast<CallBrInst>(&Call)->arg_size() - NumMatchingOps))) {
-        const auto *BA = cast<BlockAddress>(OpInfo.CallOperandVal);
-        EVT VT = TLI.getValueType(DAG.getDataLayout(), BA->getType(), true);
-        OpInfo.CallOperand = DAG.getTargetBlockAddress(BA, VT);
-      } else if (const auto *BB = dyn_cast<BasicBlock>(OpInfo.CallOperandVal)) {
-        OpInfo.CallOperand = DAG.getBasicBlock(FuncInfo.MBBMap[BB]);
-      } else {
-        OpInfo.CallOperand = getValue(OpInfo.CallOperandVal);
-      }
-
+      OpInfo.CallOperand = getValue(OpInfo.CallOperandVal);
       Type *ParamElemTy = Call.getAttributes().getParamElementType(ArgNo);
       EVT VT = OpInfo.getCallOperandValEVT(*DAG.getContext(), TLI,
                                            DAG.getDataLayout(), ParamElemTy);
@@ -8606,9 +8590,6 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
       OpInfo.ConstraintVT = MVT::Other;
     }
 
-    if (OpInfo.hasMatchingInput())
-      ++NumMatchingOps;
-
     if (!HasSideEffect)
       HasSideEffect = OpInfo.hasMemory(TLI);
 

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

@@ -4592,13 +4592,7 @@ void TargetLowering::LowerAsmOperandForConstraint(SDValue Op,
   char ConstraintLetter = Constraint[0];
   switch (ConstraintLetter) {
   default: break;
-  case 'X':     // Allows any operand; labels (basic block) use this.
-    if (Op.getOpcode() == ISD::BasicBlock ||
-        Op.getOpcode() == ISD::TargetBlockAddress) {
-      Ops.push_back(Op);
-      return;
-    }
-    LLVM_FALLTHROUGH;
+  case 'X':    // Allows any operand
   case 'i':    // Simple Integer or Relocatable Constant
   case 'n':    // Simple Integer
   case 's': {  // Relocatable Constant
@@ -4639,6 +4633,10 @@ void TargetLowering::LowerAsmOperandForConstraint(SDValue Op,
               Offset + BA->getOffset(), BA->getTargetFlags()));
           return;
         }
+        if (isa<BasicBlockSDNode>(Op)) {
+          Ops.push_back(Op);
+          return;
+        }
       }
       const unsigned OpCode = Op.getOpcode();
       if (OpCode == ISD::ADD || OpCode == ISD::SUB) {
@@ -5085,17 +5083,18 @@ void TargetLowering::ComputeConstraintToUse(AsmOperandInfo &OpInfo,
 
   // 'X' matches anything.
   if (OpInfo.ConstraintCode == "X" && OpInfo.CallOperandVal) {
-    // Labels and constants are handled elsewhere ('X' is the only thing
-    // that matches labels).  For Functions, the type here is the type of
-    // the result, which is not what we want to look at; leave them alone.
+    // Constants are handled elsewhere.  For Functions, the type here is the
+    // type of the result, which is not what we want to look at; leave them
+    // alone.
     Value *v = OpInfo.CallOperandVal;
-    if (isa<BasicBlock>(v) || isa<ConstantInt>(v) || isa<Function>(v)) {
-      OpInfo.CallOperandVal = v;
+    if (isa<ConstantInt>(v) || isa<Function>(v)) {
       return;
     }
 
-    if (Op.getNode() && Op.getOpcode() == ISD::TargetBlockAddress)
+    if (isa<BasicBlock>(v) || isa<BlockAddress>(v)) {
+      OpInfo.ConstraintCode = "i";
       return;
+    }
 
     // Otherwise, try to resolve it to something we know about by looking at
     // the actual operand type.

llvm/test/CodeGen/AArch64/inlineasm-X-constraint.ll

@@ -125,17 +125,11 @@ entry:
 ;  A:
 ;   return;
 ; }
-;
-; Ideally this would give the block address of bb, but it requires us to see
-; through blockaddress, which we can't do at the moment. This might break some
-; existing use cases where a user would expect to get a block label and instead
-; gets the block address in a register. However, note that according to the
-; "no constraints" definition this behaviour is correct (although not very nice).
 
 ; CHECK-LABEL: f7
-; CHECK: bl
+; CHECK: bl .Ltmp3
 define void @f7() {
-  call void asm sideeffect "br $0", "X"( i8* blockaddress(@f7, %bb) )
+  call void asm sideeffect "bl $0", "X"( i8* blockaddress(@f7, %bb) )
   br label %bb
 bb:
   ret void

llvm/test/CodeGen/X86/asm-block-labels2.ll

@@ -0,0 +1,22 @@
+; RUN: not llc -mtriple=x86_64-linux-gnu -o - %s 2>&1 | FileCheck %s
+
+; Test that the blockaddress with X, i, or s constraint is printed as an
+; immediate (.Ltmp0).
+; Test that blockaddress with n constraint is an error.
+define void @test1() {
+; CHECK: error: constraint 'n' expects an integer constant expression
+; CHECK-LABEL: test1:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:  .Ltmp0: # Block address taken
+; CHECK-NEXT:  # %bb.1: # %b
+; CHECK-NEXT:    #APP
+; CHECK-NEXT:    # .Ltmp0 .Ltmp0 .Ltmp0
+; CHECK-NEXT:    #NO_APP
+; CHECK-NEXT:    retq
+entry:
+  br label %b
+b:
+  call void asm "# $0 $1 $2", "X,i,s"(i8* blockaddress(@test1, %b), i8* blockaddress(@test1, %b), i8* blockaddress(@test1, %b))
+  call void asm "# $0", "n"(i8* blockaddress(@test1, %b))
+  ret void
+}

llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll

@@ -6,6 +6,7 @@
 ; output from SelectionDAG.
 
 ; CHECK: t0: ch = EntryToken
+; CHECK-NEXT: t16: i64 = BlockAddress<@test, %fail> 0
 ; CHECK-NEXT: t4: i32,ch = CopyFromReg t0, Register:i32 %3
 ; CHECK-NEXT: t10: i32 = add t4, Constant:i32<1>
 ; CHECK-NEXT: t12: ch = CopyToReg t0, Register:i32 %0, t10
@@ -16,7 +17,7 @@
 ; CHECK-NEXT: t2: i32,ch = CopyFromReg t0, Register:i32 %2
 ; CHECK-NEXT: t8: i32 = add t2, Constant:i32<4>
 ; CHECK-NEXT: t22: ch,glue = CopyToReg t17, Register:i32 %5, t8
-; CHECK-NEXT: t29: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t30: ch,glue = inlineasm_br t22, TargetExternalSymbol:i64'xorl $0, $0; jmp ${1:l}', MDNode:ch<null>, TargetConstant:i64<8>, TargetConstant:i32<2293769>, Register:i32 %5, TargetConstant:i64<13>, TargetBlockAddress:i64<@test, %fail> 0, TargetConstant:i32<12>, Register:i32 $df, TargetConstant:i32<12>, Register:i16 $fpsw, TargetConstant:i32<12>, Register:i32 $eflags, t22:1
 
 define i32 @test(i32 %a, i32 %b, i32 %c) {
 entry:

llvm/test/CodeGen/X86/callbr-asm-outputs.ll

@@ -204,3 +204,31 @@ return:                                           ; preds = %entry, %asm.fallthr
   %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
   ret i32 %retval.0
 }
+
+; Test5 - test that we don't rely on a positional constraint. ie. +r in
+; GCCAsmStmt being turned into "={esp},0" since after D87279 they're turned
+; into "={esp},{esp}". This previously caused an ICE; this test is more so
+; about avoiding another ICE than what the actual output is.
+define dso_local void @test5() {
+; CHECK-LABEL: test5:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    #APP
+; CHECK-NEXT:    #NO_APP
+; CHECK-NEXT:  .Ltmp6: # Block address taken
+; CHECK-NEXT:  # %bb.1:
+; CHECK-NEXT:    retl
+  %1 = call i32 @llvm.read_register.i32(metadata !3)
+  %2 = callbr i32 asm "", "={esp},X,{esp},~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@test5, %4), i32 %1)
+          to label %3 [label %4]
+
+3:
+  call void @llvm.write_register.i32(metadata !3, i32 %2)
+  br label %4
+
+4:
+  ret void
+}
+
+declare i32 @llvm.read_register.i32(metadata)
+declare void @llvm.write_register.i32(metadata, i32)
+!3 = !{!"esp"}