diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -8220,7 +8220,8 @@ argument types must match the types implied by this signature. This type can be omitted if the function is not varargs. #. '``fnptrval``': An LLVM value containing a pointer to a function to - be called. In most cases, this is a direct function call, but + be called. In most cases, this is an + :ref:`Inline Assembler Expression`, but other ``callbr``'s are just as possible, calling an arbitrary pointer to function value. #. '``function args``': argument list whose types match the function @@ -8251,6 +8252,11 @@ assembly where additional labels can be provided as locations for the inline assembly to jump to. +When multiple inputs occur for an +:ref:`Inline Assembler Expression`, the order of inputs must +follow the order: non-destination non-tied inputs, destination ``blockaddress`` +inputs, inputs that are tied to outputs. + Example: """""""" diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -8553,7 +8553,23 @@ unsigned ArgNo = 0; // ArgNo - The argument of the CallInst. unsigned ResNo = 0; // ResNo - The result number of the next output. - unsigned NumMatchingOps = 0; + const auto *CBR = dyn_cast(&Call); + + // Because "inout" args follow the list of labels, we need to bias the index + // calculation to ensure we convert the actual indirect destination + // parameters to TargetBlockAddresses correctly. + unsigned CBRStart, CBREnd; + if (CBR) { + unsigned InOutArgBias = 0; + for (const Use &Arg : llvm::reverse(CBR->args())) { + if (isa(Arg)) + break; + ++InOutArgBias; + } + CBREnd = CBR->arg_size() - InOutArgBias; + CBRStart = CBREnd - CBR->getNumIndirectDests(); + } + for (auto &T : TargetConstraints) { ConstraintOperands.push_back(SDISelAsmOperandInfo(T)); SDISelAsmOperandInfo &OpInfo = ConstraintOperands.back(); @@ -8561,18 +8577,9 @@ // Compute the value type for each operand. if (OpInfo.Type == InlineAsm::isInput || (OpInfo.Type == InlineAsm::isOutput && OpInfo.isIndirect)) { - OpInfo.CallOperandVal = Call.getArgOperand(ArgNo++); - - // Process the call argument. BasicBlocks are labels, currently appearing - // only in asm's. - if (isa(Call) && - ArgNo - 1 >= (cast(&Call)->arg_size() - - cast(&Call)->getNumIndirectDests() - - NumMatchingOps) && - (NumMatchingOps == 0 || - ArgNo - 1 < - (cast(&Call)->arg_size() - NumMatchingOps))) { - const auto *BA = cast(OpInfo.CallOperandVal); + OpInfo.CallOperandVal = Call.getArgOperand(ArgNo); + if (CBR && ArgNo >= CBRStart && ArgNo < CBREnd) { + const auto *BA = dyn_cast(OpInfo.CallOperandVal); EVT VT = TLI.getValueType(DAG.getDataLayout(), BA->getType(), true); OpInfo.CallOperand = DAG.getTargetBlockAddress(BA, VT); } else if (const auto *BB = dyn_cast(OpInfo.CallOperandVal)) { @@ -8584,6 +8591,7 @@ EVT VT = OpInfo.getCallOperandValEVT(*DAG.getContext(), TLI, DAG.getDataLayout()); OpInfo.ConstraintVT = VT.isSimple() ? VT.getSimpleVT() : MVT::Other; + ++ArgNo; } else if (OpInfo.Type == InlineAsm::isOutput && !OpInfo.isIndirect) { // The return value of the call is this value. As such, there is no // corresponding argument. @@ -8601,9 +8609,6 @@ OpInfo.ConstraintVT = MVT::Other; } - if (OpInfo.hasMatchingInput()) - ++NumMatchingOps; - if (!HasSideEffect) HasSideEffect = OpInfo.hasMemory(TLI); diff --git a/llvm/test/CodeGen/X86/callbr-asm-outputs.ll b/llvm/test/CodeGen/X86/callbr-asm-outputs.ll --- a/llvm/test/CodeGen/X86/callbr-asm-outputs.ll +++ b/llvm/test/CodeGen/X86/callbr-asm-outputs.ll @@ -204,3 +204,31 @@ %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"} diff --git a/llvm/test/CodeGen/X86/callbr-asm.ll b/llvm/test/CodeGen/X86/callbr-asm.ll --- a/llvm/test/CodeGen/X86/callbr-asm.ll +++ b/llvm/test/CodeGen/X86/callbr-asm.ll @@ -188,3 +188,39 @@ cleanup: ; preds = %asm.fallthrough, %quux ret void } + +; Test5 - There's an ambiguity when we simultaneously pass the address of a +; label as an input to an inline asm statement, then pass the same label as +; an indirect destination. The same block address appears twice in the arg list +; for the callbr, and they even use the same X constraint. So which one is the +; indirect destination that we could branch to, and which one is just an +; immediate? This is important for two reasons: +; 1. Instruction selection has to decided whether to create a +; TargetBlockAddress or BlockAddress. +; 2. The order of arguments matters for the asm string which may contain +; output templates that specify the order of operands that they are +; referring to. +; Support for outputs can complicate this further, tied operands ("+r" output +; constraint) are boiled down into "=r" with the output re-specified as a +; hidden input. To understand precisely which inputs are actually intended +; indirect destinations, the convention that GotoLabels come after +; InputOperands before hidden tied inputs was added to the Langref in D114895. +define dso_local void @test5() { +; CHECK-LABEL: test5: +; CHECK: # %bb.0: +; CHECK-NEXT: movl $.Ltmp7, %eax +; CHECK-NEXT: #APP +; CHECK-NEXT: # %eax +; CHECK-NEXT: # .Ltmp7 +; CHECK-EMPTY: +; CHECK-NEXT: #NO_APP +; CHECK-NEXT: .Ltmp7: # Block address taken +; CHECK-NEXT: # %bb.1: +; CHECK-NEXT: retl + callbr void asm sideeffect "# $0\0A\09# ${1:l}\0A\09", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@test5, %2), i8* blockaddress(@test5, %2)) + to label %1 [label %2] +1: + br label %2 +2: + ret void +}