JIT: Avoid reordering operands in fgMorphModToSubMulDiv

src/coreclr/jit/compiler.h

@@ -5506,7 +5506,6 @@ class Compiler
     //                  Create a new temporary variable to hold the result of *ppTree,
     //                  and transform the graph accordingly.
     GenTree* fgInsertCommaFormTemp(GenTree** ppTree, CORINFO_CLASS_HANDLE structType = nullptr);
-    bool fgIsSafeToClone(GenTree* tree);
     TempInfo fgMakeTemp(GenTree* rhs, CORINFO_CLASS_HANDLE structType = nullptr);
     GenTree* fgMakeMultiUse(GenTree** ppTree, CORINFO_CLASS_HANDLE structType = nullptr);
 

src/coreclr/jit/morph.cpp

@@ -1785,39 +1785,6 @@ void CallArgs::SetNeedsTemp(CallArg* arg)
     m_needsTemps   = true;
 }
 
-//------------------------------------------------------------------------------
-// fgIsSafeToClone: If the node is an unaliased local or constant,
-//                  then it is safe to clone.
-//
-// Arguments:
-//    tree - The node to check if it is safe to clone.
-//
-// Return Value:
-//    True if the tree is cloneable. False if the tree is not cloneable.
-//
-// Notes:
-//    This is conservative as this will return False if the local's address
-//    is exposed.
-//
-bool Compiler::fgIsSafeToClone(GenTree* tree)
-{
-    if (tree->IsInvariant())
-    {
-        return true;
-    }
-    else if (tree->IsLocal())
-    {
-        // Can't rely on GTF_GLOB_REF here.
-        //
-        if (!lvaGetDesc(tree->AsLclVarCommon())->IsAddressExposed())
-        {
-            return true;
-        }
-    }
-
-    return false;
-}
-
 //------------------------------------------------------------------------------
 // fgMakeTemp: Make a temp variable with a right-hand side expression as the assignment.
 //
@@ -1865,18 +1832,18 @@ TempInfo Compiler::fgMakeTemp(GenTree* rhs, CORINFO_CLASS_HANDLE structType /*=
 //    A fresh GT_LCL_VAR node referencing the temp which has not been used
 //
 // Notes:
-//    Caller must ensure that if the node is an unaliased local, the second use this
-//    creates will be evaluated before the local can be reassigned.
-//
-//    Can be safely called in morph preorder, before GTF_GLOB_REF is reliable.
+//    This function will clone invariant nodes and locals, so this function
+//    should only be used in situations where no interference between the
+//    original use and new use is possible. Otherwise, fgInsertCommaFormTemp
+//    should be used directly.
 //
 GenTree* Compiler::fgMakeMultiUse(GenTree** pOp, CORINFO_CLASS_HANDLE structType /*= nullptr*/)
 {
     GenTree* const tree = *pOp;
 
-    if (fgIsSafeToClone(tree))
+    if (tree->IsInvariant() || tree->OperIsLocal())
     {
-        return gtClone(tree);
+        return gtCloneExpr(tree);
     }
 
     return fgInsertCommaFormTemp(pOp, structType);
@@ -13816,20 +13783,43 @@ GenTree* Compiler::fgMorphModToSubMulDiv(GenTreeOp* tree)
 
     GenTreeOp* const div = tree;
 
+    assert(!div->IsReverseOp());
+
     GenTree* dividend = div->gtGetOp1();
     GenTree* divisor  = div->gtGetOp2();
 
-    TempInfo tempInfos[2]{};
+    TempInfo tempInfos[2];
     int      tempInfoCount = 0;
 
-    if (!fgIsSafeToClone(dividend))
+    // This transform runs in pre-morph so we cannot rely on GTF_GLOB_REF.
+    // Furthermore, this logic is somewhat complicated since the divisor and
+    // dividend are arbitrary nodes. For instance, if we spill the divisor and
+    // the dividend is a local, we need to spill the dividend too unless the
+    // divisor could not cause it to be reassigned.
+    // This could be slightly better via GTF_CALL and GTF_ASG checks on the
+    // divisor but the diffs of this were minor and the extra complexity seemed
+    // not worth it.
+    bool spillDividend;
+    bool spillDivisor;
+    if (divisor->IsInvariant() || divisor->OperIsLocal())
+    {
+        spillDivisor  = false;
+        spillDividend = !dividend->IsInvariant() && !dividend->OperIsLocal();
+    }
+    else
+    {
+        spillDivisor  = true;
+        spillDividend = !dividend->IsInvariant();
+    }
+
+    if (spillDividend)
     {
         tempInfos[tempInfoCount] = fgMakeTemp(dividend);
         dividend                 = tempInfos[tempInfoCount].load;
         tempInfoCount++;
     }
 
-    if (!fgIsSafeToClone(divisor))
+    if (spillDivisor)
     {
         tempInfos[tempInfoCount] = fgMakeTemp(divisor);
         divisor                  = tempInfos[tempInfoCount].load;

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

@@ -0,0 +1,58 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Generated by Fuzzlyn v1.5 on 2022-07-03 14:40:52
+// Run on Arm64 MacOS
+// Seed: 5960657379714964908
+// Reduced from 328.5 KiB to 0.8 KiB in 00:02:08
+// Debug: Outputs 0
+// Release: Outputs 1
+using System.Runtime.CompilerServices;
+
+public struct S0
+{
+    public long F0;
+    public long F1;
+    public S0(long f1): this()
+    {
+        F1 = f1;
+    }
+}
+
+public struct S1
+{
+    public uint F1;
+    public ushort F2;
+    public S0 F4;
+    public S1(ulong f5): this()
+    {
+    }
+
+    public long M82(ref short[] arg0)
+    {
+        Runtime_71600.s_13 = this.F1++;
+        S1 var1 = new S1(0);
+        return Runtime_71600.s_39[0].F1;
+    }
+}
+
+public class Runtime_71600
+{
+    public static uint s_13;
+    public static short[] s_28;
+    public static S0[] s_39;
+    public static int Main()
+    {
+        S0[] vr2 = new S0[]{new S0(-1)};
+        s_39 = vr2;
+        S1 vr3 = default(S1);
+        return M80(vr3) == 0 ? 100 : -1;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static int M80(S1 arg0)
+    {
+        arg0.F2 += (ushort)(arg0.F1 % (uint)arg0.M82(ref s_28));
+        return arg0.F2;
+    }
+}

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

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>