Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3244072
Hide align behind a jmp
kunalspathak Oct 8, 2021
e7c0710
Fix a problem where curIG==0 and loop might be emitted in curIG, adju…
kunalspathak Oct 22, 2021
bd922aa
Add stress mode to emit int3 for xarch
kunalspathak Oct 22, 2021
4d0f912
Add stress mode to emit bkpt for arm64
kunalspathak Oct 22, 2021
8d64351
Add a loop align instruction placement phase
kunalspathak Oct 29, 2021
9b9b616
review comments
kunalspathak Oct 29, 2021
6302975
Change from unsigned short to unsigned
kunalspathak Oct 29, 2021
d20da6d
review comments around cleanup
kunalspathak Nov 10, 2021
c6a2d70
emitForceNewIG
kunalspathak Nov 10, 2021
e9c5eec
Remove emitPrevIG
kunalspathak Nov 10, 2021
c1c5db3
Revert change to forceNewIG for align instruction
kunalspathak Nov 10, 2021
b8a9742
Use loopAlignCandidates
kunalspathak Nov 11, 2021
db98ec2
Use loopHeadIG reference
kunalspathak Nov 11, 2021
5ab9edc
jit format
kunalspathak Nov 11, 2021
c8a9e01
Remove unneeded method
kunalspathak Nov 11, 2021
5bb1563
Misc changes
kunalspathak Nov 11, 2021
2c6e81d
Review feedback
kunalspathak Nov 12, 2021
bbc2ac5
Do not include align behind Jmp in PerfScore calculation
kunalspathak Nov 13, 2021
64bba41
jit format and fix a bug
kunalspathak Nov 15, 2021
1e24fcb
fix the loopCandidates == 0 scenario
kunalspathak Nov 15, 2021
b301fa5
Add unmarkLoopAlign(), add check for fgFirstBB
kunalspathak Nov 16, 2021
57759d0
merge conflict fix
kunalspathak Nov 16, 2021
ef0e859
Add missing }
kunalspathak Nov 16, 2021
976b253
Grammar nit
kunalspathak Nov 18, 2021
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
Prev Previous commit
Next Next commit
Use loopHeadIG reference
  • Loading branch information
kunalspathak committed Nov 16, 2021
commit db98ec2f30e0d020855f2f8c8c0da496da28aeb3
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ void CodeGen::genCodeForBBlist()
{
// The current IG is the one that is just before the IG having loop start.
// Establish a connection of recent align instruction emitted to the loop
// it actually is aligning using 'idaTargetIG'.
// it actually is aligning using 'idaLoopHeadPredIG'.
GetEmitter()->emitConnectAlignInstrWithCurIG();
}
}
Expand Down
61 changes: 33 additions & 28 deletions src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4864,14 +4864,14 @@ void emitter::emitLoopAlign(unsigned paddingBytes, bool isFirstAlign)

if (isFirstAlign)
{
// For multiple align instructions, set the targetIG only for the
// For multiple align instructions, set the idaLoopHeadPredIG only for the
// first align instruction
id->idaTargetIG = emitCurIG;
id->idaLoopHeadPredIG = emitCurIG;
emitAlignLastGroup = id;
}
else
{
id->idaTargetIG = nullptr;
id->idaLoopHeadPredIG = nullptr;
}

/* Append this instruction to this IG's alignment list */
Expand Down Expand Up @@ -4929,20 +4929,23 @@ void emitter::emitLongLoopAlign(unsigned alignmentBoundary)

//-----------------------------------------------------------------------------
// emitConnectAlignInstrWithCurIG: If "align" instruction is not just before the loop start,
// setting idaTargetIG lets us know the exact IG that the "align"
// instruction is trying to align. This is used during loop size
// calculation.
// setting idaLoopHeadPredIG lets us know the exact IG that the "align"
// instruction is trying to align. This is used to track the last IG that
// needs alignment after which VEX encoding optimization is enabled.
//
// TODO: Once over-estimation problem is solved, consider replacing
// idaLoopHeadPredIG with idaLoopHeadIG itself.
//
void emitter::emitConnectAlignInstrWithCurIG()
{
JITDUMP("Mapping 'align' instruction in IG%02u to target IG%02u\n", emitAlignLastGroup->idaIG->igNum,
emitCurIG->igNum);
// Since we never align overlapping instructions, it is always guaranteed that
// the emitRecentFirstAlign points to the loop that is in process of getting aligned.
// the emitAlignLastGroup points to the loop that is in process of getting aligned.

emitAlignLastGroup->idaTargetIG = emitCurIG;
emitAlignLastGroup->idaLoopHeadPredIG = emitCurIG;

// For a new IG to ensure that loop doesn't start from IG that idaTargetIG points to.
// For a new IG to ensure that loop doesn't start from IG that idaLoopHeadPredIG points to.
emitNxtIG();
}

