Skip to content

Commit ac28efa

Browse files
[SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches.
- Before this patch, loop metadata (if exists) will override the metadata of each predecessor; if the predecessor block already has loop metadata, the orignal loop metadata won't be preserved and could cause missed loop transformations (see 'test2' in llvm/test/Transforms/SimplifyCFG/preserve-llvm-loop-metadata.ll). To illustrate how inner-loop metadata might be dropped before this patch: CFG Before entry | v ---> while.cond -------------> while.end | | | v | while.body | | | v | for.body <---- (md1) | | |______| | v | while.cond.exit (md2) | | |_______| CFG After entry | v ---> while.cond.rewrite -------------> while.end | | | v | while.body | | | v | for.body <---- (md2) |_______| |______| Basically, when 'while.cond.exit' is folded into 'while.cond', 'md2' overrides 'md1' and 'md1' is dropped from the CFG. Differential Revision: https://reviews.llvm.org/D134152
1 parent 4d06861 commit ac28efa

File tree

2 files changed

+81
-4
lines changed

2 files changed

+81
-4
lines changed

llvm/lib/Transforms/Utils/Local.cpp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,80 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
10901090
}
10911091
}
10921092

1093+
// 'BB' and 'BB->Pred' are loop latches, bail out to presrve inner loop
1094+
// metadata.
1095+
//
1096+
// FIXME: This is a stop-gap solution to preserve inner-loop metadata given
1097+
// current status (that loop metadata is implemented as metadata attached to
1098+
// the branch instruction in the loop latch block). To quote from review
1099+
// comments, "the current representation of loop metadata (using a loop latch
1100+
// terminator attachment) is known to be fundamentally broken. Loop latches
1101+
// are not uniquely associated with loops (both in that a latch can be part of
1102+
// multiple loops and a loop may have multiple latches). Loop headers are. The
1103+
// solution to this problem is also known: Add support for basic block
1104+
// metadata, and attach loop metadata to the loop header."
1105+
//
1106+
// Why bail out:
1107+
// In this case, we expect 'BB' is the latch for outer-loop and 'BB->Pred' is
1108+
// the latch for inner-loop (see reason below), so bail out to prerserve
1109+
// inner-loop metadata rather than eliminating 'BB' and attaching its metadata
1110+
// to this inner-loop.
1111+
// - The reason we believe 'BB' and 'BB->Pred' have different inner-most
1112+
// loops: assuming 'BB' and 'BB->Pred' are from the same inner-most loop L,
1113+
// then 'BB' is the header and latch of 'L' and thereby 'L' must consist of
1114+
// one self-looping basic block, which is contradictory with the assumption.
1115+
//
1116+
// To illustrate how inner-loop metadata is dropped:
1117+
//
1118+
// CFG Before
1119+
//
1120+
// BB is while.cond.exit, attached with loop metdata md2.
1121+
// BB->Pred is for.body, attached with loop metadata md1.
1122+
//
1123+
// entry
1124+
// |
1125+
// v
1126+
// ---> while.cond -------------> while.end
1127+
// | |
1128+
// | v
1129+
// | while.body
1130+
// | |
1131+
// | v
1132+
// | for.body <---- (md1)
1133+
// | | |______|
1134+
// | v
1135+
// | while.cond.exit (md2)
1136+
// | |
1137+
// |_______|
1138+
//
1139+
// CFG After
1140+
//
1141+
// while.cond1 is the merge of while.cond.exit and while.cond above.
1142+
// for.body is attached with md2, and md1 is dropped.
1143+
// If LoopSimplify runs later (as a part of loop pass), it could create
1144+
// dedicated exits for inner-loop (essentially adding `while.cond.exit`
1145+
// back), but won't it won't see 'md1' nor restore it for the inner-loop.
1146+
//
1147+
// entry
1148+
// |
1149+
// v
1150+
// ---> while.cond1 -------------> while.end
1151+
// | |
1152+
// | v
1153+
// | while.body
1154+
// | |
1155+
// | v
1156+
// | for.body <---- (md2)
1157+
// |_______| |______|
1158+
if (Instruction *TI = BB->getTerminator())
1159+
if (MDNode *LoopMD = TI->getMetadata(LLVMContext::MD_loop)) {
1160+
for (BasicBlock *Pred : predecessors(BB)) {
1161+
if (Instruction *PredTI = Pred->getTerminator())
1162+
if (MDNode *PredLoopMD = PredTI->getMetadata(LLVMContext::MD_loop))
1163+
return false;
1164+
}
1165+
}
1166+
10931167
LLVM_DEBUG(dbgs() << "Killing Trivial BB: \n" << *BB);
10941168

10951169
SmallVector<DominatorTree::UpdateType, 32> Updates;

llvm/test/Transforms/SimplifyCFG/preserve-llvm-loop-metadata.ll

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ while.end: ; preds = %while.cond
4646
ret void
4747
}
4848

49-
; The test case is constructed based on the following C++ code,
50-
; as a simplified test case to show why `llvm.loop.unroll.enable`
51-
; could be dropped.
49+
; Test that empty loop latch `while.cond.loopexit` will not be folded into its successor if its
50+
; predecessor blocks are also loop latches.
5251
;
52+
; The test case is constructed based on the following C++ code.
5353
; While the C++ code itself might have the inner-loop unrolled (e.g., with -O3),
5454
; the loss of inner-loop unroll metadata is a bug.
5555
; Under some optimization pipelines (e.g., FullLoopUnroll pass is skipped in ThinLTO prelink stage),
@@ -111,4 +111,7 @@ while.end: ; preds = %while.cond
111111
!5 = !{!"llvm.loop.unroll.enable"}
112112
; CHECK: !0 = distinct !{!0, !1}
113113
; CHECK: !1 = !{!"llvm.loop.distribute.enable", i1 true}
114-
; CHECK-NOT: !{!"llvm.loop.unroll.enable"}
114+
; CHECK: !2 = distinct !{!2, !3}
115+
; CHECK: !3 = !{!"llvm.loop.mustprogress"}
116+
; CHECK: !4 = distinct !{!4, !3, !5}
117+
; CHECK: !5 = !{!"llvm.loop.unroll.enable"}

0 commit comments

Comments
 (0)