Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

JIT: Use likelihoods in block reordering decision #101132

Merged
merged 2 commits into from
Apr 17, 2024
Merged

JIT: Use likelihoods in block reordering decision #101132

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4381b9
Use likelihoods in block reordering decision
amanasifkhalid Apr 16, 2024
084b6d9
Rename + comments
amanasifkhalid Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3615,19 +3615,9 @@ bool Compiler::fgReorderBlocks(bool useProfile)
FlowEdge* edgeToBlock = bPrev->GetFalseEdge();
noway_assert(edgeToDest != nullptr);
noway_assert(edgeToBlock != nullptr);
//
// Calculate the taken ratio
// A takenRatio of 0.10 means taken 10% of the time, not taken 90% of the time
// A takenRatio of 0.50 means taken 50% of the time, not taken 50% of the time
// A takenRatio of 0.90 means taken 90% of the time, not taken 10% of the time
//
double takenCount = edgeToDest->getLikelyWeight();
double notTakenCount = edgeToBlock->getLikelyWeight();
double totalCount = takenCount + notTakenCount;

// If the takenRatio (takenCount / totalCount) is greater or equal to 51% then we will reverse
// the branch
if (takenCount < (0.51 * totalCount))
// If we take the true branch more than half the time, we will reverse the branch.
if (edgeToDest->getLikelihood() < 0.51)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename these edge vars? Something like trueEdge / falseEdge? There are some mentions up in the comment and vestiges of the old min/max there too.

Also down below this a bit, subtracting 1 is not semantically meaningful. However perhaps we should leave this for now as this logic will ultimately not be needed when we do the new block layout.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing.

{
reorderBlock = false;
}
Expand Down
Loading