Page MenuHomePhabricator

D62860.diff
No OneTemporary

File Metadata

Created
Wed, Dec 11, 9:04 AM

D62860.diff

Index: lib/Transforms/Scalar/GuardWidening.cpp
===================================================================
--- lib/Transforms/Scalar/GuardWidening.cpp
+++ lib/Transforms/Scalar/GuardWidening.cpp
@@ -88,6 +88,13 @@
"expressed as branches by widenable conditions"),
cl::init(true));
+static cl::opt<bool>
+ AggressivelyHoist("guard-widening-aggressively-hoist", cl::Hidden,
+ cl::desc("Should guards be hoisted out of loops "
+ "(whether they're known to execute or not)"),
+ cl::init(true));
+
+
namespace {
// Get the condition of \p I. It can either be a guard or a conditional branch.
@@ -306,16 +313,77 @@
return false;
}
+/// If it is legal to hoist a guard out of a loop into the preheader, do so.
+/// The assumption is that it is always profitable to do so and that backoff
+/// (i.e. unprofitable transforms triggering recompiles w/decreased
+/// optimization aggressiveness) are handled externally. In particular, this
+/// may introduce deoptimizations which would have never happened in the
+/// original program.
+static bool hoistGuardToPreheader(ICFLoopSafetyInfo &SI,
+ Instruction &Guard, Loop *L) {
+ // TODO: implement a similiar transform for widenable_conditions
+ assert(isGuard(&Guard));
+
+ BasicBlock *Preheader = L->getLoopPreheader();
+ if (!Preheader)
+ return false;
+
+ // TODO: enhancement ideas:
+ // *) Hoist out of outer most loop we can establish legality for, not simply
+ // the innermost one as we do now.
+ // *) Limit the transform to cases where BPI tells us the block is likely to
+ // run, and thus we're not widening "too much". (Possibly rephrase this
+ // as unconditional splitting followed by conditional widening?)
+
+ bool SplitGuard = !L->isLoopInvariant(Guard.getOperand(0));
+ for (unsigned i = 1; i < Guard.getNumOperands(); i++)
+ if (!L->isLoopInvariant(Guard.getOperand(i)))
+ return false;
+ if (!SI.doesNotWriteMemoryBefore(Guard, L))
+ return false;
+ if (SplitGuard) {
+ auto *NewGuard = Guard.clone();
+ auto *True = ConstantInt::getTrue(Guard.getOperand(0)->getType());
+ NewGuard->setOperand(0, True);
+ SI.insertInstructionTo(NewGuard, Preheader);
+ NewGuard->insertBefore(Preheader->getTerminator());
+ } else {
+ SI.removeInstruction(&Guard);
+ SI.insertInstructionTo(&Guard, Preheader);
+ Guard.moveBefore(Preheader->getTerminator());
+ }
+ return true;
+}
+
bool GuardWideningImpl::run() {
- DenseMap<BasicBlock *, SmallVector<Instruction *, 8>> GuardsInBlock;
bool Changed = false;
+
+ if (AggressivelyHoist) {
+ // As a first pass, hoist any guards out of loops that we can. Doing this
+ // before the second pass ensures that we can merge them after hoisting.
+ ICFLoopSafetyInfo SI(&DT);
+ for (auto DFI = df_begin(Root), DFE = df_end(Root);
+ DFI != DFE; ++DFI) {
+ auto *BB = (*DFI)->getBlock();
+ if (!BlockFilter(BB))
+ continue;
+
+ Loop *L = LI.getLoopFor(BB);
+ for (auto Itr = BB->begin(); Itr != BB->end();) {
+ Instruction &I = *(Itr);
+ ++Itr;
+ if (isGuard(&I) && L && hoistGuardToPreheader(SI, I, L))
+ Changed = true;
+ }
+ }
+ }
+ DenseMap<BasicBlock *, SmallVector<Instruction *, 8>> GuardsInBlock;
Optional<BranchProbability> LikelyTaken = None;
if (WidenFrequentBranches && BPI) {
unsigned Threshold = FrequentBranchThreshold;
assert(Threshold > 0 && "Zero threshold makes no sense!");
LikelyTaken = BranchProbability(Threshold - 1, Threshold);
}
-
for (auto DFI = df_begin(Root), DFE = df_end(Root);
DFI != DFE; ++DFI) {
auto *BB = (*DFI)->getBlock();
@@ -327,9 +395,10 @@
for (auto &I : *BB)
if (isSupportedGuardInstruction(&I))
CurrentList.push_back(cast<Instruction>(&I));
-
- for (auto *II : CurrentList)
- Changed |= eliminateInstrViaWidening(II, DFI, GuardsInBlock);
+
+ for (auto *I : CurrentList)
+ Changed |= eliminateInstrViaWidening(I, DFI, GuardsInBlock);
+
if (WidenFrequentBranches && BPI)
if (auto *BI = dyn_cast<BranchInst>(BB->getTerminator()))
if (BI->isConditional()) {

Event Timeline