Fix a bug which prevented tail merging of return instructions in

beneficial cases. See the changes in test/CodeGen/X86/tail-opts.ll and
test/CodeGen/ARM/ifcvt2.ll for details.

The fix is to change HashEndOfMBB to hash at most one instruction,
instead of trying to apply heuristics about when it will be profitable to
consider more than one instruction. The regular tail-merging heuristics
are already prepared to handle the same cases, and they're more precise.

Also, make test/CodeGen/ARM/ifcvt5.ll and
test/CodeGen/Thumb2/thumb2-branch.ll slightly more complex so that they
continue to test what they're intended to test.

And, this eliminates the problem in
test/CodeGen/Thumb2/2009-10-15-ITBlockBranch.ll, the testcase from
PR5204. Update it accordingly.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@102907 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Dan Gohman 2010-05-03 14:35:47 +00:00
parent 3a2a4846a6
commit 30fc5bbfd1
6 changed files with 52 additions and 50 deletions

View File

@ -264,14 +264,8 @@ static unsigned HashMachineInstr(const MachineInstr *MI) {
return Hash; return Hash;
} }
/// HashEndOfMBB - Hash the last few instructions in the MBB. For blocks /// HashEndOfMBB - Hash the last instruction in the MBB.
/// with no successors, we hash two instructions, because cross-jumping static unsigned HashEndOfMBB(const MachineBasicBlock *MBB) {
/// only saves code when at least two instructions are removed (since a
/// branch must be inserted). For blocks with a successor, one of the
/// two blocks to be tail-merged will end with a branch already, so
/// it gains to cross-jump even for one instruction.
static unsigned HashEndOfMBB(const MachineBasicBlock *MBB,
unsigned minCommonTailLength) {
MachineBasicBlock::const_iterator I = MBB->end(); MachineBasicBlock::const_iterator I = MBB->end();
if (I == MBB->begin()) if (I == MBB->begin())
return 0; // Empty MBB. return 0; // Empty MBB.
@ -283,20 +277,8 @@ static unsigned HashEndOfMBB(const MachineBasicBlock *MBB,
return 0; // MBB empty except for debug info. return 0; // MBB empty except for debug info.
--I; --I;
} }
unsigned Hash = HashMachineInstr(I);
if (I == MBB->begin() || minCommonTailLength == 1) return HashMachineInstr(I);
return Hash; // Single instr MBB.
--I;
while (I->isDebugValue()) {
if (I==MBB->begin())
return Hash; // MBB with single non-debug instr.
--I;
}
// Hash in the second-to-last instruction.
Hash ^= HashMachineInstr(I) << 2;
return Hash;
} }
/// ComputeCommonTailLength - Given two machine basic blocks, compute the number /// ComputeCommonTailLength - Given two machine basic blocks, compute the number
@ -811,7 +793,7 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
MergePotentials.clear(); MergePotentials.clear();
for (MachineFunction::iterator I = MF.begin(), E = MF.end(); I != E; ++I) { for (MachineFunction::iterator I = MF.begin(), E = MF.end(); I != E; ++I) {
if (I->succ_empty()) if (I->succ_empty())
MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(I, 2U), I)); MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(I), I));
} }
// See if we can do any tail merging on those. // See if we can do any tail merging on those.
@ -897,8 +879,7 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
// reinsert conditional branch only, for now // reinsert conditional branch only, for now
TII->InsertBranch(*PBB, (TBB == IBB) ? FBB : TBB, 0, NewCond); TII->InsertBranch(*PBB, (TBB == IBB) ? FBB : TBB, 0, NewCond);
} }
MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(PBB, 1U), MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(PBB), *P));
*P));
} }
} }
if (MergePotentials.size() >= 2) if (MergePotentials.size() >= 2)

View File

@ -1,7 +1,8 @@
; RUN: llc < %s -march=arm ; RUN: llc < %s -march=arm > %t
; RUN: llc < %s -march=arm | grep bxlt | count 1 ; RUN: grep bxlt %t | count 1
; RUN: llc < %s -march=arm | grep bxgt | count 1 ; RUN: grep bxgt %t | count 1
; RUN: llc < %s -march=arm | grep bxge | count 1 ; RUN: not grep bxge %t
; RUN: not grep bxle %t
define i32 @t1(i32 %a, i32 %b, i32 %c, i32 %d) { define i32 @t1(i32 %a, i32 %b, i32 %c, i32 %d) {
%tmp2 = icmp sgt i32 %c, 10 %tmp2 = icmp sgt i32 %c, 10

View File

@ -9,7 +9,7 @@ entry:
ret void ret void
} }
define void @t1(i32 %a, i32 %b) { define i32 @t1(i32 %a, i32 %b) {
; CHECK: t1: ; CHECK: t1:
; CHECK: ldmialt sp!, {r7, pc} ; CHECK: ldmialt sp!, {r7, pc}
entry: entry:
@ -18,8 +18,8 @@ entry:
cond_true: ; preds = %entry cond_true: ; preds = %entry
tail call void @foo( i32 %b ) tail call void @foo( i32 %b )
ret void ret i32 0
UnifiedReturnBlock: ; preds = %entry UnifiedReturnBlock: ; preds = %entry
ret void ret i32 1
} }

View File

