[llvm] r330345 - [if-converter] Handle BBs that terminate in ret during diamond conversion
Krzysztof Parzyszek via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 19 10:26:46 PDT 2018
Author: kparzysz
Date: Thu Apr 19 10:26:46 2018
New Revision: 330345
URL: http://llvm.org/viewvc/llvm-project?rev=330345&view=rev
Log:
[if-converter] Handle BBs that terminate in ret during diamond conversion
This fixes https://llvm.org/PR36825.
Original patch by Valentin Churavy (D45218).
Differential Revision: https://reviews.llvm.org/D45731
Added:
llvm/trunk/test/CodeGen/Hexagon/ifcvt-diamond-ret.mir
llvm/trunk/test/CodeGen/MIR/PowerPC/ifcvt-diamond-ret.mir
Modified:
llvm/trunk/lib/CodeGen/IfConversion.cpp
Modified: llvm/trunk/lib/CodeGen/IfConversion.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/IfConversion.cpp?rev=330345&r1=330344&r2=330345&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/IfConversion.cpp (original)
+++ llvm/trunk/lib/CodeGen/IfConversion.cpp Thu Apr 19 10:26:46 2018
@@ -1714,20 +1714,25 @@ bool IfConverter::IfConvertDiamondCommon
}
// Remove the duplicated instructions at the beginnings of both paths.
- // Skip dbg_value instructions
+ // Skip dbg_value instructions.
MachineBasicBlock::iterator DI1 = MBB1.getFirstNonDebugInstr();
MachineBasicBlock::iterator DI2 = MBB2.getFirstNonDebugInstr();
BBI1->NonPredSize -= NumDups1;
BBI2->NonPredSize -= NumDups1;
// Skip past the dups on each side separately since there may be
- // differing dbg_value entries.
+ // differing dbg_value entries. NumDups1 can include a "return"
+ // instruction, if it's not marked as "branch".
for (unsigned i = 0; i < NumDups1; ++DI1) {
+ if (DI1 == MBB1.end())
+ break;
if (!DI1->isDebugValue())
++i;
}
while (NumDups1 != 0) {
++DI2;
+ if (DI2 == MBB2.end())
+ break;
if (!DI2->isDebugValue())
--NumDups1;
}
@@ -1738,11 +1743,16 @@ bool IfConverter::IfConvertDiamondCommon
Redefs.stepForward(MI, Dummy);
}
}
+
BBI.BB->splice(BBI.BB->end(), &MBB1, MBB1.begin(), DI1);
MBB2.erase(MBB2.begin(), DI2);
- // The branches have been checked to match, so it is safe to remove the branch
- // in BB1 and rely on the copy in BB2
+ // The branches have been checked to match, so it is safe to remove the
+ // branch in BB1 and rely on the copy in BB2. The complication is that
+ // the blocks may end with a return instruction, which may or may not
+ // be marked as "branch". If it's not, then it could be included in
+ // "dups1", leaving the blocks potentially empty after moving the common
+ // duplicates.
#ifndef NDEBUG
// Unanalyzable branches must match exactly. Check that now.
if (!BBI1->IsBrAnalyzable)
@@ -1768,11 +1778,14 @@ bool IfConverter::IfConvertDiamondCommon
if (RemoveBranch)
BBI2->NonPredSize -= TII->removeBranch(*BBI2->BB);
else {
- do {
- assert(DI2 != MBB2.begin());
- DI2--;
- } while (DI2->isBranch() || DI2->isDebugValue());
- DI2++;
+ // Make DI2 point to the end of the range where the common "tail"
+ // instructions could be found.
+ while (DI2 != MBB2.begin()) {
+ MachineBasicBlock::iterator Prev = std::prev(DI2);
+ if (!Prev->isBranch() && !Prev->isDebugValue())
+ break;
+ DI2 = Prev;
+ }
}
while (NumDups2 != 0) {
// NumDups2 only counted non-dbg_value instructions, so this won't
@@ -1833,11 +1846,15 @@ bool IfConverter::IfConvertDiamondCommon
// a non-predicated in BBI2, then we don't want to predicate the one from
// BBI2. The reason is that if we merged these blocks, we would end up with
// two predicated terminators in the same block.
+ // Also, if the branches in MBB1 and MBB2 were non-analyzable, then don't
+ // predicate them either. They were checked to be identical, and so the
+ // same branch would happen regardless of which path was taken.
if (!MBB2.empty() && (DI2 == MBB2.end())) {
MachineBasicBlock::iterator BBI1T = MBB1.getFirstTerminator();
MachineBasicBlock::iterator BBI2T = MBB2.getFirstTerminator();
- if (BBI1T != MBB1.end() && TII->isPredicated(*BBI1T) &&
- BBI2T != MBB2.end() && !TII->isPredicated(*BBI2T))
+ bool BB1Predicated = BBI1T != MBB1.end() && TII->isPredicated(*BBI1T);
+ bool BB2NonPredicated = BBI2T != MBB2.end() && !TII->isPredicated(*BBI2T);
+ if (BB2NonPredicated && (BB1Predicated || !BBI2->IsBrAnalyzable))
--DI2;
}
Added: llvm/trunk/test/CodeGen/Hexagon/ifcvt-diamond-ret.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Hexagon/ifcvt-diamond-ret.mir?rev=330345&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Hexagon/ifcvt-diamond-ret.mir (added)
+++ llvm/trunk/test/CodeGen/Hexagon/ifcvt-diamond-ret.mir Thu Apr 19 10:26:46 2018
@@ -0,0 +1,25 @@
+# RUN: llc -march=hexagon -run-pass if-converter %s -o - | FileCheck %s
+
+# Make sure this gets if-converted and it doesn't crash.
+# CHECK-LABEL: bb.0
+# CHECK: PS_jmpret $r31
+# CHECK-NOT: bb.{{[1-9]+}}:
+
+---
+name: fred
+tracksRegLiveness: true
+body: |
+ bb.0:
+ successors: %bb.1, %bb.2
+ liveins: $r0
+ renamable $p0 = C2_cmpeqi killed renamable $r0, 0
+ J2_jumpf killed renamable $p0, %bb.2, implicit-def dead $pc
+
+ bb.1:
+ S4_storeiri_io undef renamable $r0, 0, 32768 :: (store 4 into `i32* undef`)
+ PS_jmpret $r31, implicit-def dead $pc
+
+ bb.2:
+ S4_storeiri_io undef renamable $r0, 0, 32768 :: (store 4 into `i32* undef`)
+ PS_jmpret $r31, implicit-def dead $pc
+...
Added: llvm/trunk/test/CodeGen/MIR/PowerPC/ifcvt-diamond-ret.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/MIR/PowerPC/ifcvt-diamond-ret.mir?rev=330345&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/MIR/PowerPC/ifcvt-diamond-ret.mir (added)
+++ llvm/trunk/test/CodeGen/MIR/PowerPC/ifcvt-diamond-ret.mir Thu Apr 19 10:26:46 2018
@@ -0,0 +1,34 @@
+# RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -run-pass=if-converter %s -o - | FileCheck %s
+---
+name: foo
+body: |
+ bb.0:
+ liveins: $x0, $x3
+ successors: %bb.1(0x40000000), %bb.2(0x40000000)
+
+ dead renamable $x3 = ANDIo8 killed renamable $x3, 1, implicit-def dead $cr0, implicit-def $cr0gt
+ $cr2lt = CROR $cr0gt, $cr0gt
+ BCn killed renamable $cr2lt, %bb.2
+ B %bb.1
+
+ bb.1:
+ renamable $x3 = LIS8 4096
+ MTLR8 $x0, implicit-def $lr8
+ BLR8 implicit $lr8, implicit $rm, implicit $x3
+
+ bb.2:
+ renamable $x3 = LIS8 4096
+ MTLR8 $x0, implicit-def $lr8
+ BLR8 implicit $lr8, implicit $rm, implicit $x3
+...
+
+# Diamond testcase with equivalent branches terminating in returns.
+
+# CHECK: body: |
+# CHECK: bb.0:
+# CHECK: dead renamable $x3 = ANDIo8 killed renamable $x3, 1, implicit-def dead $cr0, implicit-def $cr0gt
+# CHECK: $cr2lt = CROR $cr0gt, $cr0gt
+# CHECK: renamable $x3 = LIS8 4096
+# CHECK: MTLR8 $x0, implicit-def $lr8
+# CHECK: BLR8 implicit $lr8, implicit $rm, implicit $x3
+
More information about the llvm-commits
mailing list