Skip to content

Commit

Permalink
JIT: Fix emitSplit to properly handle the remainder (dotnet#90246)
Browse files Browse the repository at this point in the history
emitSplit did not handle the edge case where the last instruction group
of the function/funclet resulted in the size to overflow the max. Use a
lambda so we can call the necessary code both as part of the loop and
after the loop.

Fix dotnet#85063
  • Loading branch information
jakobbotsch authored Aug 9, 2023
1 parent d32cb1f commit 405a62f
Showing 1 changed file with 52 additions and 48 deletions.
100 changes: 52 additions & 48 deletions src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3058,68 +3058,69 @@ void emitter::emitSplit(emitLocation* startLoc,
UNATIVE_OFFSET curSize;
UNATIVE_OFFSET candidateSize;

for (igPrev = NULL, ig = igLastReported = igStart, igLastCandidate = NULL, candidateSize = 0, curSize = 0;
ig != igEnd && ig != NULL; igPrev = ig, ig = ig->igNext)
{
// Keep looking until we've gone past the maximum split size
if (curSize >= maxSplitSize)
auto splitIfNecessary = [&]() {
if (curSize < maxSplitSize)
{
bool reportCandidate = true;
return;
}

// Is there a candidate?
if (igLastCandidate == NULL)
{
// Is there a candidate?
if (igLastCandidate == NULL)
{
#ifdef DEBUG
if (EMITVERBOSE)
printf("emitSplit: can't split at IG%02u; we don't have a candidate to report\n", ig->igNum);
if (EMITVERBOSE)
printf("emitSplit: can't split at IG%02u; we don't have a candidate to report\n", ig->igNum);
#endif
reportCandidate = false;
}
return;
}

// Don't report the same thing twice (this also happens for the first block, since igLastReported is
// initialized to igStart).
if (igLastCandidate == igLastReported)
{
// Don't report the same thing twice (this also happens for the first block, since igLastReported is
// initialized to igStart).
if (igLastCandidate == igLastReported)
{
#ifdef DEBUG
if (EMITVERBOSE)
printf("emitSplit: can't split at IG%02u; we already reported it\n", igLastCandidate->igNum);
if (EMITVERBOSE)
printf("emitSplit: can't split at IG%02u; we already reported it\n", igLastCandidate->igNum);
#endif
reportCandidate = false;
}
return;
}

// Don't report a zero-size candidate. This will only occur in a stress mode with JitSplitFunctionSize
// set to something small, and a zero-sized IG (possibly inserted for use by the alignment code). Normally,
// the split size will be much larger than the maximum size of an instruction group. The invariant we want
// to maintain is that each fragment contains a non-zero amount of code.
if (reportCandidate && (candidateSize == 0))
{
// Don't report a zero-size candidate. This will only occur in a stress mode with JitSplitFunctionSize
// set to something small, and a zero-sized IG (possibly inserted for use by the alignment code). Normally,
// the split size will be much larger than the maximum size of an instruction group. The invariant we want
// to maintain is that each fragment contains a non-zero amount of code.
if (candidateSize == 0)
{
#ifdef DEBUG
if (EMITVERBOSE)
printf("emitSplit: can't split at IG%02u; zero-sized candidate\n", igLastCandidate->igNum);
if (EMITVERBOSE)
printf("emitSplit: can't split at IG%02u; zero-sized candidate\n", igLastCandidate->igNum);
#endif
reportCandidate = false;
}
return;
}

// Report it!

// Report it!
if (reportCandidate)
{
#ifdef DEBUG
if (EMITVERBOSE)
{
printf("emitSplit: split at IG%02u is size %d, %s than requested maximum size of %d\n",
igLastCandidate->igNum, candidateSize, (candidateSize >= maxSplitSize) ? "larger" : "less",
maxSplitSize);
}
if (EMITVERBOSE)
{
printf("emitSplit: split at IG%02u is size %x, %s than requested maximum size of %x\n",
igLastCandidate->igNum, candidateSize, (candidateSize >= maxSplitSize) ? "larger" : "less",
maxSplitSize);
}
#endif

// hand memory ownership to the callback function
emitLocation* pEmitLoc = new (emitComp, CMK_Unknown) emitLocation(igLastCandidate);
callbackFunc(context, pEmitLoc);
igLastReported = igLastCandidate;
igLastCandidate = NULL;
curSize -= candidateSize;
}
}
// hand memory ownership to the callback function
emitLocation* pEmitLoc = new (emitComp, CMK_Unknown) emitLocation(igLastCandidate);
callbackFunc(context, pEmitLoc);
igLastReported = igLastCandidate;
igLastCandidate = NULL;
curSize -= candidateSize;
};

for (igPrev = NULL, ig = igLastReported = igStart, igLastCandidate = NULL, candidateSize = 0, curSize = 0;
ig != igEnd && ig != NULL; igPrev = ig, ig = ig->igNext)
{
splitIfNecessary();

// Update the current candidate to be this block, if it isn't in the middle of a
// prolog or epilog, which we can't split. All we know is that certain
Expand All @@ -3140,6 +3141,9 @@ void emitter::emitSplit(emitLocation* startLoc,
curSize += ig->igSize;

} // end for loop

splitIfNecessary();
assert(curSize < maxSplitSize);
}

/*****************************************************************************
Expand Down

0 comments on commit 405a62f

Please sign in to comment.