[ARM] Fix invalid symbol redefinition due to duplicated jumptable (PR42760)

[nikic]

Summary

Fix for https://bugs.llvm.org/show_bug.cgi?id=42760. A tBR_JTr instruction is duplicated by tail duplication, which results in the same jumptable with the same label being emitted twice.

Fix this by marking tBR_JTr as not duplicable. The corresponding ARM instructions (BR_JTr, BR_JTm_i12, BR_JTm_rs and BR_JTadd) are already marked as not duplicable.

Not sure whether the test-case adds any value, it's a fairly ugly bugpoint reduction of the original issue.

[efriedma]

Maybe you can make a simpler MIR testcase? (http://llvm.org/docs/MIRLangRef.html)

It's probably feasible to teach constant islands and the asmprinter to handle jump tables with multiple uses correctly, but if we've disabled it for ARM/Thumb2 I'm fine doing the same for Thumb1.

[nikic]

Simplify testcase.

[nikic]

Also mark tTBB_JT and tTBH_JT as not duplicable.

[nikic]

I've reduced the test case to a more reasonable size now...

In D65606#1611070, @efriedma wrote:

It's probably feasible to teach constant islands and the asmprinter to handle jump tables with multiple uses correctly, but if we've disabled it for ARM/Thumb2 I'm fine doing the same for Thumb1.

I just checked over all the jumptable opcodes: All the ARM+Thumb2 were marked, and all the Thumb1 weren't. I've now added the isNotDuplicable annotation to tTBB_JT and tTBH_JT as well.

As to actually making the duplication work, I don't really have a handle on how complicated that would be. We should probably land a minimal fix first in any case, because this also needs to go into the LLVM9 branch.

[efriedma]

llvm/lib/Target/ARM/ARMInstrThumb.td

1469

I can't imagine this actually matters, given we only form tbb/tbh in constant islands, but fine.

llvm/test/CodeGen/Thumb/pr42760.ll

2

The goal is that it doesn't crash, yes, but CHECK lines are also important to ensure that you're actually triggering the codepath you think you're triggering, if the compiler changes in the future. In particular, that you're generating a jump table, and it would be feasible to tail-duplicate the jump table branch if it wasn't marked noduplicate).

[nikic]

Generate check lines.

[efriedma]

LGTM

[nikic]

llvm/lib/Target/ARM/ARMInstrThumb.td

1469

I've added this for consistency with the t2TBB_JT/t2TBH_JT opcodes. Happy to drop if not needed.

llvm/test/CodeGen/Thumb/pr42760.ll

2

I've added the CHECK lines now.

llvm/lib/Target/ARM/ARMInstrThumb.td

def tBfar : tPseudoExpand<(outs), (ins thumb_bl_target:$target, pred:$p),
                           Sched<[WriteBrTbl]>;
 
   def tBR_JTr : tPseudoInst<(outs),
                       (ins tGPR:$target, i32imm:$jt),
                       0, IIC_Br,
                       [(ARMbrjt tGPR:$target, tjumptable:$jt)]>,
                       Sched<[WriteBrTbl]> {
     let Size = 2;
+    let isNotDuplicable = 1;
     list<Predicate> Predicates = [IsThumb, IsThumb1Only];
   }
 }
 
 // FIXME: should be able to write a pattern for ARMBrcond, but can't use
 // a two-value operand where a dag node expects two operands. :(
 let isBranch = 1, isTerminator = 1 in
   def tBcc : T1I<(outs), (ins thumb_bcc_target:$target, pred:$p), IIC_Br,

 let hasSideEffects = 1 in
 def tLEApcrelJT : tPseudoInst<(outs tGPR:$Rd),
                               (ins i32imm:$label, pred:$p),
                               2, IIC_iALUi, []>, Sched<[WriteALU]>;
 
 // Thumb-1 doesn't have the TBB or TBH instructions, but we can synthesize them
 // and make use of the same compressed jump table format as Thumb-2.
 let Size = 2, isBranch = 1, isTerminator = 1, isBarrier = 1,
-    isIndirectBranch = 1 in {
+    isIndirectBranch = 1, isNotDuplicable = 1 in {
 def tTBB_JT : tPseudoInst<(outs),
         (ins tGPRwithpc:$base, tGPR:$index, i32imm:$jt, i32imm:$pclbl), 0,
          IIC_Br, []>, Sched<[WriteBr]>;
 
 def tTBH_JT : tPseudoInst<(outs),
         (ins tGPRwithpc:$base, tGPR:$index, i32imm:$jt, i32imm:$pclbl), 0,
          IIC_Br, []>,  Sched<[WriteBr]>;
 }

llvm/test/CodeGen/Thumb/pr42760.ll

+; RUN: llc -tail-dup-placement-threshold=3 < %s
+; We only check that this doesn't trigger an error due to duplicated jumptable.
+
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv6m-none-unknown-eabi"
+
+define hidden void @test() {
+entry:
+  br label %switch
+
+switch:                                           ; preds = %bb2, %entry
+  switch i32 undef, label %dead2 [
+    i32 0, label %dead
+    i32 1, label %bb2
+    i32 2, label %dead
+    i32 3, label %bb
+  ]
+
+dead:                                             ; preds = %bb2, %bb, %switch, %switch
+  unreachable
+
+dead2:                                            ; preds = %switch
+  unreachable
+
+bb:                                               ; preds = %switch
+  br i1 undef, label %dead, label %bb2
+
+bb2:                                              ; preds = %bb, %switch
+  br i1 undef, label %dead, label %switch
+}