[coro][pgo] Do not insert counters in the suspend block
#71262
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,8 @@ | |
| #include "llvm/Analysis/BlockFrequencyInfo.h" | ||
| #include "llvm/Analysis/BranchProbabilityInfo.h" | ||
| #include "llvm/Analysis/CFG.h" | ||
| #include "llvm/IR/Instructions.h" | ||
| #include "llvm/IR/IntrinsicInst.h" | ||
| #include "llvm/Support/BranchProbability.h" | ||
| #include "llvm/Support/Debug.h" | ||
| #include "llvm/Support/raw_ostream.h" | ||
|
|
@@ -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); | ||
| 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++; | ||
| } | ||
| 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; | ||
|
There was a problem hiding this comment. Is setting MAX weighted needed if the removed flag is set? There was a problem hiding this comment. IIUC yes (as per @xur-llvm) because it helps pick the other edges for instrumentation. If there's a more explicit way to do that, I'd prefer that instead. There was a problem hiding this comment. I take that back. Fixing. |
||
| LLVM_DEBUG(dbgs() << " Edge: from " << BB.getName() << " to " | ||
| << TargetBB->getName() << " w=" << Weight << "\n"); | ||
| // Keep track of entry/exit edges: | ||
| if (&BB == Entry) { | ||
| if (Weight > MaxEntryOutWeight) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest move all co-routine handling code in one helper function and call it after Edge creation:
void HandleCoroutine(Edge *E, ....) {
if (!F.notPresplitCoroutine)
return;
if (!NotSwitch(SI))
return;
...
E->Weight = UINT64_MAX;
E->Removed = true;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done