Skip to content

Commit c4a78b6

Browse files
authored
[SimplifyCFG] Always allow hoisting if all instructions match. (#97158)
Generalize hoistCommonCodeFromSuccessors's `EqTermsOnly` to `AllInstsEqOnly` and always allow hoisting if all instructions match. In that case, all instructions can be hoisted and the original branch will be replaced and selects for PHIs are added. This allows preserving metadata in more cases, using the existing hoisting logic, whereas previously FoldTwoEntryPHINode would drop the metadata. https://llvm-compile-time-tracker.com/compare.php?from=716360367fbdabac2c374c19b8746f4de49a5599&to=986b2c47df516b31d998c055400e4f62aa76edc6&stat=instructions:u PR: #97158
1 parent 5f72f2c commit c4a78b6

File tree

3 files changed

+121
-116
lines changed

3 files changed

+121
-116
lines changed

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 105 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ class SimplifyCFGOpt {
285285
bool tryToSimplifyUncondBranchWithICmpInIt(ICmpInst *ICI,
286286
IRBuilder<> &Builder);
287287

288-
bool hoistCommonCodeFromSuccessors(Instruction *TI, bool EqTermsOnly);
288+
bool hoistCommonCodeFromSuccessors(Instruction *TI, bool AllInstsEqOnly);
289289
bool hoistSuccIdenticalTerminatorToSwitchOrIf(
290290
Instruction *TI, Instruction *I1,
291291
SmallVectorImpl<Instruction *> &OtherSuccTIs);
@@ -1772,13 +1772,84 @@ static bool isSafeCheapLoadStore(const Instruction *I,
17721772
getLoadStoreAlignment(I) < Value::MaximumAlignment;
17731773
}
17741774

1775+
namespace {
1776+
1777+
// LockstepReverseIterator - Iterates through instructions
1778+
// in a set of blocks in reverse order from the first non-terminator.
1779+
// For example (assume all blocks have size n):
1780+
// LockstepReverseIterator I([B1, B2, B3]);
1781+
// *I-- = [B1[n], B2[n], B3[n]];
1782+
// *I-- = [B1[n-1], B2[n-1], B3[n-1]];
1783+
// *I-- = [B1[n-2], B2[n-2], B3[n-2]];
1784+
// ...
1785+
class LockstepReverseIterator {
1786+
ArrayRef<BasicBlock *> Blocks;
1787+
SmallVector<Instruction *, 4> Insts;
1788+
bool Fail;
1789+
1790+
public:
1791+
LockstepReverseIterator(ArrayRef<BasicBlock *> Blocks) : Blocks(Blocks) {
1792+
reset();
1793+
}
1794+
1795+
void reset() {
1796+
Fail = false;
1797+
Insts.clear();
1798+
for (auto *BB : Blocks) {
1799+
Instruction *Inst = BB->getTerminator();
1800+
for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
1801+
Inst = Inst->getPrevNode();
1802+
if (!Inst) {
1803+
// Block wasn't big enough.
1804+
Fail = true;
1805+
return;
1806+
}
1807+
Insts.push_back(Inst);
1808+
}
1809+
}
1810+
1811+
bool isValid() const { return !Fail; }
1812+
1813+
void operator--() {
1814+
if (Fail)
1815+
return;
1816+
for (auto *&Inst : Insts) {
1817+
for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
1818+
Inst = Inst->getPrevNode();
1819+
// Already at beginning of block.
1820+
if (!Inst) {
1821+
Fail = true;
1822+
return;
1823+
}
1824+
}
1825+
}
1826+
1827+
void operator++() {
1828+
if (Fail)
1829+
return;
1830+
for (auto *&Inst : Insts) {
1831+
for (Inst = Inst->getNextNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
1832+
Inst = Inst->getNextNode();
1833+
// Already at end of block.
1834+
if (!Inst) {
1835+
Fail = true;
1836+
return;
1837+
}
1838+
}
1839+
}
1840+
1841+
ArrayRef<Instruction *> operator*() const { return Insts; }
1842+
};
1843+
1844+
} // end anonymous namespace
1845+
17751846
/// Hoist any common code in the successor blocks up into the block. This
1776-
/// function guarantees that BB dominates all successors. If EqTermsOnly is
1777-
/// given, only perform hoisting in case both blocks only contain a terminator.
1778-
/// In that case, only the original BI will be replaced and selects for PHIs are
1779-
/// added.
1847+
/// function guarantees that BB dominates all successors. If AllInstsEqOnly is
1848+
/// given, only perform hoisting in case all successors blocks contain matching
1849+
/// instructions only. In that case, all instructions can be hoisted and the
1850+
/// original branch will be replaced and selects for PHIs are added.
17801851
bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(Instruction *TI,
1781-
bool EqTermsOnly) {
1852+
bool AllInstsEqOnly) {
17821853
// This does very trivial matching, with limited scanning, to find identical
17831854
// instructions in the two blocks. In particular, we don't want to get into
17841855
// O(N1*N2*...) situations here where Ni are the sizes of these successors. As
@@ -1807,17 +1878,35 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(Instruction *TI,
18071878
SuccIterPairs.push_back(SuccIterPair(SuccItr, 0));
18081879
}
18091880

1810-
// Check if only hoisting terminators is allowed. This does not add new
1811-
// instructions to the hoist location.
1812-
if (EqTermsOnly) {
1813-
// Skip any debug intrinsics, as they are free to hoist.
1814-
for (auto &SuccIter : make_first_range(SuccIterPairs)) {
1815-
auto *INonDbg = &*skipDebugIntrinsics(SuccIter);
1816-
if (!INonDbg->isTerminator())
1817-
return false;
1881+
if (AllInstsEqOnly) {
1882+
// Check if all instructions in the successor blocks match. This allows
1883+
// hoisting all instructions and removing the blocks we are hoisting from,
1884+
// so does not add any new instructions.
1885+
SmallVector<BasicBlock *> Succs = to_vector(successors(BB));
1886+
// Check if sizes and terminators of all successors match.
1887+
bool AllSame = none_of(Succs, [&Succs](BasicBlock *Succ) {
1888+
Instruction *Term0 = Succs[0]->getTerminator();
1889+
Instruction *Term = Succ->getTerminator();
1890+
return !Term->isSameOperationAs(Term0) ||
1891+
!equal(Term->operands(), Term0->operands()) ||
1892+
Succs[0]->size() != Succ->size();
1893+
});
1894+
if (!AllSame)
1895+
return false;
1896+
if (AllSame) {
1897+
LockstepReverseIterator LRI(Succs);
1898+
while (LRI.isValid()) {
1899+
Instruction *I0 = (*LRI)[0];
1900+
if (any_of(*LRI, [I0](Instruction *I) {
1901+
return !areIdenticalUpToCommutativity(I0, I);
1902+
})) {
1903+
return false;
1904+
}
1905+
--LRI;
1906+
}
18181907
}
1819-
// Now we know that we only need to hoist debug intrinsics and the
1820-
// terminator. Let the loop below handle those 2 cases.
1908+
// Now we know that all instructions in all successors can be hoisted. Let
1909+
// the loop below handle the hoisting.
18211910
}
18221911

18231912
// Count how many instructions were not hoisted so far. There's a limit on how
@@ -2350,81 +2439,6 @@ static void sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
23502439
}
23512440
}
23522441

2353-
namespace {
2354-
2355-
// LockstepReverseIterator - Iterates through instructions
2356-
// in a set of blocks in reverse order from the first non-terminator.
2357-
// For example (assume all blocks have size n):
2358-
// LockstepReverseIterator I([B1, B2, B3]);
2359-
// *I-- = [B1[n], B2[n], B3[n]];
2360-
// *I-- = [B1[n-1], B2[n-1], B3[n-1]];
2361-
// *I-- = [B1[n-2], B2[n-2], B3[n-2]];
2362-
// ...
2363-
class LockstepReverseIterator {
2364-
ArrayRef<BasicBlock*> Blocks;
2365-
SmallVector<Instruction*,4> Insts;
2366-
bool Fail;
2367-
2368-
public:
2369-
LockstepReverseIterator(ArrayRef<BasicBlock*> Blocks) : Blocks(Blocks) {
2370-
reset();
2371-
}
2372-
2373-
void reset() {
2374-
Fail = false;
2375-
Insts.clear();
2376-
for (auto *BB : Blocks) {
2377-
Instruction *Inst = BB->getTerminator();
2378-
for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
2379-
Inst = Inst->getPrevNode();
2380-
if (!Inst) {
2381-
// Block wasn't big enough.
2382-
Fail = true;
2383-
return;
2384-
}
2385-
Insts.push_back(Inst);
2386-
}
2387-
}
2388-
2389-
bool isValid() const {
2390-
return !Fail;
2391-
}
2392-
2393-
void operator--() {
2394-
if (Fail)
2395-
return;
2396-
for (auto *&Inst : Insts) {
2397-
for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
2398-
Inst = Inst->getPrevNode();
2399-
// Already at beginning of block.
2400-
if (!Inst) {
2401-
Fail = true;
2402-
return;
2403-
}
2404-
}
2405-
}
2406-
2407-
void operator++() {
2408-
if (Fail)
2409-
return;
2410-
for (auto *&Inst : Insts) {
2411-
for (Inst = Inst->getNextNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
2412-
Inst = Inst->getNextNode();
2413-
// Already at end of block.
2414-
if (!Inst) {
2415-
Fail = true;
2416-
return;
2417-
}
2418-
}
2419-
}
2420-
2421-
ArrayRef<Instruction*> operator * () const {
2422-
return Insts;
2423-
}
2424-
};
2425-
2426-
} // end anonymous namespace
2427-
24282442
/// Check whether BB's predecessors end with unconditional branches. If it is
24292443
/// true, sink any common code from the predecessors to BB.
24302444
static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,

llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,10 @@ define i32 @foo(i32 %i) nounwind ssp !dbg !0 {
77
; CHECK-NEXT: entry:
88
; CHECK-NEXT: #dbg_value(i32 [[I:%.*]], [[META7:![0-9]+]], !DIExpression(), [[META8:![0-9]+]])
99
; CHECK-NEXT: #dbg_value(i32 0, [[META9:![0-9]+]], !DIExpression(), [[META11:![0-9]+]])
10-
; CHECK-NEXT: [[COND:%.*]] = icmp ne i32 [[I]], 0, !dbg [[DBG12:![0-9]+]]
11-
; CHECK-NEXT: br i1 [[COND]], label [[THEN:%.*]], label [[ELSE:%.*]], !dbg [[DBG12]]
12-
; CHECK: then:
13-
; CHECK-NEXT: [[CALL_1:%.*]] = call i32 (...) @bar(), !dbg [[DBG13:![0-9]+]]
14-
; CHECK-NEXT: #dbg_value(i32 [[CALL_1]], [[META9]], !DIExpression(), [[DBG13]])
15-
; CHECK-NEXT: br label [[EXIT:%.*]], !dbg [[DBG15:![0-9]+]]
16-
; CHECK: else:
17-
; CHECK-NEXT: [[CALL_2:%.*]] = call i32 (...) @bar(), !dbg [[DBG16:![0-9]+]]
18-
; CHECK-NEXT: #dbg_value(i32 [[CALL_2]], [[META9]], !DIExpression(), [[DBG16]])
19-
; CHECK-NEXT: br label [[EXIT]], !dbg [[DBG18:![0-9]+]]
20-
; CHECK: exit:
21-
; CHECK-NEXT: [[K_0:%.*]] = phi i32 [ [[CALL_1]], [[THEN]] ], [ [[CALL_2]], [[ELSE]] ]
22-
; CHECK-NEXT: ret i32 [[K_0]], !dbg [[DBG19:![0-9]+]]
10+
; CHECK-NEXT: [[CALL_1:%.*]] = call i32 (...) @bar(), !dbg [[DBG12:![0-9]+]]
11+
; CHECK-NEXT: #dbg_value(i32 [[CALL_1]], [[META9]], !DIExpression(), [[META13:![0-9]+]])
12+
; CHECK-NEXT: #dbg_value(i32 [[CALL_1]], [[META9]], !DIExpression(), [[META15:![0-9]+]])
13+
; CHECK-NEXT: ret i32 [[CALL_1]], !dbg [[DBG17:![0-9]+]]
2314
;
2415
entry:
2516
call void @llvm.dbg.value(metadata i32 %i, metadata !6, metadata !DIExpression()), !dbg !7
@@ -46,8 +37,8 @@ define i1 @hoist_with_debug2(i32 %x) !dbg !22 {
4637
; CHECK-LABEL: @hoist_with_debug2(
4738
; CHECK-NEXT: entry:
4839
; CHECK-NEXT: [[TOBOOL_NOT:%.*]] = icmp ugt i32 [[X:%.*]], 2
49-
; CHECK-NEXT: #dbg_value(i32 [[X]], [[META21:![0-9]+]], !DIExpression(), [[META23:![0-9]+]])
50-
; CHECK-NEXT: #dbg_value(i32 [[X]], [[META21]], !DIExpression(), [[META23]])
40+
; CHECK-NEXT: #dbg_value(i32 [[X]], [[META19:![0-9]+]], !DIExpression(), [[META21:![0-9]+]])
41+
; CHECK-NEXT: #dbg_value(i32 [[X]], [[META19]], !DIExpression(), [[META21]])
5142
; CHECK-NEXT: [[DOT:%.*]] = select i1 [[TOBOOL_NOT]], i1 false, i1 true
5243
; CHECK-NEXT: ret i1 [[DOT]]
5344
;

llvm/test/Transforms/SimplifyCFG/hoisting-metadata.ll

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@ define i64 @hoist_load_with_matching_pointers_and_tbaa(i1 %c) {
88
; CHECK-NEXT: [[ENTRY:.*:]]
99
; CHECK-NEXT: [[TMP:%.*]] = alloca i64, align 8
1010
; CHECK-NEXT: call void @init(ptr [[TMP]])
11-
; CHECK-NEXT: [[TMP0:%.*]] = load i64, ptr [[TMP]], align 8
12-
; CHECK-NOT: !tbaa
13-
; CHECK-NEXT: [[TMP1:%.*]] = load i64, ptr [[TMP]], align 8
14-
; CHECK-NOT: !tbaa
15-
; CHECK-NEXT: [[P:%.*]] = select i1 [[C]], i64 [[TMP0]], i64 [[TMP1]]
11+
; CHECK-NEXT: [[P:%.*]] = load i64, ptr [[TMP]], align 8, !tbaa [[TBAA0:![0-9]+]]
1612
; CHECK-NEXT: ret i64 [[P]]
1713
;
1814
entry:
@@ -74,11 +70,7 @@ define i64 @hoist_load_with_different_tbaa(i1 %c) {
7470
; CHECK-NEXT: [[ENTRY:.*:]]
7571
; CHECK-NEXT: [[TMP:%.*]] = alloca i64, align 8
7672
; CHECK-NEXT: call void @init(ptr [[TMP]])
77-
; CHECK-NEXT: [[TMP0:%.*]] = load i64, ptr [[TMP]], align 8
78-
; CHECK-NOT: !tbaa
79-
; CHECK-NEXT: [[TMP1:%.*]] = load i64, ptr [[TMP]], align 8
80-
; CHECK-NOT: !tbaa
81-
; CHECK-NEXT: [[P:%.*]] = select i1 [[C]], i64 [[TMP0]], i64 [[TMP1]]
73+
; CHECK-NEXT: [[P:%.*]] = load i64, ptr [[TMP]], align 8, !tbaa [[TBAA5:![0-9]+]]
8274
; CHECK-NEXT: ret i64 [[P]]
8375
;
8476
entry:
@@ -135,3 +127,11 @@ exit:
135127
!3 = !{!"omnipotent char", !4, i64 0}
136128
!4 = !{!"Simple C++ TBAA"}
137129
!5 = !{!3, !3, i64 0}
130+
;.
131+
; CHECK: [[TBAA0]] = !{[[META1:![0-9]+]], [[META1]], i64 0}
132+
; CHECK: [[META1]] = !{!"p2 long long", [[META2:![0-9]+]], i64 0}
133+
; CHECK: [[META2]] = !{!"any pointer", [[META3:![0-9]+]], i64 0}
134+
; CHECK: [[META3]] = !{!"omnipotent char", [[META4:![0-9]+]], i64 0}
135+
; CHECK: [[META4]] = !{!"Simple C++ TBAA"}
136+
; CHECK: [[TBAA5]] = !{[[META3]], [[META3]], i64 0}
137+
;.

0 commit comments

Comments
 (0)