[PATCH] D136871: [SelectionDAG] remove stale check assuming INLINEASM_BR is terminator

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers created this revision.
Herald added subscribers: ecnelises, pengfei, hiraditya.
Herald added a project: All.
nickdesaulniers requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

D79794 <https://reviews.llvm.org/D79794> changed INLINEASM_BR from being a termintor to not.

While reading though SelectionDAGBuilder::visitInlineAsm, I noticed a
comment regarding INLINEASM_BR being a terminator, which seemed stale.

Removing the code that has an assumption does change the placement of a
TokenFactor in one test, but does not change the final output. The
assembler generated is ultimately the same.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136871

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


Index: llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
===================================================================
--- llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
+++ llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
@@ -1,22 +1,27 @@
 ; REQUIRES: asserts
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -o /dev/null -debug-only=isel 2>&1 | FileCheck %s
 
-; Make sure we emit the basic block exports and the TokenFactor before the
-; inlineasm_br. Not sure how to get a MachineIR change so this reads the debug
+; Make sure the TokenFactor depends on the basic block exports and the
+; INLINEASM_BR.
+; Not sure how to get a MachineIR change so this reads the debug
 ; output from SelectionDAG.
 
-; CHECK: t0: ch,glue = EntryToken
+; CHECK: Optimized legalized selection DAG: %bb.0 'test:entry'
+; CHECK-NEXT: SelectionDAG has 32 nodes:
+; CHECK-NEXT: t0: ch,glue = EntryToken
+; CHECK-NEXT: t2: i32,ch = CopyFromReg t0, Register:i32 %2
+; CHECK-NEXT: t8: i32 = add t2, Constant:i32<4>
+; CHECK-NEXT: t21: ch,glue = CopyToReg t0, Register:i32 %5, t8
 ; 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
 ; CHECK-NEXT: t6: i32,ch = CopyFromReg t0, Register:i32 %4
 ; CHECK-NEXT: t13: i32 = add t6, Constant:i32<1>
 ; CHECK-NEXT: t15: ch = CopyToReg t0, Register:i32 %1, t13
-; CHECK-NEXT: t17: ch = TokenFactor t12, t15
-; 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, TargetExternalSymbol:i64'xorl $0, $0; jmp ${1:l}', MDNode:ch<null>, TargetConstant:i64<0>, TargetConstant:i32<2359305>, Register:i32 %5, TargetConstant:i64<13>, BasicBlock:ch<fail 0x{{[0-9a-f]+}}>, TargetConstant:i32<12>, Register:i32 $df, TargetConstant:i32<12>, Register:i16 $fpsw, TargetConstant:i32<12>, Register:i32 $eflags, t22:1
+; CHECK-NEXT: t28: ch,glue = inlineasm_br t21, TargetExternalSymbol:i64'xorl $0, $0; jmp ${1:l}', MDNode:ch<null>, TargetConstant:i64<0>, TargetConstant:i32<2359305>, Register:i32 %5, TargetConstant:i64<13>, BasicBlock:ch<fail 0x{{[0-9a-f]+}}>, TargetConstant:i32<12>, Register:i32 $df, TargetConstant:i32<12>, Register:i16 $fpsw, TargetConstant:i32<12>, Register:i32 $eflags, t21:1
+; CHECK-NEXT: t29: ch = TokenFactor t12, t15, t28
+; CHECK-NEXT: t31: ch = br t29, BasicBlock:ch<normal 0x{{[0-9a-f]+}}>
+
 
 define i32 @test(i32 %a, i32 %b, i32 %c) {
 entry:
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -8820,14 +8820,11 @@
   if (EmitEHLabels) {
     assert(EHPadBB && "InvokeInst must have an EHPadBB");
   }
-  bool IsCallBr = isa<CallBrInst>(Call);
 
-  if (IsCallBr || EmitEHLabels) {
-    // If this is a callbr or invoke we need to flush pending exports since
-    // inlineasm_br and invoke are terminators.
-    // We need to do this before nodes are glued to the inlineasm_br node.
+  // If this is an invoke we need to flush pending exports since it is a
+  // terminator.
+  if (EmitEHLabels)
     Chain = getControlRoot();
-  }
 
   MCSymbol *BeginLabel = nullptr;
   if (EmitEHLabels) {
@@ -9197,6 +9194,7 @@
   AsmNodeOperands[InlineAsm::Op_InputChain] = Chain;
   if (Flag.getNode()) AsmNodeOperands.push_back(Flag);
 
+  const bool IsCallBr = isa<CallBrInst>(Call);
   unsigned ISDOpc = IsCallBr ? ISD::INLINEASM_BR : ISD::INLINEASM;
   Chain = DAG.getNode(ISDOpc, getCurSDLoc(),
                       DAG.getVTList(MVT::Other, MVT::Glue), AsmNodeOperands);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136871.471244.patch
Type: text/x-patch
Size: 3789 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221027/ce0c187b/attachment.bin>