[llvm] 883758e - [ARM] Fix Arm block placement creating branches after jump tables.

[David Green via llvm-commits]

Author: David Green
Date: 2021-09-25T11:32:25+01:00
New Revision: 883758ed480012a875146bc206cb8bcdd384405b

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

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

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.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/ARM/ARMBlockPlacement.cpp b/llvm/lib/Target/ARM/ARMBlockPlacement.cpp
index ec58f2051f63f..3488ed03e7ebf 100644
--- a/llvm/lib/Target/ARM/ARMBlockPlacement.cpp
+++ b/llvm/lib/Target/ARM/ARMBlockPlacement.cpp
@@ -259,18 +259,22 @@ void ARMBlockPlacement::moveBasicBlock(MachineBasicBlock *BB,
     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.

diff  --git a/llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir b/llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir
index a942d7c18abfa..9b39d469fff86 100644
--- a/llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir
+++ b/llvm/test/CodeGen/Thumb2/mve-wls-block-placement.mir
@@ -821,7 +821,6 @@ body:             |
   ; 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)