JIT: bail on flipping post-layout JTRUE when reversal needs new IR

src/coreclr/jit/compiler.h

@@ -3879,6 +3879,7 @@ class Compiler
     bool gtComplexityExceeds(GenTree* tree, unsigned limit, TFunc getComplexity);
 
     GenTree* gtReverseCond(GenTree* tree);
+    bool     gtTryReverseCond(GenTree* tree);
 
     static bool gtHasRef(GenTree* tree, unsigned lclNum);
 

src/coreclr/jit/gentree.cpp

@@ -3730,12 +3730,19 @@ genTreeOps GenTree::SwapRelop(genTreeOps relop)
     return swapOps[relop - GT_EQ];
 }
 
-/*****************************************************************************
- *
- *  Reverse the meaning of the given test condition.
- */
-
-GenTree* Compiler::gtReverseCond(GenTree* tree)
+//------------------------------------------------------------------------
+// gtTryReverseCond: Try to reverse the meaning of the given test condition
+//    in-place, without introducing any new IR nodes.
+//
+// Arguments:
+//    tree - The condition tree to reverse
+//
+// Return Value:
+//    True if the condition was reversed in-place. False if reversing the
+//    condition would require introducing a new node (e.g. wrapping the tree
+//    in a "tree == 0" comparison); in that case, the tree is not modified.
+//
+bool Compiler::gtTryReverseCond(GenTree* tree)
 {
     if (tree->OperIsCompare())
     {
@@ -3749,23 +3756,45 @@ GenTree* Compiler::gtReverseCond(GenTree* tree)
         {
             tree->gtFlags ^= GTF_RELOP_NAN_UN;
         }
+        return true;
     }
-    else if (tree->OperIs(GT_JCC, GT_SETCC))
+    if (tree->OperIs(GT_JCC, GT_SETCC))
     {
         GenTreeCC* cc   = tree->AsCC();
         cc->gtCondition = GenCondition::Reverse(cc->gtCondition);
+        return true;
     }
-    else if (tree->OperIs(GT_JCMP, GT_JTEST))
+    if (tree->OperIs(GT_JCMP, GT_JTEST))
     {
         GenTreeOpCC* opCC = tree->AsOpCC();
         opCC->gtCondition = GenCondition::Reverse(opCC->gtCondition);
+        return true;
     }
-    else if (tree->IsIntegralConst())
+    if (tree->IsIntegralConst())
     {
         GenTreeIntConCommon* con = tree->AsIntConCommon();
         con->SetIntegralValue(con->IsIntegralConst(0) ? 1 : 0);
+        return true;
     }
-    else
+
+    return false;
+}
+
+//------------------------------------------------------------------------
+// gtReverseCond: Reverse the meaning of the given test condition.
+//
+// Arguments:
+//    tree - The condition tree to reverse
+//
+// Return Value:
+//    The reversed condition tree. This is normally the same node as the
+//    input tree, with its operator (or condition) reversed in-place. If the
+//    tree cannot be reversed in-place, a new GT_EQ node comparing the tree
+//    to zero is returned instead.
+//
+GenTree* Compiler::gtReverseCond(GenTree* tree)
+{
+    if (!gtTryReverseCond(tree))
     {
         tree = gtNewOperNode(GT_EQ, TYP_INT, tree, gtNewZeroConNode(TYP_INT));
     }

src/coreclr/jit/optimizer.cpp

@@ -2350,22 +2350,21 @@ PhaseStatus Compiler::optOptimizePostLayout()
             GenTree* const test = block->lastNode();
             assert(test->OperIsConditionalJump());
 
+            // Try to reverse the condition in-place. We are running after LSRA, so we cannot
+            // introduce new IR nodes here. If the condition cannot be reversed in-place, bail
+            // on flipping for this block.
+            //
+            // For GT_JTRUE the operand may have a GT_COPY/GT_RELOAD inserted by LSRA on top of
+            // the actual condition node, so skip those to find the underlying condition.
+            GenTree* cond = test;
             if (test->OperIs(GT_JTRUE))
             {
-                // Flip GT_JTRUE node's conditional operand, and handle any new nodes this may introduce
-                GenTree* const cond    = test->gtGetOp1();
-                GenTree* const newCond = gtReverseCond(cond);
-                if (cond != newCond)
-                {
-                    LIR::AsRange(block).InsertAfter(cond, newCond);
-                    test->AsUnOp()->gtOp1 = newCond;
-                }
+                cond = test->gtGetOp1()->gtSkipReloadOrCopy();
             }
-            else
+
+            if (!gtTryReverseCond(cond))
             {
-                // gtReverseCond can handle other conditional jumps without introducing a new node
-                GenTree* const cond = gtReverseCond(test);
-                assert(cond == test);
+                continue;
             }
 
             FlowEdge* const oldTrueEdge  = block->GetTrueEdge();

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

@@ -0,0 +1,99 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Regression test: optOptimizePostLayout could assert "found use of a node that
+// is not in the LIR sequence" on arm32 when LSRA inserted a RELOAD on top of
+// a SETCC result under a JTRUE. gtReverseCond then returned a newly-allocated
+// EQ(reload, 0) tree whose CNS_INT 0 child was never threaded into LIR.
+
+namespace Runtime_127745;
+
+using System;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public class Runtime_127745
+{
+    public static IRuntime127745 s_rt;
+    public static short[][][] s_15;
+    public static uint[,] s_34;
+    public static byte[] s_53;
+    public static int s_62;
+    public static long[][] s_90;
+    public static int s_108;
+    public static int[] s_110;
+    public static long s_114;
+    public static ushort s_130;
+
+    [Fact]
+    public static void TestEntryPoint()
+    {
+        // Calling M96 forces the JIT to compile it. The regression was a
+        // compile-time assertion rather than wrong output. The static fields are
+        // all null, so execution throws NullReferenceException immediately.
+        try
+        {
+            RunM96();
+        }
+        catch (NullReferenceException)
+        {
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void RunM96()
+    {
+        M96(s_90);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static ref byte M96(long[][] arg0)
+    {
+        byte lvar0 = default(byte);
+        do
+        {
+            s_rt.WriteLine("c_677", 0);
+        }
+        while (++lvar0 < 252);
+        try
+        {
+            var vr2 = new short[] { 1 };
+            M100(ref arg0[0][0], ref arg0[0][0], vr2);
+        }
+        finally
+        {
+            arg0[0] = arg0[0];
+        }
+
+        var vr1 = s_15[0][0][0];
+        if ((1 <= s_114) | (0 > M97(vr1)))
+        {
+            arg0[0][0] >>= 1;
+        }
+        else
+        {
+            s_62 = s_110[0];
+        }
+
+        s_rt.WriteLine("c_715", arg0[0][0]);
+        s_rt.WriteLine("c_716", s_108);
+        return ref s_53[0];
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static ushort M97(short arg0)
+    {
+        bool vr0 = 0 > s_34[0, 0];
+        return s_130;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void M100(ref long arg0, ref long arg1, short[] arg2)
+    {
+    }
+}
+
+public interface IRuntime127745
+{
+    void WriteLine<T>(string site, T value);
+}

src/tests/JIT/Regression/Regression_ro_2.csproj

@@ -102,6 +102,7 @@
     <Compile Include="JitBlue\Runtime_128801\Runtime_128801.cs" />
     <Compile Include="JitBlue\Runtime_129076\Runtime_129076.cs" />
     <Compile Include="JitBlue\Runtime_129176\Runtime_129176.cs" />
+    <Compile Include="JitBlue\Runtime_127745\Runtime_127745.cs" />
   </ItemGroup>
   <Import Project="$(TestSourceDir)MergedTestRunner.targets" />
 </Project>