diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index ca9bcdfcb26983..bea58c590c2c49 100644 --- a/src/coreclr/jit/compiler.h +++ b/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); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 7a366338cc3759..317f71a47a564c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/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; diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_71600/Runtime_71600.cs b/src/tests/JIT/Regression/JitBlue/Runtime_71600/Runtime_71600.cs new file mode 100644 index 00000000000000..9520ca0597b9dc --- /dev/null +++ b/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; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_71600/Runtime_71600.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_71600/Runtime_71600.csproj new file mode 100644 index 00000000000000..f492aeac9d056b --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_71600/Runtime_71600.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + \ No newline at end of file