Expand Down Expand Up @@ -5192,7 +5195,7 @@ void emitter::emitSetLoopBackEdge(BasicBlock* loopTopBlock)
{
// Find the IG that has 'align' instruction to align the current loop
// and clear the IGF_HAS_ALIGN flag.
if (!alignCurrentLoop && (alignInstr->idaTargetIG->igNext == dstIG))
if (!alignCurrentLoop && (alignInstr->loopHeadIG() == dstIG))
{
assert(!markedCurrLoop);

Expand All @@ -5207,8 +5210,8 @@ void emitter::emitSetLoopBackEdge(BasicBlock* loopTopBlock)

// Find the IG that has 'align' instruction to align the last loop
// and clear the IGF_HAS_ALIGN flag.
if (!alignLastLoop && (alignInstr->idaTargetIG->igNext != nullptr) &&
(alignInstr->idaTargetIG->igNext->igNum == emitLastLoopStart))
if (!alignLastLoop && (alignInstr->loopHeadIG() != nullptr) &&
(alignInstr->loopHeadIG()->igNum == emitLastLoopStart))
{
assert(!markedLastLoop);
assert(alignInstr->idaIG->endsWithAlignInstr());
Expand Down Expand Up @@ -5270,16 +5273,17 @@ void emitter::emitLoopAlignAdjustments()
{
assert(alignInstr->idIns() == INS_align);

insGroup* alignIG = alignInstr->idaTargetIG;
insGroup* loopHeadPredIG = alignInstr->idaLoopHeadPredIG;
insGroup* loopHeadIG = alignInstr->loopHeadIG();
insGroup* containingIG = alignInstr->idaIG;

JITDUMP(" Adjusting 'align' instruction in IG%02u that is targeted for IG%02u \n", containingIG->igNum,
alignIG->igNum);
loopHeadIG->igNum);

// Since we only adjust the padding up to the next align instruction which is behind the jump, we make sure
// that we take into account all the alignBytes we removed until that point. Hence " - alignBytesRemoved"

loopIGOffset = alignIG->igNext->igOffs - alignBytesRemoved;
loopIGOffset = loopHeadIG->igOffs - alignBytesRemoved;

// igSize also includes INS_align instruction, take it off.
loopIGOffset -= estimatedPaddingNeeded;
Expand All @@ -5289,7 +5293,7 @@ void emitter::emitLoopAlignAdjustments()

unsigned actualPaddingNeeded =
containingIG->endsWithAlignInstr()
? emitCalculatePaddingForLoopAlignment(alignIG, loopIGOffset DEBUG_ARG(false))
? emitCalculatePaddingForLoopAlignment(loopHeadIG, loopIGOffset DEBUG_ARG(false))
: 0;

assert(estimatedPaddingNeeded >= actualPaddingNeeded);
Expand Down Expand Up @@ -5358,7 +5362,7 @@ void emitter::emitLoopAlignAdjustments()
assert(instrAdjusted == 0);
}

JITDUMP("Adjusted alignment of %s from %u to %u.\n", emitLabelString(alignIG), estimatedPaddingNeeded,
JITDUMP("Adjusted alignment for %s from %u to %u.\n", emitLabelString(loopHeadIG), estimatedPaddingNeeded,
actualPaddingNeeded);
JITDUMP("Adjusted size of %s from %u to %u.\n", emitLabelString(containingIG),
(containingIG->igSize + diff), containingIG->igSize);
Expand All @@ -5383,7 +5387,8 @@ void emitter::emitLoopAlignAdjustments()
{
// Record the last loop IG that will be aligned. No overestimation
// adjustment will be done after emitLastAlignedIgNum.
emitLastAlignedIgNum = alignIG->igNum;
JITDUMP("Recording last aligned IG: %s\n", emitLabelString(loopHeadPredIG));
emitLastAlignedIgNum = loopHeadPredIG->igNum;
}
}

Expand All @@ -5397,7 +5402,7 @@ void emitter::emitLoopAlignAdjustments()
// end of 'ig' so the loop that starts after 'ig' is aligned.
//
// Arguments:
// ig - The IG before the IG that has the loop head that need to be aligned.
// loopHeadIG - The IG that has the loop head that need to be aligned.
// offset - The offset at which the IG that follows 'ig' starts.
// isAlignAdjusted - Determine if adjustments are done to the align instructions or not.
// During generating code, it is 'false' (because we haven't adjusted the size yet).
Expand Down Expand Up @@ -5425,14 +5430,14 @@ void emitter::emitLoopAlignAdjustments()
// 3b. If the loop already fits in minimum alignmentBoundary blocks, then return 0. // already best aligned
// 3c. return paddingNeeded.
//
unsigned emitter::emitCalculatePaddingForLoopAlignment(insGroup* ig, size_t offset DEBUG_ARG(bool isAlignAdjusted))
unsigned emitter::emitCalculatePaddingForLoopAlignment(insGroup* loopHeadIG, size_t offset DEBUG_ARG(bool isAlignAdjusted))
{
unsigned alignmentBoundary = emitComp->opts.compJitAlignLoopBoundary;

// No padding if loop is already aligned
if ((offset & (alignmentBoundary - 1)) == 0)
{
JITDUMP(";; Skip alignment: 'Loop at %s already aligned at %dB boundary.'\n", emitLabelString(ig->igNext),
JITDUMP(";; Skip alignment: 'Loop at %s already aligned at %dB boundary.'\n", emitLabelString(loopHeadIG),
alignmentBoundary);
return 0;
}
Expand All @@ -5452,12 +5457,12 @@ unsigned emitter::emitCalculatePaddingForLoopAlignment(insGroup* ig, size_t offs
maxLoopSize = emitComp->opts.compJitAlignLoopMaxCodeSize;
}

unsigned loopSize = getLoopSize(ig->igNext, maxLoopSize DEBUG_ARG(isAlignAdjusted));
unsigned loopSize = getLoopSize(loopHeadIG, maxLoopSize DEBUG_ARG(isAlignAdjusted));

// No padding if loop is big
if (loopSize > maxLoopSize)
{
JITDUMP(";; Skip alignment: 'Loop at %s is big. LoopSize= %d, MaxLoopSize= %d.'\n", emitLabelString(ig->igNext),
JITDUMP(";; Skip alignment: 'Loop at %s is big. LoopSize= %d, MaxLoopSize= %d.'\n", emitLabelString(loopHeadIG),
loopSize, maxLoopSize);
return 0;
}
Expand Down Expand Up @@ -5494,15 +5499,15 @@ unsigned emitter::emitCalculatePaddingForLoopAlignment(insGroup* ig, size_t offs
{
skipPadding = true;
JITDUMP(";; Skip alignment: 'Loop at %s already aligned at %uB boundary.'\n",
emitLabelString(ig->igNext), alignmentBoundary);
emitLabelString(loopHeadIG), alignmentBoundary);
}
// Check if the alignment exceeds new maxPadding limit
else if (nPaddingBytes > nMaxPaddingBytes)
{
skipPadding = true;
JITDUMP(";; Skip alignment: 'Loop at %s PaddingNeeded= %d, MaxPadding= %d, LoopSize= %d, "
"AlignmentBoundary= %dB.'\n",
emitLabelString(ig->igNext), nPaddingBytes, nMaxPaddingBytes, loopSize, alignmentBoundary);
emitLabelString(loopHeadIG), nPaddingBytes, nMaxPaddingBytes, loopSize, alignmentBoundary);
}
}

Expand All @@ -5525,7 +5530,7 @@ unsigned emitter::emitCalculatePaddingForLoopAlignment(insGroup* ig, size_t offs
{
// Otherwise, the loop just fits in minBlocksNeededForLoop and so can skip alignment.
JITDUMP(";; Skip alignment: 'Loop at %s is aligned to fit in %d blocks of %d chunks.'\n",
emitLabelString(ig->igNext), minBlocksNeededForLoop, alignmentBoundary);
emitLabelString(loopHeadIG), minBlocksNeededForLoop, alignmentBoundary);
}
}
}
Expand Down Expand Up @@ -5554,12 +5559,12 @@ unsigned emitter::emitCalculatePaddingForLoopAlignment(insGroup* ig, size_t offs
{
// Otherwise, the loop just fits in minBlocksNeededForLoop and so can skip alignment.
JITDUMP(";; Skip alignment: 'Loop at %s is aligned to fit in %d blocks of %d chunks.'\n",
emitLabelString(ig->igNext), minBlocksNeededForLoop, alignmentBoundary);
emitLabelString(loopHeadIG), minBlocksNeededForLoop, alignmentBoundary);
}
}

