[llvm] r250343 - [SimplifyCFG] Speculatively flatten CFG based on profiling metadata
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 15 09:56:36 PDT 2015
I've reverted it for another reason before seeing this, see 250430.
In the future, please feel free to preemptively revert if you encounter
self host problems. I can reapply if needed; getting the bots green is
the important thing.
Philip
On 10/15/2015 08:35 AM, Manman Ren wrote:
> Hi Philip,
>
> Is it okay to temporarily revert r250343? Is it possible that this commit can cause the following issue?
>
> With r250345 and r250343 (a few other ones that seem unrelated), we start to observe the following failure when bootstrap clang with lto and pgo:
>
> PHI node entries do not match predecessors!
> %.sroa.029.3.i = phi %"class.llvm::SDNode.13298"* [ null, %30953 ], [ null, %31017 ], [ null, %30998 ], [ null, %_ZN4llvm8dyn_castINS_14ConstantSDNodeENS_7SDValueEEENS_10cast_rettyIT_T0_E8ret_typeERS5_.exit.i.1804 ], [ null, %30975 ], [ null, %30991 ], [ null, %_ZNK4llvm3EVT13getScalarTypeEv.exit.i.1812 ], [ %..sroa.029.0.i, %_ZN4llvm11SmallVectorIiLj8EED1Ev.exit.i.1826 ], !dbg !451895
> label %30998
> label %_ZNK4llvm3EVTeqES0_.exit19.thread.i
> LLVM ERROR: Broken function found, compilation aborted!
>
> I reverted r250345 earlier, but our bot didn’t recover.
>
> Thanks,
> Manman
>
>> On Oct 14, 2015, at 3:46 PM, Philip Reames via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: reames
>> Date: Wed Oct 14 17:46:19 2015
>> New Revision: 250343
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=250343&view=rev
>> Log:
>> [SimplifyCFG] Speculatively flatten CFG based on profiling metadata
>>
>> If we have a series of branches which are all unlikely to fail, we can possibly combine them into a single check on the fastpath combined with a bit of dispatch logic on the slowpath. We don't want to do this unconditionally since it requires speculating instructions past a branch, but if the profiling metadata on the branch indicates profitability, this can reduce the number of checks needed along the fast path.
>>
>> The canonical example this is trying to handle is removing the second bounds check implied by the Java code: a[i] + a[i+1]. Note that it can currently only do so for really simple conditions and the values of a[i] can't be used anywhere except in the addition. (i.e. the load has to have been sunk already and not prevent speculation.) I plan on extending this transform over the next few days to handle alternate sequences.
>>
>> Differential Revision: http://reviews.llvm.org/D13070
>>
>>
>> Modified:
>> llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=250343&r1=250342&r2=250343&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Wed Oct 14 17:46:19 2015
>> @@ -73,6 +73,17 @@ static cl::opt<bool> HoistCondStores(
>> "simplifycfg-hoist-cond-stores", cl::Hidden, cl::init(true),
>> cl::desc("Hoist conditional stores if an unconditional store precedes"));
>>
>> +static cl::opt<unsigned> SpeculativeFlattenBias(
>> + "speculative-flatten-bias", cl::Hidden, cl::init(100),
>> + cl::desc("Control how biased a branch needs to be to be considered rarely"
>> + " failing for speculative flattening (default = 100)"));
>> +
>> +static cl::opt<unsigned> SpeculativeFlattenThreshold(
>> + "speculative-flatten-threshold", cl::Hidden, cl::init(10),
>> + cl::desc("Control how much speculation happens due to speculative"
>> + " flattening (default = 10)"));
>> +
>> +
>> STATISTIC(NumBitMaps, "Number of switch instructions turned into bitmaps");
>> STATISTIC(NumLinearMaps, "Number of switch instructions turned into linear mapping");
>> STATISTIC(NumLookupTables, "Number of switch instructions turned into lookup tables");
>> @@ -2332,11 +2343,115 @@ bool llvm::FoldBranchToCommonDest(Branch
>> return false;
>> }
>>
>> +
>> +/// Return true if B is known to be implied by A. A & B must be i1 (boolean)
>> +static bool implies(Value *A, Value *B, const DataLayout &DL) {
>> + assert(A->getType()->isIntegerTy(1) && B->getType()->isIntegerTy(1));
>> + // Note that the truth table for implication is the same as <=u on i1
>> + // values.
>> + Value *Simplified = SimplifyICmpInst(ICmpInst::ICMP_ULE, A, B, DL);
>> + Constant *Con = dyn_cast_or_null<Constant>(Simplified);
>> + return Con && Con->isOneValue();
>> +}
>> +
>> +/// If we have a series of tests leading to a frequently executed fallthrough
>> +/// path and we can test all the conditions at once, we can execute a single
>> +/// test on the fast path and figure out which condition failed on the slow
>> +/// path. This transformation considers a pair of branches at a time since
>> +/// recursively considering branches pairwise will cause an entire chain to
>> +/// collapse. This transformation is code size neutral, but makes it
>> +/// dynamically more expensive to fail either check. As such, we only want to
>> +/// do this if both checks are expected to essentially never fail.
>> +/// The key motivating examples are cases like:
>> +/// br (i < Length), in_bounds, fail1
>> +/// in_bounds:
>> +/// br (i+1 < Length), in_bounds2, fail2
>> +/// in_bounds2:
>> +/// ...
>> +///
>> +/// We can rewrite this as:
>> +/// br (i+1 < Length), in_bounds2, dispatch
>> +/// in_bounds2:
>> +/// ...
>> +/// dispatch:
>> +/// br (i < Length), fail2, fail1
>> +///
>> +/// TODO: we could consider duplicating some (non-speculatable) instructions
>> +/// from BI->getParent() down both paths
>> +static bool SpeculativelyFlattenCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
>> + const DataLayout &DL) {
>> + auto *PredBB = PBI->getParent();
>> + auto *BB = BI->getParent();
>> +
>> + /// Is the failing path of this branch taken rarely if at all?
>> + auto isRarelyUntaken = [](BranchInst *BI) {
>> + uint64_t ProbTrue;
>> + uint64_t ProbFalse;
>> + if (!ExtractBranchMetadata(BI, ProbTrue, ProbFalse))
>> + return false;
>> + return ProbTrue > ProbFalse * SpeculativeFlattenBias;
>> + };
>> +
>> + if (PBI->getSuccessor(0) != BB ||
>> + !isRarelyUntaken(PBI) || !isRarelyUntaken(BI) ||
>> + !implies(BI->getCondition(), PBI->getCondition(), DL))
>> + return false;
>> +
>> + // TODO: The following code performs a similiar, but slightly distinct
>> + // transformation to that done by SpeculativelyExecuteBB. We should consider
>> + // combining them at some point.
>> +
>> + // Can we speculate everything in the given block (except for the terminator
>> + // instruction) at the instruction boundary before 'At'?
>> + unsigned SpeculationCost = 0;
>> + for (Instruction &I : *BB) {
>> + if (isa<TerminatorInst>(I)) break;
>> + if (!isSafeToSpeculativelyExecute(&I, PBI))
>> + return false;
>> + SpeculationCost++;
>> + // Only flatten relatively small BBs to avoid making the bad case terrible
>> + if (SpeculationCost > SpeculativeFlattenThreshold || isa<CallInst>(I))
>> + return false;
>> + }
>> +
>> + DEBUG(dbgs() << "Outlining slow path comparison: "
>> + << *PBI->getCondition() << " implied by "
>> + << *BI->getCondition() << "\n");
>> + // See the example in the function comment.
>> + Value *WhichCond = PBI->getCondition();
>> + auto *Success = BI->getSuccessor(0);
>> + auto *FailPBI = PBI->getSuccessor(1);
>> + auto *FailBI = BI->getSuccessor(1);
>> + // Have PBI branch directly to the fast path using BI's condition, branch
>> + // to this BI's block for the slow path dispatch
>> + PBI->setSuccessor(0, Success);
>> + PBI->setSuccessor(1, BB);
>> + PBI->setCondition(BI->getCondition());
>> + // Rewrite BI to distinguish between the two failing cases
>> + BI->setSuccessor(0, FailBI);
>> + BI->setSuccessor(1, FailPBI);
>> + BI->setCondition(WhichCond);
>> + // Move all of the instructions from BI->getParent other than
>> + // the terminator into the fastpath. This requires speculating them past
>> + // the original PBI branch, but we checked for that legality above.
>> + // TODO: This doesn't handle dependent loads. We could duplicate those
>> + // down both paths, but that involves further code growth. We need to
>> + // figure out a good cost model here.
>> + PredBB->getInstList().splice(PBI, BB->getInstList(),
>> + BB->begin(), std::prev(BB->end()));
>> +
>> + // To be conservatively correct, drop all metadata on the rewritten
>> + // branches. TODO: update metadata
>> + PBI->dropUnknownNonDebugMetadata();
>> + BI->dropUnknownNonDebugMetadata();
>> + return true;
>> +}
>> /// If we have a conditional branch as a predecessor of another block,
>> /// this function tries to simplify it. We know
>> /// that PBI and BI are both conditional branches, and BI is in one of the
>> /// successor blocks of PBI - PBI branches to BI.
>> -static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI) {
>> +static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
>> + const DataLayout &DL) {
>> assert(PBI->isConditional() && BI->isConditional());
>> BasicBlock *BB = BI->getParent();
>>
>> @@ -2385,6 +2500,13 @@ static bool SimplifyCondBranchToCondBran
>> }
>> }
>>
>> + if (auto *CE = dyn_cast<ConstantExpr>(BI->getCondition()))
>> + if (CE->canTrap())
>> + return false;
>> +
>> + if (SpeculativelyFlattenCondBranchToCondBranch(PBI, BI, DL))
>> + return true;
>> +
>> // If this is a conditional branch in an empty block, and if any
>> // predecessors are a conditional branch to one of our destinations,
>> // fold the conditions into logical ops and one cond br.
>> @@ -2395,11 +2517,6 @@ static bool SimplifyCondBranchToCondBran
>> if (&*BBI != BI)
>> return false;
>>
>> -
>> - if (ConstantExpr *CE = dyn_cast<ConstantExpr>(BI->getCondition()))
>> - if (CE->canTrap())
>> - return false;
>> -
>> int PBIOp, BIOp;
>> if (PBI->getSuccessor(0) == BI->getSuccessor(0))
>> PBIOp = BIOp = 0;
>> @@ -4652,7 +4769,7 @@ bool SimplifyCFGOpt::SimplifyCondBranch(
>> for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI)
>> if (BranchInst *PBI = dyn_cast<BranchInst>((*PI)->getTerminator()))
>> if (PBI != BI && PBI->isConditional())
>> - if (SimplifyCondBranchToCondBranch(PBI, BI))
>> + if (SimplifyCondBranchToCondBranch(PBI, BI, DL))
>> return SimplifyCFG(BB, TTI, BonusInstThreshold, AC) | true;
>>
>> return false;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list