@ -1,6 +1,5 @@
; RUN: llc < %s -mtriple=thumbv7-eabi -mcpu=cortex-a8 -float-abi=hard | FileCheck %s ; RUN: llc < %s -mtriple=thumbv7-eabi -mcpu=cortex-a8 -float-abi=hard | FileCheck %s
; PR5204
; A fix for PR5204 will require this check to be changed.
%"struct.__gnu_cxx::__normal_iterator<char*,std::basic_string<char, std::char_traits<char>, std::allocator<char> > >" = type { i8* } %"struct.__gnu_cxx::__normal_iterator<char*,std::basic_string<char, std::char_traits<char>, std::allocator<char> > >" = type { i8* }
%"struct.__gnu_cxx::new_allocator<char>" = type <{ i8 }> %"struct.__gnu_cxx::new_allocator<char>" = type <{ i8 }>
@ -11,11 +10,9 @@
define weak arm_aapcs_vfpcc i32 @_ZNKSs7compareERKSs(%"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %this, %"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %__str) { define weak arm_aapcs_vfpcc i32 @_ZNKSs7compareERKSs(%"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %this, %"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %__str) {
; CHECK: _ZNKSs7compareERKSs: ; CHECK: _ZNKSs7compareERKSs:
; CHECK: it ne ; CHECK: it eq
; CHECK-NEXT: ldmiane.w ; CHECK-NEXT: subeq.w r0, r6, r8
; CHECK-NEXT: itt eq ; CHECK-NEXT: ldmia.w sp, {r4, r5, r6, r8, r9, pc}
; CHECK-NEXT: subeq.w
; CHECK-NEXT: ldmiaeq.w
entry: entry:
%0 = tail call arm_aapcs_vfpcc i32 @_ZNKSs4sizeEv(%"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %this) ; <i32> [#uses=3] %0 = tail call arm_aapcs_vfpcc i32 @_ZNKSs4sizeEv(%"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %this) ; <i32> [#uses=3]
%1 = tail call arm_aapcs_vfpcc i32 @_ZNKSs4sizeEv(%"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %__str) ; <i32> [#uses=3] %1 = tail call arm_aapcs_vfpcc i32 @_ZNKSs4sizeEv(%"struct.std::basic_string<char,std::char_traits<char>,std::allocator<char> >"* %__str) ; <i32> [#uses=3]

View File

@ -1,6 +1,6 @@
; RUN: llc < %s -mtriple=thumbv7-apple-darwin -mattr=+thumb2 | FileCheck %s ; RUN: llc < %s -mtriple=thumbv7-apple-darwin -mattr=+thumb2 | FileCheck %s
define void @f1(i32 %a, i32 %b, i32* %v) { define i32 @f1(i32 %a, i32 %b, i32* %v) {
entry: entry:
; CHECK: f1: ; CHECK: f1:
; CHECK: bne LBB ; CHECK: bne LBB
@ -9,13 +9,13 @@ entry:
cond_true: ; preds = %entry cond_true: ; preds = %entry
store i32 0, i32* %v store i32 0, i32* %v
ret void ret i32 0
return: ; preds = %entry return: ; preds = %entry
ret void ret i32 1
} }
define void @f2(i32 %a, i32 %b, i32* %v) { define i32 @f2(i32 %a, i32 %b, i32* %v) {
entry: entry:
; CHECK: f2: ; CHECK: f2:
; CHECK: bge LBB ; CHECK: bge LBB
@ -24,13 +24,13 @@ entry:
cond_true: ; preds = %entry cond_true: ; preds = %entry
store i32 0, i32* %v store i32 0, i32* %v
ret void ret i32 0
return: ; preds = %entry return: ; preds = %entry
ret void ret i32 1
} }
define void @f3(i32 %a, i32 %b, i32* %v) { define i32 @f3(i32 %a, i32 %b, i32* %v) {
entry: entry:
; CHECK: f3: ; CHECK: f3:
; CHECK: bhs LBB ; CHECK: bhs LBB
@ -39,13 +39,13 @@ entry:
cond_true: ; preds = %entry cond_true: ; preds = %entry
store i32 0, i32* %v store i32 0, i32* %v
ret void ret i32 0
return: ; preds = %entry return: ; preds = %entry
ret void ret i32 1
} }
define void @f4(i32 %a, i32 %b, i32* %v) { define i32 @f4(i32 %a, i32 %b, i32* %v) {
entry: entry:
; CHECK: f4: ; CHECK: f4:
; CHECK: blo LBB ; CHECK: blo LBB
@ -54,8 +54,8 @@ entry:
cond_true: ; preds = %entry cond_true: ; preds = %entry
store i32 0, i32* %v store i32 0, i32* %v
ret void ret i32 0
return: ; preds = %entry return: ; preds = %entry
ret void ret i32 1
} }

View File

@ -406,3 +406,26 @@ bb12:
return: return:
ret void ret void
} }
; Tail-merging should merge the two ret instructions since one side
; can fall-through into the ret and the other side has to branch anyway.
; CHECK: TESTE:
; CHECK: imulq
; CHECK-NEXT: LBB8_2:
; CHECK-NEXT: ret
define i64 @TESTE(i64 %parami, i64 %paraml) nounwind readnone {
entry:
%cmp = icmp slt i64 %parami, 1 ; <i1> [#uses=1]
%varx.0 = select i1 %cmp, i64 1, i64 %parami ; <i64> [#uses=1]
%cmp410 = icmp slt i64 %paraml, 1 ; <i1> [#uses=1]
br i1 %cmp410, label %for.end, label %bb.nph
bb.nph: ; preds = %entry
%tmp15 = mul i64 %paraml, %parami ; <i64> [#uses=1]
ret i64 %tmp15
for.end: ; preds = %entry
ret i64 %varx.0
}