JIT: fix redundant zero init issue (.NET 8 port)

"Port" the changes from dotnet#103788 to .NET 8.

We don't have the DFS tree, so enhance the topological sort that SSA uses
to detect cycles and backedges.

Use this info to stop searching for redundant zero inits once flow from
entry has entered a cycle.

Fixes dotnet#103477 (for .NET 8).

src/coreclr/jit/block.h

@@ -1883,6 +1883,12 @@ class AllSuccessorEnumerator
         return m_block;
     }
 
+    // True if NextSuccessor will return the first successor or null
+    bool AtStart() const
+    {
+        return m_curSucc == UINT_MAX;
+    }
+
     // Returns the next available successor or `nullptr` if there are no more successors.
     BasicBlock* NextSuccessor(Compiler* comp)
     {

src/coreclr/jit/compiler.h

@@ -6334,7 +6334,7 @@ class Compiler
     PhaseStatus optCloneLoops();
     void optCloneLoop(unsigned loopInd, LoopCloneContext* context);
     PhaseStatus optUnrollLoops(); // Unrolls loops (needs to have cost info)
-    void        optRemoveRedundantZeroInits();
+    void        optRemoveRedundantZeroInits(bool hasCycle);
     PhaseStatus optIfConversion(); // If conversion
 
 protected:

src/coreclr/jit/optimizer.cpp

@@ -9007,6 +9007,9 @@ typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, unsigned> Lc
 //------------------------------------------------------------------------------------------
 // optRemoveRedundantZeroInits: Remove redundant zero initializations.
 //
+// Arguments:
+//    hasCycle -- true if SSA's topological sort detected a cycle in the flowgraph
+//
 // Notes:
 //    This phase iterates over basic blocks starting with the first basic block until there is no unique
 //    basic block successor or until it detects a loop. It keeps track of local nodes it encounters.
@@ -9024,8 +9027,10 @@ typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, unsigned> Lc
 //            either the local has no gc pointers or there are no gc-safe points between the prolog and the assignment,
 //         then the local is marked with lvHasExplicitInit which tells the codegen not to insert zero initialization
 //         for this local in the prolog.
-
-void Compiler::optRemoveRedundantZeroInits()
+//
+//   It depends on pre/post order numbers set by TopologicalSort.
+//
+void Compiler::optRemoveRedundantZeroInits(bool hasCycle)
 {
 #ifdef DEBUG
     if (verbose)
@@ -9043,10 +9048,37 @@ void Compiler::optRemoveRedundantZeroInits()
 
     assert(fgNodeThreading == NodeThreading::AllTrees);
 
-    for (BasicBlock* block = fgFirstBB; (block != nullptr) && ((block->bbFlags & BBF_MARKED) == 0);
-         block             = block->GetUniqueSucc())
+    for (BasicBlock* block = fgFirstBB; (block != nullptr); block = block->GetUniqueSucc())
     {
-        block->bbFlags |= BBF_MARKED;
+        if (hasCycle)
+        {
+            // See if this block is a cycle entry
+            //
+            bool stop = false;
+            for (FlowEdge* predEdge = BlockPredsWithEH(block); predEdge != nullptr;
+                 predEdge           = predEdge->getNextPredEdge())
+            {
+                BasicBlock* const predBlock = predEdge->getSourceBlock();
+
+                if ((block->bbPreorderNum <= predBlock->bbPreorderNum) &&
+                    (predBlock->bbPostorderNum <= block->bbPostorderNum))
+                {
+                    JITDUMP(FMT_BB " is part of a cycle, stopping the block scan\n", block->bbNum);
+                    stop = true;
+                    break;
+                }
+            }
+
+            // If so, stop looking for redundant zero inits
+            //
+            if (stop)
+            {
+                break;
+            }
+        }
+
+        JITDUMP("Analyzing " FMT_BB "\n", block->bbNum);
+
         CompAllocator   allocator(getAllocator(CMK_ZeroInit));
         LclVarRefCounts defsInBlock(allocator);
         bool            removedTrackedDefs = false;
@@ -9167,7 +9199,7 @@ void Compiler::optRemoveRedundantZeroInits()
 
                         if (tree->Data()->IsIntegralConst(0))
                         {
-                            bool bbInALoop  = (block->bbFlags & BBF_BACKWARD_JUMP) != 0;
+                            bool bbInALoop  = false;
                             bool bbIsReturn = block->bbJumpKind == BBJ_RETURN;
 
                             if (!bbInALoop || bbIsReturn)
@@ -9244,12 +9276,6 @@ void Compiler::optRemoveRedundantZeroInits()
             }
         }
     }
-
-    for (BasicBlock* block = fgFirstBB; (block != nullptr) && ((block->bbFlags & BBF_MARKED) != 0);
-         block             = block->GetUniqueSucc())
-    {
-        block->bbFlags &= ~BBF_MARKED;
-    }
 }
 
 //------------------------------------------------------------------------