JITDUMP(";; Calculated padding to add %d bytes to align %s at %dB boundary.\n", paddingToAdd,
emitLabelString(ig->igNext), alignmentBoundary);
emitLabelString(loopHeadIG), alignmentBoundary);

// Either no padding is added because it is too expensive or the offset gets aligned
// to the alignment boundary
Expand Down
16 changes: 11 additions & 5 deletions src/coreclr/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -1383,11 +1383,17 @@ class emitter
#if FEATURE_LOOP_ALIGN
struct instrDescAlign : instrDesc
{
instrDescAlign* idaNext; // next align in the group/method
insGroup* idaIG; // containing group
insGroup* idaTargetIG; // The IG before the loop IG.
// If no 'jmp' instructions were found until idaTargetIG,
// then idaTargetIG == idaIG.
instrDescAlign* idaNext; // next align in the group/method
insGroup* idaIG; // containing group
insGroup* idaLoopHeadPredIG; // The IG before the loop IG.
// If no 'jmp' instructions were found until idaLoopHeadPredIG,
// then idaLoopHeadPredIG == idaIG.

inline insGroup* loopHeadIG()
{
assert(idaLoopHeadPredIG);
return idaLoopHeadPredIG->igNext;
}

void removeAlignFlags()
{
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/emitarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11460,7 +11460,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
instrDescAlign* alignInstr = (instrDescAlign*)id;

if (emitComp->compStressCompile(Compiler::STRESS_EMITTER, 50) &&
(alignInstr->idaIG != alignInstr->idaTargetIG) && !skipIns)
(alignInstr->idaIG != alignInstr->idaLoopHeadPredIG) && !skipIns)
{
// There is no good way to squeeze in "bkpt" as well as display it
// in the disassembly because there is no corresponding instrDesc for
Expand Down Expand Up @@ -13351,9 +13351,9 @@ void emitter::emitDispIns(
printf("[%d bytes", id->idIsEmptyAlign() ? 0 : INSTR_ENCODED_SIZE);

// targetIG is only set for 1st of the series of align instruction
if ((alignInstrId->idaTargetIG != nullptr) && (alignInstrId->idaTargetIG->igNext != nullptr))
if ((alignInstrId->idaLoopHeadPredIG != nullptr) && (alignInstrId->loopHeadIG() != nullptr))
{
printf(" for IG%02u", alignInstrId->idaTargetIG->igNext->igNum);
printf(" for IG%02u", alignInstrId->loopHeadIG()->igNum);
}
printf("]");
}
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9833,9 +9833,9 @@ void emitter::emitDispIns(
instrDescAlign* alignInstrId = (instrDescAlign*)id;
printf("[%d bytes", alignInstrId->idCodeSize());
// targetIG is only set for 1st of the series of align instruction
if ((alignInstrId->idaTargetIG != nullptr) && (alignInstrId->idaTargetIG->igNext != nullptr))
if ((alignInstrId->idaLoopHeadPredIG != nullptr) && (alignInstrId->loopHeadIG() != nullptr))
{
printf(" for IG%02u", alignInstrId->idaTargetIG->igNext->igNum);
printf(" for IG%02u", alignInstrId->loopHeadIG()->igNum);
}
printf("]");
}
Expand Down Expand Up @@ -10031,7 +10031,7 @@ BYTE* emitter::emitOutputAlign(insGroup* ig, instrDesc* id, BYTE* dst)
// For cases where 'align' was placed behing a 'jmp' in an IG that does not
// immediately preced the loop IG, we do not know in advance the offset of
// IG having loop. For such cases, skip the padding calculation validation.
bool validatePadding = alignInstr->idaIG == alignInstr->idaTargetIG;
bool validatePadding = alignInstr->idaIG == alignInstr->idaLoopHeadPredIG;
#endif

// Candidate for loop alignment
Expand All @@ -10050,7 +10050,7 @@ BYTE* emitter::emitOutputAlign(insGroup* ig, instrDesc* id, BYTE* dst)
#ifdef DEBUG
if (validatePadding)
{
unsigned paddingNeeded = emitCalculatePaddingForLoopAlignment(((instrDescAlign*)id)->idaIG, (size_t)dst, true);
unsigned paddingNeeded = emitCalculatePaddingForLoopAlignment(((instrDescAlign*)id)->idaIG->igNext, (size_t)dst, true);

// For non-adaptive, padding size is spread in multiple instructions, so don't bother checking
if (emitComp->opts.compJitAlignLoopAdaptive)
Expand Down