[llvm] r186735 - Refactor AnalyzeBranch on ARM. The previous version did not always analyze
Lang Hames
lhames at gmail.com
Fri Jul 19 16:52:47 PDT 2013
Author: lhames
Date: Fri Jul 19 18:52:47 2013
New Revision: 186735
URL: http://llvm.org/viewvc/llvm-project?rev=186735&view=rev
Log:
Refactor AnalyzeBranch on ARM. The previous version did not always analyze
indirect branches correctly. Under some circumstances, this led to the deletion
of basic blocks that were the destination of indirect branches. In that case it
left indirect branches to nowhere in the code.
This patch replaces, and is more general than either of the previous fixes for
indirect-branch-analysis issues, r181161 and r186461.
For other branches (not indirect) this refactor should have *almost* identical
behavior to the previous version. There are some corner cases where this
refactor is able to analyze blocks that the previous version could not (e.g.
this necessitated the update to thumb2-ifcvt2.ll).
<rdar://problem/14464830>
Added:
llvm/trunk/test/CodeGen/ARM/indirectbr-3.ll
Modified:
llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp
llvm/trunk/test/CodeGen/Thumb2/thumb2-ifcvt2.ll
Modified: llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp?rev=186735&r1=186734&r2=186735&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp Fri Jul 19 18:52:47 2013
@@ -272,111 +272,90 @@ ARMBaseInstrInfo::AnalyzeBranch(MachineB
MachineBasicBlock *&FBB,
SmallVectorImpl<MachineOperand> &Cond,
bool AllowModify) const {
- // If the block has no terminators, it just falls into the block after it.
+ TBB = 0;
+ FBB = 0;
+
MachineBasicBlock::iterator I = MBB.end();
if (I == MBB.begin())
- return false;
+ return false; // Empty blocks are easy.
--I;
- while (I->isDebugValue()) {
- if (I == MBB.begin())
- return false;
- --I;
- }
- // Get the last instruction in the block.
- MachineInstr *LastInst = I;
- unsigned LastOpc = LastInst->getOpcode();
-
- // Check if it's an indirect branch first, this should return 'unanalyzable'
- // even if it's predicated.
- if (isIndirectBranchOpcode(LastOpc))
- return true;
-
- if (!isUnpredicatedTerminator(I))
- return false;
-
- // Check whether the second-to-last branch is indirect, return
- // 'unanalyzeable' here too.
- if (I != MBB.begin() && prior(I)->isIndirectBranch())
- return true;
-
- // If there is only one terminator instruction, process it.
- if (I == MBB.begin() || !isUnpredicatedTerminator(--I)) {
- if (isUncondBranchOpcode(LastOpc)) {
- TBB = LastInst->getOperand(0).getMBB();
- return false;
+ // Walk backwards from the end of the basic block until the branch is
+ // analyzed or we give up.
+ while (isPredicated(I) || I->isTerminator()) {
+
+ // Flag to be raised on unanalyzeable instructions. This is useful in cases
+ // where we want to clean up on the end of the basic block before we bail
+ // out.
+ bool CantAnalyze = false;
+
+ // Skip over DEBUG values and predicated nonterminators.
+ while (I->isDebugValue() || !I->isTerminator()) {
+ if (I == MBB.begin())
+ return false;
+ --I;
}
- if (isCondBranchOpcode(LastOpc)) {
- // Block ends with fall-through condbranch.
- TBB = LastInst->getOperand(0).getMBB();
- Cond.push_back(LastInst->getOperand(1));
- Cond.push_back(LastInst->getOperand(2));
- return false;
+
+ if (isIndirectBranchOpcode(I->getOpcode()) ||
+ isJumpTableBranchOpcode(I->getOpcode())) {
+ // Indirect branches and jump tables can't be analyzed, but we still want
+ // to clean up any instructions at the tail of the basic block.
+ CantAnalyze = true;
+ } else if (isUncondBranchOpcode(I->getOpcode())) {
+ TBB = I->getOperand(0).getMBB();
+ } else if (isCondBranchOpcode(I->getOpcode())) {
+ // Bail out if we encounter multiple conditional branches.
+ if (!Cond.empty())
+ return true;
+
+ assert(!FBB && "FBB should have been null.");
+ FBB = TBB;
+ TBB = I->getOperand(0).getMBB();
+ Cond.push_back(I->getOperand(1));
+ Cond.push_back(I->getOperand(2));
+ } else if (I->isReturn()) {
+ // Returns can't be analyzed, but we should run cleanup.
+ CantAnalyze = !isPredicated(I);
+ } else {
+ // We encountered other unrecognized terminator. Bail out immediately.
+ return true;
}
- return true; // Can't handle indirect branch.
- }
- // Get the instruction before it if it is a terminator.
- MachineInstr *SecondLastInst = I;
- unsigned SecondLastOpc = SecondLastInst->getOpcode();
-
- // If AllowModify is true and the block ends with two or more unconditional
- // branches, delete all but the first unconditional branch.
- if (AllowModify && isUncondBranchOpcode(LastOpc)) {
- while (isUncondBranchOpcode(SecondLastOpc)) {
- LastInst->eraseFromParent();
- LastInst = SecondLastInst;
- LastOpc = LastInst->getOpcode();
- if (I != MBB.begin() && prior(I)->isIndirectBranch())
- return true; // Indirect branches are unanalyzeable.
- if (I == MBB.begin() || !isUnpredicatedTerminator(--I)) {
- // Return now the only terminator is an unconditional branch.
- TBB = LastInst->getOperand(0).getMBB();
- return false;
- } else {
- SecondLastInst = I;
- SecondLastOpc = SecondLastInst->getOpcode();
+ // Cleanup code - to be run for unpredicated unconditional branches and
+ // returns.
+ if (!isPredicated(I) &&
+ (isUncondBranchOpcode(I->getOpcode()) ||
+ isIndirectBranchOpcode(I->getOpcode()) ||
+ isJumpTableBranchOpcode(I->getOpcode()) ||
+ I->isReturn())) {
+ // Forget any previous condition branch information - it no longer applies.
+ Cond.clear();
+ FBB = 0;
+
+ // If we can modify the function, delete everything below this
+ // unconditional branch.
+ if (AllowModify) {
+ MachineBasicBlock::iterator DI = llvm::next(I);
+ while (DI != MBB.end()) {
+ MachineInstr *InstToDelete = DI;
+ ++DI;
+ InstToDelete->eraseFromParent();
+ }
}
}
- }
- // If there are three terminators, we don't know what sort of block this is.
- if (SecondLastInst && I != MBB.begin() && isUnpredicatedTerminator(--I))
- return true;
-
- // If the block ends with a B and a Bcc, handle it.
- if (isCondBranchOpcode(SecondLastOpc) && isUncondBranchOpcode(LastOpc)) {
- TBB = SecondLastInst->getOperand(0).getMBB();
- Cond.push_back(SecondLastInst->getOperand(1));
- Cond.push_back(SecondLastInst->getOperand(2));
- FBB = LastInst->getOperand(0).getMBB();
- return false;
- }
+ if (CantAnalyze)
+ return true;
- // If the block ends with two unconditional branches, handle it. The second
- // one is not executed, so remove it.
- if (isUncondBranchOpcode(SecondLastOpc) && isUncondBranchOpcode(LastOpc)) {
- TBB = SecondLastInst->getOperand(0).getMBB();
- I = LastInst;
- if (AllowModify)
- I->eraseFromParent();
- return false;
- }
+ if (I == MBB.begin())
+ return false;
- // ...likewise if it ends with a branch table followed by an unconditional
- // branch. The branch folder can create these, and we must get rid of them for
- // correctness of Thumb constant islands.
- if ((isJumpTableBranchOpcode(SecondLastOpc) ||
- isIndirectBranchOpcode(SecondLastOpc)) &&
- isUncondBranchOpcode(LastOpc)) {
- I = LastInst;
- if (AllowModify)
- I->eraseFromParent();
- return true;
+ --I;
}
- // Otherwise, can't handle this.
- return true;
+ // We made it past the terminators without bailing out - we must have
+ // analyzed this branch successfully.
+ return false;
}
Added: llvm/trunk/test/CodeGen/ARM/indirectbr-3.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/indirectbr-3.ll?rev=186735&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/indirectbr-3.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/indirectbr-3.ll Fri Jul 19 18:52:47 2013
@@ -0,0 +1,32 @@
+; RUN: llc < %s -mtriple=thumbv7-apple-ios | FileCheck %s
+
+; If ARMBaseInstrInfo::AnalyzeBlocks returns the wrong value, which was possible
+; for blocks with indirect branches, the IfConverter could end up deleting
+; blocks that were the destinations of indirect branches, leaving branches to
+; nowhere.
+; <rdar://problem/14464830>
+
+define i32 @preserve_blocks(i32 %x) {
+; preserve_blocks:
+; CHECK: Block address taken
+; CHECK: movs r0, #2
+; CHECK: movs r0, #1
+; CHECK-NOT: Address of block that was removed by CodeGen
+entry:
+ %c2 = icmp slt i32 %x, 3
+ %blockaddr = select i1 %c2, i8* blockaddress(@preserve_blocks, %ibt1), i8* blockaddress(@preserve_blocks, %ibt2)
+ %c1 = icmp eq i32 %x, 0
+ br i1 %c1, label %pre_ib, label %nextblock
+
+nextblock:
+ ret i32 3
+
+ibt1:
+ ret i32 2
+
+ibt2:
+ ret i32 1
+
+pre_ib:
+ indirectbr i8* %blockaddr, [ label %ibt1, label %ibt2 ]
+}
Modified: llvm/trunk/test/CodeGen/Thumb2/thumb2-ifcvt2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Thumb2/thumb2-ifcvt2.ll?rev=186735&r1=186734&r2=186735&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/Thumb2/thumb2-ifcvt2.ll (original)
+++ llvm/trunk/test/CodeGen/Thumb2/thumb2-ifcvt2.ll Fri Jul 19 18:52:47 2013
@@ -29,13 +29,13 @@ declare i32 @bar(...)
define fastcc i32 @CountTree(%struct.quad_struct* %tree) {
entry:
; CHECK-LABEL: CountTree:
-; CHECK: itt eq
-; CHECK: moveq
-; CHECK: popeq
; CHECK: bne
; CHECK: cmp
; CHECK: it eq
; CHECK: cmpeq
+; CHECK: itt eq
+; CHECK: moveq
+; CHECK: popeq
br label %tailrecurse
tailrecurse: ; preds = %bb, %entry
More information about the llvm-commits
mailing list