src/coreclr/jit/ssabuilder.cpp

@@ -131,6 +131,7 @@ SsaBuilder::SsaBuilder(Compiler* pCompiler)
     , m_allocator(pCompiler->getAllocator(CMK_SSA))
     , m_visitedTraits(0, pCompiler) // at this point we do not know the size, SetupBBRoot can add a block
     , m_renameStack(m_allocator, pCompiler->lvaCount)
+    , m_hasCycle(false)
 {
 }
 
@@ -179,6 +180,7 @@ int SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
     };
 
     // Compute order.
+    int         preIndex  = 0;
     int         postIndex = 0;
     BasicBlock* block     = comp->fgFirstBB;
     BitVecOps::AddElemD(&m_visitedTraits, m_visited, block->bbNum);
@@ -189,11 +191,26 @@ int SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
 
     while (!blocks.Empty())
     {
-        BasicBlock* block = blocks.TopRef().Block();
-        BasicBlock* succ  = blocks.TopRef().NextSuccessor(comp);
+        AllSuccessorEnumerator& e     = blocks.TopRef();
+        BasicBlock*             block = e.Block();
+
+        if (e.AtStart())
+        {
+            DBG_SSA_JITDUMP("[SsaBuilder::TopologicalSort] preOrder[%d] = " FMT_BB "\n", preIndex, block->bbNum);
+            block->bbPreorderNum  = preIndex;
+            block->bbPostorderNum = UINT_MAX;
+            preIndex++;
+        }
+
+        BasicBlock* succ = e.NextSuccessor(comp);
 
         if (succ != nullptr)
         {
+            if ((succ->bbPreorderNum <= block->bbPreorderNum) && (succ->bbPostorderNum == UINT_MAX))
+            {
+                m_hasCycle = true;
+            }
+
             // if the block on TOS still has unreached successors, visit them
             if (BitVecOps::TryAddElemD(&m_visitedTraits, m_visited, succ->bbNum))
             {
@@ -1541,7 +1558,7 @@ void SsaBuilder::Build()
     m_pCompiler->fgLocalVarLiveness();
     EndPhase(PHASE_BUILD_SSA_LIVENESS);
 
-    m_pCompiler->optRemoveRedundantZeroInits();
+    m_pCompiler->optRemoveRedundantZeroInits(m_hasCycle);
     EndPhase(PHASE_ZERO_INITS);
 
     // Mark all variables that will be tracked by SSA

src/coreclr/jit/ssabuilder.h

@@ -101,4 +101,6 @@ class SsaBuilder
     BitVec       m_visited;
 
     SsaRenameState m_renameStack;
+
+    bool m_hasCycle;
 };

src/tests/JIT/Regression/JitBlue/Runtime_103477/Runtime_103477.cs

@@ -0,0 +1,71 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public class Runtime_103477
+{
+    static int s_count;
+
+    [Fact]
+    public static int Test()
+    {
+        int result = -1;
+        try
+        {
+            Problem();
+            result = 100;
+        }
+        catch (Exception)
+        {
+        }
+        return result;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void Problem()
+    {
+        string s = "12151";
+        int i = 0;
+
+        char? a = null;
+        int count = 0;
+        while (true)
+        {
+            string? res = get(s, ref i, ref a);
+            if (res != null)
+            {
+                Count(res);
+                a = null; // !!! this line is removed from the published version
+                continue;
+            }
+
+            if (i >= s.Length)
+                break;
+
+            a = '.';
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void Count(string s)
+    {
+        s_count++;
+        if (s_count > 5) throw new Exception();
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static string? get(string s, ref int index, ref char? a)
+    {
+        if (index >= s.Length)
+            return null;
+
+        if (a == '.')
+            return ".";
+
+        a ??= s[index++];
+        return (a == '1') ? "1" : null;
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_103477/Runtime_103477.csproj

@@ -0,0 +1,5 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>