[PATCH] D110434: [ARM] Fix Arm block placement creating branches after jump tables.

[Dave Green via Phabricator via llvm-commits]

dmgreen created this revision.
dmgreen added reviewers: samtebbs, SjoerdMeijer, chill, lenary, simon_tatham.
Herald added subscribers: hiraditya, kristof.beyls.
dmgreen requested review of this revision.
Herald added a project: LLVM.

Given:

- A jump table
- Which jumps to the next block
- The next block ends in a WLS
- Where the WLS conditionally jumps to block earlier in the program.

The Arm block placement pass would attempt to move the block containing the WLS earlier, as the WLS instruction can only branch forward. In doing so it would add a branch from the jumptable block to the WLS block, thinking it previously fell-through. This in itself would be fine, if a little inefficient, but the constant island pass expects all instructions after a jump-table branch to have been removed by analyzeBranch. So it gets confused and can assign the same labels to multiple jump table blocks.

I've changed the condition to the same as used in analyzeBranch.


https://reviews.llvm.org/D110434

Files:
  llvm/lib/Target/ARM/ARMBlockPlacement.cpp
  llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir


Index: llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir
===================================================================
--- llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir
+++ llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir
@@ -821,7 +821,6 @@
   ; CHECK-NEXT:   renamable $r5 = t2LEApcrelJT %jump-table.0, 14 /* CC::al */, $noreg
   ; CHECK-NEXT:   renamable $r5 = t2ADDrs killed renamable $r5, renamable $r0, 18, 14 /* CC::al */, $noreg, $noreg
   ; CHECK-NEXT:   t2BR_JT killed renamable $r5, killed renamable $r0, %jump-table.0
-  ; CHECK-NEXT:   t2B %bb.1, 14 /* CC::al */, $noreg
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.4:
   ; CHECK-NEXT:   successors: %bb.4(0x7c000000), %bb.2(0x04000000)
Index: llvm/lib/Target/ARM/ARMBlockPlacement.cpp
===================================================================
--- llvm/lib/Target/ARM/ARMBlockPlacement.cpp
+++ llvm/lib/Target/ARM/ARMBlockPlacement.cpp
@@ -259,18 +259,22 @@
     assert(From->isSuccessor(To) &&
            "'To' is expected to be a successor of 'From'");
     MachineInstr &Terminator = *(--From->terminators().end());
-    if (!Terminator.isUnconditionalBranch()) {
-      // The BB doesn't have an unconditional branch so it relied on
-      // fall-through. Fix by adding an unconditional branch to the moved BB.
-      MachineInstrBuilder MIB =
-          BuildMI(From, Terminator.getDebugLoc(), TII->get(ARM::t2B));
-      MIB.addMBB(To);
-      MIB.addImm(ARMCC::CondCodes::AL);
-      MIB.addReg(ARM::NoRegister);
-      LLVM_DEBUG(dbgs() << DEBUG_PREFIX << "Adding unconditional branch from "
-                        << From->getName() << " to " << To->getName() << ": "
-                        << *MIB.getInstr());
-    }
+    if (!TII->isPredicated(Terminator) &&
+        (isUncondBranchOpcode(Terminator.getOpcode()) ||
+         isIndirectBranchOpcode(Terminator.getOpcode()) ||
+         isJumpTableBranchOpcode(Terminator.getOpcode()) ||
+         Terminator.isReturn()))
+      return;
+    // The BB doesn't have an unconditional branch so it relied on
+    // fall-through. Fix by adding an unconditional branch to the moved BB.
+    MachineInstrBuilder MIB =
+        BuildMI(From, Terminator.getDebugLoc(), TII->get(ARM::t2B));
+    MIB.addMBB(To);
+    MIB.addImm(ARMCC::CondCodes::AL);
+    MIB.addReg(ARM::NoRegister);
+    LLVM_DEBUG(dbgs() << DEBUG_PREFIX << "Adding unconditional branch from "
+                      << From->getName() << " to " << To->getName() << ": "
+                      << *MIB.getInstr());
   };
 
   // Fix fall-through to the moved BB from the one that used to be before it.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110434.374907.patch
Type: text/x-patch
Size: 2624 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210924/4169bf18/attachment.bin>