[PATCH] D114895: [SelectionDagBuilder] improve CallBrInst BlockAddress constraint handling

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: void, jyknight, efriedma, craig.topper.
Herald added subscribers: pengfei, hiraditya.
nickdesaulniers requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

D87279 <https://reviews.llvm.org/D87279> exposed the existing reliance on tied operands for CallBr to
process its constraints for BlockAddress parameters was brittle; a case
of using asm goto with a single output with the +r constraint caused an
ICE.

The code originally was very careful to only consider arguments to the
asm which were the indirect targets of the callbr (see the careful
arithmetic around ArgNo removed in this patch).

This became brittle when adding support for "asm goto with outputs"
(D69868 <https://reviews.llvm.org/D69868>); there's an ambiguity where we know the number of arguments to
the CallBrInst, and the number of indirect destinations, but we don't
easily know which arguments are the indirect destinations and the
addition of outputs makes it more ambiguous whether an argument is an
output, indirect destination, or input.

But that doesn't really matter; CallBrInst can have a BlockAddress
parameter that's not necessarily in the formal indirect destination
list; so just process all BlockAddress parameters to a CallBrInst the
same way.

Reported-by: kernel test robot <lkp at intel.com>
Fixes: https://github.com/ClangBuiltLinux/linux/issues/1512


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114895

Files:
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll


Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===================================================================
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -204,3 +204,27 @@
   %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
   ret i32 %retval.0
 }
+
+define dso_local void @vmcs_set_bits() {
+; CHECK-LABEL: vmcs_set_bits:
+; 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(@vmcs_set_bits, %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"}
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -8557,7 +8557,6 @@
 
   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,17 +8565,8 @@
     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<CallBrInst>(Call) &&
-          ArgNo - 1 >= (cast<CallBrInst>(&Call)->arg_size() -
-                        cast<CallBrInst>(&Call)->getNumIndirectDests() -
-                        NumMatchingOps) &&
-          (NumMatchingOps == 0 ||
-           ArgNo - 1 <
-               (cast<CallBrInst>(&Call)->arg_size() - NumMatchingOps))) {
-        const auto *BA = cast<BlockAddress>(OpInfo.CallOperandVal);
+      const auto *BA = dyn_cast<BlockAddress>(OpInfo.CallOperandVal);
+      if (BA && isa<CallBrInst>(Call)) {
         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)) {
@@ -8605,9 +8595,6 @@
       OpInfo.ConstraintVT = MVT::Other;
     }
 
-    if (OpInfo.hasMatchingInput())
-      ++NumMatchingOps;
-
     if (!HasSideEffect)
       HasSideEffect = OpInfo.hasMemory(TLI);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114895.391094.patch
Type: text/x-patch
Size: 2919 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211201/d181eea1/attachment.bin>