[PATCH] D130127: [llvm][TailDuplicator] don't taildup isInlineAsmBrIndirectTargets

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers updated this revision to Diff 457006.
nickdesaulniers marked an inline comment as not done.
nickdesaulniers added a comment.

- fix typo in comment. :set spell


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130127/new/

https://reviews.llvm.org/D130127

Files:
  llvm/lib/CodeGen/TailDuplicator.cpp
  llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/tail-dup-asm-goto.ll


Index: llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
===================================================================
--- llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
+++ llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 ; RUN: llc -mtriple=x86_64-linux -stop-after=early-tailduplication \
-; RUN:   -verify-machineinstrs=0 < %s | FileCheck %s
+; RUN:  -verify-machineinstrs < %s | FileCheck %s
 
 ; Ensure that we don't duplicate a block with an "INLINEASM_BR" instruction
 ; during code gen.
@@ -75,7 +75,6 @@
 ; BOTH the fallthrough/direct target and the indirect target are the same basic
 ; block. We might one day permit tail duplication here, but we need to ensure
 ; that we don't crash or run afoul of any MachineVerifier checks.
-; FIXME: enable -verify-machineinstrs for this test!
 define void @ceph_con_v2_try_read(i32 %__trans_tmp_3.sroa.0.0.copyload, i1 %tobool.not.i.i) nounwind {
   ; CHECK-LABEL: name: ceph_con_v2_try_read
   ; CHECK: bb.0.entry:
@@ -109,11 +108,10 @@
   ; CHECK-NEXT:   JMP_1 %bb.3
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.3.if.else.i.i:
-  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   successors: %bb.5(0x80000000)
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   INLINEASM_BR &"", 1 /* sideeffect attdialect */, 13 /* imm */, %bb.5
-  ; CHECK-NEXT:   LIFETIME_END %stack.0.skip.i.i
-  ; CHECK-NEXT:   JMP_1 %bb.1
+  ; CHECK-NEXT:   JMP_1 %bb.5
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.4.process_message_header.exit.i:
   ; CHECK-NEXT:   successors: %bb.1(0x80000000)
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
@@ -178,15 +178,15 @@
 ; CHECK-NEXT:  # %bb.2: # %asm.fallthrough2
 ; CHECK-NEXT:    addl %edx, %ecx
 ; CHECK-NEXT:    movl %ecx, %eax
+; CHECK-NEXT:  .LBB3_4: # Block address taken
+; CHECK-NEXT:    # %return
+; CHECK-NEXT:    # Label of block must be emitted
 ; CHECK-NEXT:    retl
 ; CHECK-NEXT:  .LBB3_3: # Block address taken
 ; CHECK-NEXT:    # %label_true
 ; CHECK-NEXT:    # Label of block must be emitted
 ; CHECK-NEXT:    movl $-2, %eax
-; CHECK-NEXT:  .LBB3_4: # Block address taken
-; CHECK-NEXT:    # %return
-; CHECK-NEXT:    # Label of block must be emitted
-; CHECK-NEXT:    retl
+; CHECK-NEXT:    jmp .LBB3_4
 entry:
   %0 = callbr { i32, i32 } asm sideeffect "testl $0, $0; testl $1, $2; jne ${3:l}", "=r,=r,r,!i,!i,~{dirflag},~{fpsr},~{flags}"(i32 %out1)
           to label %asm.fallthrough [label %label_true, label %return]
Index: llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
===================================================================
--- llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
+++ llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
@@ -72,7 +72,7 @@
 ; CHECK-LABEL: <test3>:
 ; CHECK-LABEL: <$d.9>:
 ; CHECK-LABEL: <$x.10>:
-; CHECK-NEXT:    b {{.*}} <test3+0x18>
+; CHECK-NEXT:    b {{.*}} <$x.12+0x4>
 ; CHECK-LABEL: <$x.12>:
 ; CHECK-NEXT:    mov w0, wzr
 ; CHECK-NEXT:    ldr x30, [sp], #16
Index: llvm/lib/CodeGen/TailDuplicator.cpp
===================================================================
--- llvm/lib/CodeGen/TailDuplicator.cpp
+++ llvm/lib/CodeGen/TailDuplicator.cpp
@@ -798,6 +798,15 @@
     return false;
   if (!PredCond.empty())
     return false;
+  // FIXME: This is overly conservative; it may be ok to relax this in the
+  // future under more specific conditions. If TailBB is an INLINEASM_BR
+  // indirect target, we need to see if the edge from PredBB to TailBB is from
+  // an INLINEASM_BR in PredBB, and then also if that edge was from the
+  // indirect target list, fallthrough/default target, or potentially both. If
+  // it's both, TailDuplicator::tailDuplicate will remove the edge, corrupting
+  // the successor list in PredBB and predecessor list in TailBB.
+  if (TailBB->isInlineAsmBrIndirectTarget())
+    return false;
   return true;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130127.457006.patch
Type: text/x-patch
Size: 4062 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220831/c1124de6/attachment.bin>