Skip to content

JIT: Align newly recognized loops #96756

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

Merged
jakobbotsch merged 5 commits into from
Jan 12, 2024
Merged

JIT: Align newly recognized loops #96756

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e606a7f
JIT: Align newly recognized loops
jakobbotsch Jan 10, 2024
6f395da
Nit
jakobbotsch Jan 10, 2024
ad5c1aa
Merge branch 'main' into align-new-loops
jakobbotsch Jan 10, 2024
8046426
Update superpmi.py
jakobbotsch Jan 11, 2024
7e1b38b
Merge branch 'main' into align-new-loops
jakobbotsch Jan 11, 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
26 changes: 1 addition & 25 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5231,21 +5231,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl

#if FEATURE_LOOP_ALIGN

// TODO-Quirk: Remove
static bool HasOldChildLoop(Compiler* comp, FlowGraphNaturalLoop* loop)
{
for (FlowGraphNaturalLoop* child = loop->GetChild(); child != nullptr; child = child->GetSibling())
{
if (child->GetHeader()->HasFlag(BBF_OLD_LOOP_HEADER_QUIRK))
return true;

if (HasOldChildLoop(comp, child))
return true;
}

return false;
}

//------------------------------------------------------------------------
// shouldAlignLoop: Check if it is legal and profitable to align a loop.
//
Expand All @@ -5265,16 +5250,7 @@ static bool HasOldChildLoop(Compiler* comp, FlowGraphNaturalLoop* loop)
//
bool Compiler::shouldAlignLoop(FlowGraphNaturalLoop* loop, BasicBlock* top)
{
// TODO-Quirk: Remove. When removing we will likely need to add some
// form of "lexicality" heuristic here: only align loops whose blocks
// are fairly tightly packed together physically.
Comment on lines -5268 to -5270
Copy link
Member Author

@jakobbotsch jakobbotsch Jan 10, 2024

Choose a reason for hiding this comment

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

I decided not to look at this here; in particular because we have a lot of work in flight that is severely changing the flow graph, so it's hard to get a good feeling for how this heuristic should eventually look.

if (!loop->GetHeader()->HasFlag(BBF_OLD_LOOP_HEADER_QUIRK))
{
return false;
}

// TODO-Quirk: Switch to loop->GetChild() != nullptr
if (HasOldChildLoop(this, loop))
if (loop->GetChild() != nullptr)
{
JITDUMP("Skipping alignment for " FMT_LP "; not an innermost loop\n", loop->GetIndex());
return false;
Expand Down