Fix a problem where curIG==0 and loop might be emitted in curIG, adju…

…st the targetIG to prevIG

Add IGF_REMOVED_ALIGN flag for special scenarios

src/coreclr/jit/emit.cpp

@@ -4932,9 +4932,24 @@ void emitter::emitLongLoopAlign(unsigned short alignmentBoundary)
 //
 void emitter::emitConnectAlignInstrWithCurIG()
 {
+    JITDUMP("Mapping 'align' instruction in IG%02u to target IG%02u\n", emitRecentFirstAlign->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.
-    emitRecentFirstAlign->idaTargetIG = emitCurIG;
+
+    if (emitCurIGsize == 0)
+    {
+        // However, if current IG is still empty (e.g. codegen didn't generate moves if they were redundant)
+        // in that case, loop will start in emitCurIG itself. For such cases, make sure that targetIG is
+        // pointing to previous IG.
+        assert(emitPrevIG != emitCurIG);
+
+        emitRecentFirstAlign->idaTargetIG = emitPrevIG;
+    }
+    else
+    {
+        emitRecentFirstAlign->idaTargetIG = emitCurIG;
+    }
 }
 
 //-----------------------------------------------------------------------------
@@ -5024,9 +5039,14 @@ unsigned emitter::getLoopSize(insGroup* igLoopHeader, unsigned maxLoopSize DEBUG
     for (insGroup* igInLoop = igLoopHeader; igInLoop != nullptr; igInLoop = igInLoop->igNext)
     {
         loopSize += igInLoop->igSize;
-        if (igInLoop->endsWithAlignInstr())
+        if (igInLoop->endsWithAlignInstr() || igInLoop->isAlignInstrRemoved())
         {
-            // If igInLoop is marked as "IGF_HAS_ALIGN", the basic block flow detected a loop start.
+            // If igInLoop can be in one of the following state:
+            // IGF_HAS_ALIGN - igInLoop contains align instruction at the end, for next IG or some future IG.
+            // IGF_REMOVED_ALIGN - igInLoop had align instruction at the end, but was removed.
+            //
+            // For both cases, remove the padding bytes from igInLoop's size so it is not included in loopSize.
+            //
             // If the loop was formed because of forward jumps like the loop IG18 below, the backedge is not
             // set for them and such loops are not aligned. For such cases, the loop size threshold will never
             // be met and we would break as soon as loopSize > maxLoopSize.
@@ -5039,9 +5059,9 @@ unsigned emitter::getLoopSize(insGroup* igLoopHeader, unsigned maxLoopSize DEBUG
             //      ...
             //      jne IG05
             //
-            // If igInLoop is a legitimate loop, and igInLoop's next IG is also a loop that needs alignment,
-            // then igInLoop should be the last IG of the current loop and should have backedge to current
-            // loop header.
+            // If igInLoop is a legitimate loop, and igInLoop's end with another 'align' instruction for different IG
+            // representing a loop that needs alignment, then igInLoop should be the last IG of the current loop and
+            // should have backedge to current loop header.
             //
             // Below, IG05 is the last IG of loop IG04-IG05 and its backedge points to IG04.
             //
@@ -5198,7 +5218,7 @@ void emitter::emitSetLoopBackEdge(BasicBlock* loopTopBlock)
                     assert(!markedCurrLoop);
 
                     // This IG should no longer contain alignment instruction
-                    alignInstr->idaIG->removeAlignInstr();
+                    alignInstr->removeAlignFlags();
 
                     markedCurrLoop = true;
                     JITDUMP("** Skip alignment for current loop IG%02u ~ IG%02u because it encloses an aligned loop "
@@ -5215,7 +5235,7 @@ void emitter::emitSetLoopBackEdge(BasicBlock* loopTopBlock)
                     assert(alignInstr->idaIG->endsWithAlignInstr());
 
                     // This IG should no longer contain alignment instruction
-                    alignInstr->idaIG->removeAlignInstr();
+                    alignInstr->removeAlignFlags();
 
                     markedLastLoop = true;
                     JITDUMP("** Skip alignment for aligned loop IG%02u ~ IG%02u because it encloses the current loop "
@@ -5275,7 +5295,7 @@ void emitter::emitLoopAlignAdjustments()
         insGroup* containingIG = alignInstr->idaIG;
 
         JITDUMP("  Adjusting 'align' instruction in IG%02u that is targeted for IG%02u \n", containingIG->igNum,
-                alignIG->igNum);
+                alignIG->igNext->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"
@@ -5307,7 +5327,7 @@ void emitter::emitLoopAlignAdjustments()
             containingIG->igFlags |= IGF_UPD_ISZ;
             if (actualPaddingNeeded == 0)
             {
-                containingIG->removeAlignInstr();
+                alignInstr->removeAlignFlags();
             }
 
 #ifdef TARGET_XARCH
@@ -8415,6 +8435,12 @@ insGroup* emitter::emitAllocAndLinkIG()
 
     ig->igFlags |= (emitCurIG->igFlags & IGF_PROPAGATE_MASK);
 
+#ifdef FEATURE_LOOP_ALIGN
+    /* Save the prev IG */
+
+    emitPrevIG = emitCurIG;
+#endif
+
     /* Set the new IG as the current IG */
 
     emitCurIG = ig;

src/coreclr/jit/emit.h

@@ -275,7 +275,10 @@ struct insGroup
 #define IGF_PLACEHOLDER 0x0100    // this is a placeholder group, to be filled in later
 #define IGF_EXTEND 0x0200         // this block is conceptually an extension of the previous block
                                   // 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) in the end;
+#define IGF_REMOVED_ALIGN 0x0400  // this group had alignment instruction(s) at the end, but was removed;
+                                  // Useful in scenario where an IG is part of a loop also contains align instruction
+                                  // for a different loop and later, we decide to not align that different loop.
+#define IGF_HAS_ALIGN 0x0800      // this group contains an alignment instruction(s) in the end;
                                   // the next IG may or may not be the head of a loop that is needing alignment
                                   // This IG might end up with 'jmp' and hence 'align' instruction might be added
                                   // in this IG, after the 'jmp' instruction.
@@ -351,9 +354,9 @@ struct insGroup
         return *(unsigned*)ptr;
     }
 
-    void removeAlignInstr()
+    bool isAlignInstrRemoved()
     {
-        igFlags &= ~IGF_HAS_ALIGN;
+        return (igFlags & IGF_REMOVED_ALIGN) != 0;
     }
 
     bool endsWithAlignInstr()
@@ -1395,6 +1398,12 @@ class emitter
         insGroup*       idaTargetIG; // The IG before the loop IG.
                                      // If no 'jmp' instructions were found until targetIG,
                                      // the 'align' will be placed in this IG.
+
+        void removeAlignFlags()
+        {
+            idaIG->igFlags |= IGF_REMOVED_ALIGN;
+            idaIG->igFlags &= ~IGF_HAS_ALIGN;
+        }
     };
     void emitCheckAlignFitInCurIG(unsigned short nAlignInstr);
 #endif // FEATURE_LOOP_ALIGN
@@ -1732,6 +1741,9 @@ class emitter
     bool emitChkAlign; // perform some alignment checks
 #endif
 
+#ifdef FEATURE_LOOP_ALIGN
+    insGroup* emitPrevIG;
+#endif
     insGroup* emitCurIG;
 
     void emitSetShortJump(instrDescJmp* id);
@@ -1805,6 +1817,7 @@ class emitter
     void emitLongLoopAlign(unsigned short alignmentBoundary);
     instrDescAlign* emitAlignInNextIG(instrDescAlign* alignInstr);
     void emitConnectAlignInstrWithCurIG();
+
 #endif
 
     void emitCheckFuncletBranch(instrDesc* jmp, insGroup* jmpIG); // Check for illegal branches between funclets

src/coreclr/jit/emitxarch.cpp

@@ -10063,7 +10063,9 @@ BYTE* emitter::emitOutputAlign(insGroup* ig, instrDesc* id, BYTE* dst)
 #endif
 
     BYTE* dstRW = dst + writeableOffset;
-    dstRW       = emitOutputNOP(dstRW, paddingToAdd);
+    if (emitComp->compStressCompile(Compiler::STRESS_EMITTER, 1) && !validatePadding && paddingToAdd >= 1)
+        // if (emitComp->opts.disAsm)
+    dstRW = emitOutputNOP(dstRW, paddingToAdd);
     return dstRW - writeableOffset;
 }