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.

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

Loop alignment: Fix loop size calculation to exclude IG's size that are marked as not aligned #68869

Merged
merged 8 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
31 changes: 28 additions & 3 deletions src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5096,10 +5096,14 @@ unsigned emitter::getLoopSize(insGroup* igLoopHeader, unsigned maxLoopSize DEBUG
{
unsigned loopSize = 0;

JITDUMP("*************** In getLoopSize() for %s\n", emitLabelString(igLoopHeader), loopSize);
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved

for (insGroup* igInLoop = igLoopHeader; igInLoop != nullptr; igInLoop = igInLoop->igNext)
{
loopSize += igInLoop->igSize;
if (igInLoop->endsWithAlignInstr())
JITDUMP(" %s has %u bytes.", emitLabelString(igInLoop), igInLoop->igSize);

if (igInLoop->endsWithAlignInstr() /*|| (igInLoop->igFlags & IGF_REMOVED_ALIGN)*/)
{
// If IGF_HAS_ALIGN is present, igInLoop contains align instruction at the end,
// for next IG or some future IG.
Expand Down Expand Up @@ -5179,16 +5183,35 @@ unsigned emitter::getLoopSize(insGroup* igLoopHeader, unsigned maxLoopSize DEBUG
else
#endif
{
JITDUMP(" but ends with align instruction, taking off %u bytes.",
emitComp->opts.compJitAlignPaddingLimit);
// The current loop size should exclude the align instruction size reserved for next loop.
loopSize -= emitComp->opts.compJitAlignPaddingLimit;
}
}
else if (igInLoop->igFlags & IGF_REMOVED_ALIGN)
{
assert("!Failed to remove 15 bytes");
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
}
if ((igInLoop->igLoopBackEdge == igLoopHeader) || (loopSize > maxLoopSize))
{
#ifdef DEBUG
if (igInLoop->igLoopBackEdge == igLoopHeader)
{
JITDUMP(" -- Found the back edge.");
}
else
{
JITDUMP(" -- loopSize exceeded the threshold of %u bytes.", maxLoopSize);
}
JITDUMP("\n");
break;
#endif
}
JITDUMP("\n");
}

JITDUMP("loopSize of %s = %u bytes.\n", emitLabelString(igLoopHeader), loopSize);
return loopSize;
}

Expand Down Expand Up @@ -5280,9 +5303,10 @@ void emitter::emitSetLoopBackEdge(BasicBlock* loopTopBlock)

// This IG should no longer contain alignment instruction
alignInstr->removeAlignFlags();
alignInstr->idaIG->igFlags |= IGF_REMOVED_ALIGN;

markedCurrLoop = true;
JITDUMP("** Skip alignment for current loop IG%02u ~ IG%02u because it encloses an aligned loop "
JITDUMP(";; Skip alignment for current loop IG%02u ~ IG%02u because it encloses an aligned loop "
"IG%02u ~ IG%02u.\n",
currLoopStart, currLoopEnd, emitLastLoopStart, emitLastLoopEnd);
}
Expand All @@ -5296,9 +5320,10 @@ void emitter::emitSetLoopBackEdge(BasicBlock* loopTopBlock)

// This IG should no longer contain alignment instruction
alignInstr->removeAlignFlags();
alignInstr->idaIG->igFlags |= IGF_REMOVED_ALIGN;

markedLastLoop = true;
JITDUMP("** Skip alignment for aligned loop IG%02u ~ IG%02u because it encloses the current loop "
JITDUMP(";; Skip alignment for aligned loop IG%02u ~ IG%02u because it encloses the current loop "
"IG%02u ~ IG%02u.\n",
emitLastLoopStart, emitLastLoopEnd, currLoopStart, currLoopEnd);
}
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ struct insGroup
// and the emitter should continue to track GC info as if there was no new block.
#define IGF_HAS_ALIGN 0x0400 // this group contains an alignment instruction(s) at the end to align either the next
// IG, or, if this IG contains with an unconditional branch, some subsequent IG.
#define IGF_REMOVED_ALIGN 0x0800 // this group had an alignment instruction(s) which was later removed without updating
// the IG's size/offsets.

// Mask of IGF_* flags that should be propagated to new blocks when they are created.
// This allows prologs and epilogs to be any number of IGs, but still be
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2625,7 +2625,7 @@ void Compiler::optIdentifyLoopsForAlignment()
}
else
{
JITDUMP("Skip alignment for " FMT_LP " that starts at " FMT_BB " weight=" FMT_WT ".\n", loopInd,
JITDUMP(";; Skip alignment for " FMT_LP " that starts at " FMT_BB " weight=" FMT_WT ".\n", loopInd,
top->bbNum, topWeight);
}
}
Expand Down