[llvm] d7474be - [llvm][TailDuplicator] don't taildup isInlineAsmBrIndirectTargets

[Nick Desaulniers via llvm-commits]

Author: Nick Desaulniers
Date: 2022-08-31T13:07:10-07:00
New Revision: d7474bef779ab9848d48e2a0625778b78eac6fee

URL: https://github.com/llvm/llvm-project/commit/d7474bef779ab9848d48e2a0625778b78eac6fee
DIFF: https://github.com/llvm/llvm-project/commit/d7474bef779ab9848d48e2a0625778b78eac6fee.diff

LOG: [llvm][TailDuplicator] don't taildup isInlineAsmBrIndirectTargets

This fixes a crash observed after
https://reviews.llvm.org/D129997.

Similar to D88823.

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D130127

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/TailDuplicator.cpp b/llvm/lib/CodeGen/TailDuplicator.cpp
index 461971ac54a9..86ba57d09c2a 100644
--- a/llvm/lib/CodeGen/TailDuplicator.cpp
+++ b/llvm/lib/CodeGen/TailDuplicator.cpp
@@ -798,6 +798,15 @@ bool TailDuplicator::canTailDuplicate(MachineBasicBlock *TailBB,
     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;
 }
 

diff  --git a/llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll b/llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
index 5105c34513a9..06d956831539 100644
--- a/llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
+++ b/llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
@@ -72,7 +72,7 @@ define hidden i32 @test2() local_unnamed_addr {
 ; 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

diff  --git a/llvm/test/CodeGen/X86/callbr-asm-outputs.ll b/llvm/test/CodeGen/X86/callbr-asm-outputs.ll
index 77b7b3362cc0..c11308273193 100644
--- a/llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ b/llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -178,15 +178,15 @@ define i32 @test4(i32 %out1, i32 %out2) {
 ; 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]

diff  --git a/llvm/test/CodeGen/X86/tail-dup-asm-goto.ll b/llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
index da8e0b6c823a..e1f8323cf206 100644
--- a/llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
+++ b/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 @@ kmem_cache_has_cpu_partial.exit:                  ; preds = %bb110
 ; 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 @@ define void @ceph_con_v2_try_read(i32 %__trans_tmp_3.sroa.0.0.copyload, i1 %tobo
   ; 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)