JIT: Fix emitSplit to properly handle the remainder (#90246)
authorJakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
Wed, 9 Aug 2023 22:39:17 +0000 (00:39 +0200)
committerGitHub <noreply@github.com>
Wed, 9 Aug 2023 22:39:17 +0000 (00:39 +0200)
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 #85063

src/coreclr/jit/emit.cpp

index 59d82cc..c85724f 100644 (file)
@@ -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
@@ -3140,6 +3141,9 @@ void emitter::emitSplit(emitLocation*         startLoc,
         curSize += ig->igSize;
 
     } // end for loop
+
+    splitIfNecessary();
+    assert(curSize < maxSplitSize);
 }
 
 /*****************************************************************************