[llvm] [coro][pgp] Do not insert counters in the `suspend` block (PR #71262)
David Li via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 9 10:38:07 PST 2023
================
@@ -121,31 +123,70 @@ template <class Edge, class BBInfo> class CFGMST {
static const uint32_t CriticalEdgeMultiplier = 1000;
+ auto GetCoroSuspendSwitch =
+ [&](const Instruction *TI) -> const SwitchInst * {
+ if (!F.isPresplitCoroutine())
+ return nullptr;
+ if (auto *SWInst = dyn_cast<SwitchInst>(TI))
+ if (auto *Intrinsic = dyn_cast<IntrinsicInst>(SWInst->getCondition()))
+ if (Intrinsic->getIntrinsicID() == Intrinsic::coro_suspend)
+ return SWInst;
+ return nullptr;
+ };
+
for (BasicBlock &BB : F) {
Instruction *TI = BB.getTerminator();
+ const SwitchInst *CoroSuspendSwitch = GetCoroSuspendSwitch(TI);
uint64_t BBWeight =
(BFI != nullptr ? BFI->getBlockFreq(&BB).getFrequency() : 2);
uint64_t Weight = 2;
if (int successors = TI->getNumSuccessors()) {
for (int i = 0; i != successors; ++i) {
BasicBlock *TargetBB = TI->getSuccessor(i);
- bool Critical = isCriticalEdge(TI, i);
- uint64_t scaleFactor = BBWeight;
- if (Critical) {
- if (scaleFactor < UINT64_MAX / CriticalEdgeMultiplier)
- scaleFactor *= CriticalEdgeMultiplier;
- else
- scaleFactor = UINT64_MAX;
+ const bool Critical = isCriticalEdge(TI, i);
+ const bool IsCoroSuspendTarget =
+ CoroSuspendSwitch &&
+ CoroSuspendSwitch->getDefaultDest() == TargetBB;
+ // We must not add instrumentation to the BB representing the
+ // "suspend" path, else CoroSplit won't be able to lower
+ // llvm.coro.suspend to a tail call. We do want profiling info for
+ // the other branches (resume/destroy). So we do 2 things:
+ // 1. we prefer instrumenting those other edges by setting the weight
+ // of the "suspend" edge to max, and
+ // 2. we mark the edge as "Removed" to guarantee it is not considered
+ // for instrumentation. That could technically happen:
+ // (from test/Transforms/Coroutines/coro-split-musttail.ll)
+ //
+ // %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
+ // switch i8 %suspend, label %exit [
+ // i8 0, label %await.ready
+ // i8 1, label %exit
+ // ]
+ if (IsCoroSuspendTarget) {
+ Weight = UINT64_MAX;
+ } else {
+ bool Critical = isCriticalEdge(TI, i);
+ uint64_t scaleFactor = BBWeight;
+ if (Critical) {
+ if (scaleFactor < UINT64_MAX / CriticalEdgeMultiplier)
+ scaleFactor *= CriticalEdgeMultiplier;
+ else
+ scaleFactor = UINT64_MAX;
+ }
+ if (BPI != nullptr)
+ Weight =
+ BPI->getEdgeProbability(&BB, TargetBB).scale(scaleFactor);
+ if (Weight == 0)
+ Weight++;
}
- if (BPI != nullptr)
- Weight = BPI->getEdgeProbability(&BB, TargetBB).scale(scaleFactor);
- if (Weight == 0)
- Weight++;
auto *E = &addEdge(&BB, TargetBB, Weight);
E->IsCritical = Critical;
+ // See comment above - we must guarantee the coro suspend BB isn't
+ // instrumented.
+ if (IsCoroSuspendTarget)
+ E->Removed = true;
----------------
david-xl wrote:
Is setting MAX weighted needed if the removed flag is set?
https://github.com/llvm/llvm-project/pull/71262
More information about the llvm-commits
mailing list