[llvm] 4edb998 - [SelectionDAG] treat X constrained labels as i for asm
Nick Desaulniers via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 11 10:30:01 PST 2022
Author: Nick Desaulniers
Date: 2022-01-11T10:29:40-08:00
New Revision: 4edb9983cb8c8b850083ee5941468f5d0ef6fe2c
URL: https://github.com/llvm/llvm-project/commit/4edb9983cb8c8b850083ee5941468f5d0ef6fe2c
DIFF: https://github.com/llvm/llvm-project/commit/4edb9983cb8c8b850083ee5941468f5d0ef6fe2c.diff
LOG: [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 at intel.com>
Fixes: https://github.com/ClangBuiltLinux/linux/issues/1512
Reviewed By: void, jyknight
Differential Revision: https://reviews.llvm.org/D115688
Added:
llvm/test/CodeGen/X86/asm-block-labels2.ll
Modified:
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/test/CodeGen/AArch64/inlineasm-X-constraint.ll
llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
llvm/test/CodeGen/X86/callbr-asm-outputs.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 9674a88bae95..161d7fa87612 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/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,7 +8560,6 @@ 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();
@@ -8566,24 +8567,7 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
// 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);
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index e4bc861f29b6..40b422fd740d 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/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.
diff --git a/llvm/test/CodeGen/AArch64/inlineasm-X-constraint.ll b/llvm/test/CodeGen/AArch64/inlineasm-X-constraint.ll
index 4a226e6d5828..c01f4a646fdc 100644
--- a/llvm/test/CodeGen/AArch64/inlineasm-X-constraint.ll
+++ b/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
diff --git a/llvm/test/CodeGen/X86/asm-block-labels2.ll b/llvm/test/CodeGen/X86/asm-block-labels2.ll
new file mode 100644
index 000000000000..25a64f12fa5f
--- /dev/null
+++ b/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
+}
diff --git a/llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll b/llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
index 8643db4f8959..78b5c7b28222 100644
--- a/llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
+++ b/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:
diff --git a/llvm/test/CodeGen/X86/callbr-asm-outputs.ll b/llvm/test/CodeGen/X86/callbr-asm-outputs.ll
index a4447bc15f11..277d8e9b15aa 100644
--- a/llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ b/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"}
More information about the llvm-commits
mailing list