From 56dbbe30a2dbfe02e82e32b59beae70b5e0eea34 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Tue, 16 Aug 2016 17:51:12 +0000 Subject: [PATCH] Merging r278575 (with changes to the test): ------------------------------------------------------------------------ r278575 | haicheng | 2016-08-12 16:13:38 -0700 (Fri, 12 Aug 2016) | 6 lines Reapply [BranchFolding] Restrict tail merging loop blocks after MBP Fixed a bug in the test case. To fix PR28104, this patch restricts tail merging to blocks that belong to the same loop after MBP. ------------------------------------------------------------------------ I had to adjust the test as it wasn't passing on the branch, presumably due to different machine block placement. git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_39@278827 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/BranchFolding.cpp | 32 ++++++--- test/CodeGen/ARM/arm-and-tst-peephole.ll | 2 +- test/CodeGen/X86/tail-merge-after-mbp.ll | 90 ++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 test/CodeGen/X86/tail-merge-after-mbp.ll diff --git a/lib/CodeGen/BranchFolding.cpp b/lib/CodeGen/BranchFolding.cpp index fa705761645..23e2aa70d0c 100644 --- a/lib/CodeGen/BranchFolding.cpp +++ b/lib/CodeGen/BranchFolding.cpp @@ -996,6 +996,24 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) { MachineBasicBlock *IBB = &*I; MachineBasicBlock *PredBB = &*std::prev(I); MergePotentials.clear(); + MachineLoop *ML; + + // Bail if merging after placement and IBB is the loop header because + // -- If merging predecessors that belong to the same loop as IBB, the + // common tail of merged predecessors may become the loop top if block + // placement is called again and the predecessors may branch to this common + // tail and require more branches. This can be relaxed if + // MachineBlockPlacement::findBestLoopTop is more flexible. + // --If merging predecessors that do not belong to the same loop as IBB, the + // loop info of IBB's loop and the other loops may be affected. Calling the + // block placement again may make big change to the layout and eliminate the + // reason to do tail merging here. + if (AfterBlockPlacement && MLI) { + ML = MLI->getLoopFor(IBB); + if (ML && IBB == ML->getHeader()) + continue; + } + for (MachineBasicBlock *PBB : I->predecessors()) { if (MergePotentials.size() == TailMergeThreshold) break; @@ -1015,16 +1033,12 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) { if (PBB->hasEHPadSuccessor()) continue; - // Bail out if the loop header (IBB) is not the top of the loop chain - // after the block placement. Otherwise, the common tail of IBB's - // predecessors may become the loop top if block placement is called again - // and the predecessors may branch to this common tail. - // FIXME: Relaxed this check if the algorithm of finding loop top is - // changed in MBP. + // After block placement, only consider predecessors that belong to the + // same loop as IBB. The reason is the same as above when skipping loop + // header. if (AfterBlockPlacement && MLI) - if (MachineLoop *ML = MLI->getLoopFor(IBB)) - if (IBB == ML->getHeader() && ML == MLI->getLoopFor(PBB)) - continue; + if (ML != MLI->getLoopFor(PBB)) + continue; MachineBasicBlock *TBB = nullptr, *FBB = nullptr; SmallVector Cond; diff --git a/test/CodeGen/ARM/arm-and-tst-peephole.ll b/test/CodeGen/ARM/arm-and-tst-peephole.ll index 151cc1b12ed..04eae8f9afe 100644 --- a/test/CodeGen/ARM/arm-and-tst-peephole.ll +++ b/test/CodeGen/ARM/arm-and-tst-peephole.ll @@ -49,7 +49,7 @@ tailrecurse.switch: ; preds = %tailrecurse ; V8-NEXT: beq ; V8-NEXT: %tailrecurse.switch ; V8: cmp -; V8-NEXT: beq +; V8-NEXT: bne ; V8-NEXT: b ; The trailing space in the last line checks that the branch is unconditional switch i32 %and, label %sw.epilog [ diff --git a/test/CodeGen/X86/tail-merge-after-mbp.ll b/test/CodeGen/X86/tail-merge-after-mbp.ll new file mode 100644 index 00000000000..411f6b2329c --- /dev/null +++ b/test/CodeGen/X86/tail-merge-after-mbp.ll @@ -0,0 +1,90 @@ +; RUN: llc -mtriple=x86_64-linux -o - %s | FileCheck %s + +%0 = type { %1, %3* } +%1 = type { %2* } +%2 = type { %2*, i8* } +%3 = type { i32, i32 (i32, i32)* } + + +declare i32 @Up(...) +declare i32 @f(i32, i32) + +define i32 @foo(%0* nocapture readonly, i32, i1 %c, i8* %p1, %2** %p2) { +; CHECK-LABEL: foo: +; CHECK: BB#6: +; CHECK-NEXT: movq (%r14), %rax +; CHECK-NEXT: testq %rax, %rax +; CHECK-NEXT: je +; CHECK-NEXT:# BB#7: +; CHECK-NEXT: cmpq $0, 8(%rax) +; CHECK-NEXT: jne +; CHECK-NEXT:# BB#8: +; CHECK-NEXT: movq (%r14), %rax +; CHECK-NEXT: testq %rax, %rax +; CHECK-NEXT: je +; CHECK-NEXT:LBB0_9: +; CHECK-NEXT: cmpq $0, 8(%rax) +; CHECK-NEXT: jne +; CHECK-NEXT:# BB#10: +; CHECK-NEXT: movq (%r14), %rax +; CHECK-NEXT: testq %rax, %rax +; CHECK-NEXT: jne + br i1 %c, label %34, label %3 + +;