diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 4a6bb66066bba3..c87fb954591bbb 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2164,7 +2164,8 @@ void fgArgInfo::EvalArgsToTemps() } //------------------------------------------------------------------------------ -// fgMakeMultiUse : If the node is a local, clone it, otherwise insert a comma form temp +// fgMakeMultiUse : If the node is an unaliased local or constant clone it, +// otherwise insert a comma form temp // // Arguments: // ppTree - a pointer to the child node we will be replacing with the comma expression that @@ -2172,18 +2173,32 @@ void fgArgInfo::EvalArgsToTemps() // // Return Value: // 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. +// GenTree* Compiler::fgMakeMultiUse(GenTree** pOp) { - GenTree* tree = *pOp; - if (tree->IsLocal()) + GenTree* const tree = *pOp; + + if (tree->IsInvariant()) { return gtClone(tree); } - else + else if (tree->IsLocal()) { - return fgInsertCommaFormTemp(pOp); + // Can't rely on GTF_GLOB_REF here. + // + if (!lvaGetDesc(tree->AsLclVarCommon())->IsAddressExposed()) + { + return gtClone(tree); + } } + + return fgInsertCommaFormTemp(pOp); } //------------------------------------------------------------------------------ @@ -14441,13 +14456,10 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) // division will be used, in that case this transform allows CSE to // eliminate the redundant div from code like "x = a / 3; y = a % 3;". // -// This method will produce the above expression in 'a' and 'b' are -// leaf nodes, otherwise, if any of them is not a leaf it will spill -// its value into a temporary variable, an example: -// (x * 2 - 1) % (y + 1) -> t1 - (t2 * ( comma(t1 = x * 2 - 1, t1) / comma(t2 = y + 1, t2) ) ) -// GenTree* Compiler::fgMorphModToSubMulDiv(GenTreeOp* tree) { + JITDUMP("\nMorphing MOD/UMOD [%06u] to Sub/Mul/Div\n", dspTreeID(tree)); + if (tree->OperGet() == GT_MOD) { tree->SetOper(GT_DIV); @@ -14461,29 +14473,15 @@ GenTree* Compiler::fgMorphModToSubMulDiv(GenTreeOp* tree) noway_assert(!"Illegal gtOper in fgMorphModToSubMulDiv"); } - var_types type = tree->gtType; - GenTree* denominator = tree->gtOp2; - GenTree* numerator = tree->gtOp1; + var_types type = tree->gtType; - if (!numerator->OperIsLeaf()) - { - numerator = fgMakeMultiUse(&tree->gtOp1); - } - - if (!denominator->OperIsLeaf()) - { - denominator = fgMakeMultiUse(&tree->gtOp2); - } + GenTree* const copyOfNumeratorValue = fgMakeMultiUse(&tree->gtOp1); + GenTree* const copyOfDenominatorValue = fgMakeMultiUse(&tree->gtOp2); + GenTree* const mul = gtNewOperNode(GT_MUL, type, tree, copyOfDenominatorValue); + GenTree* const sub = gtNewOperNode(GT_SUB, type, copyOfNumeratorValue, mul); - // The numerator and denominator may have been assigned to temps, in which case - // their defining assignments are in the current tree. Therefore, we need to - // set the execuction order accordingly on the nodes we create. - // That is, the "mul" will be evaluated in "normal" order, and the "sub" must - // be set to be evaluated in reverse order. + // Ensure "sub" does not evaluate "copyOfNumeratorValue" before it is defined by "mul". // - GenTree* mul = gtNewOperNode(GT_MUL, type, tree, gtCloneExpr(denominator)); - assert(!mul->IsReverseOp()); - GenTree* sub = gtNewOperNode(GT_SUB, type, gtCloneExpr(numerator), mul); sub->gtFlags |= GTF_REVERSE_OPS; #ifdef DEBUG diff --git a/src/tests/JIT/opt/ForwardSub/modOpt.cs b/src/tests/JIT/opt/ForwardSub/modOpt.cs new file mode 100644 index 00000000000000..c1c2492715085c --- /dev/null +++ b/src/tests/JIT/opt/ForwardSub/modOpt.cs @@ -0,0 +1,65 @@ +// 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-02-08 16:14:33 +// Run on Arm64 Linux +// Seed: 10380378818647712015 +// Reduced from 169.3 KiB to 0.7 KiB in 00:44:43 +// Debug: Outputs 0 +// Release: Outputs 1 +public class C0 +{ +} + +public struct S0 +{ + public uint F2; + public C0 F4; + public uint F5; + public int F7; + public short F9; + public S0(uint f2, int f7): this() + { + F7 = f7; + } + + public int M21(uint arg0) + { + S0[] var0 = new S0[]{new S0(0, 0)}; + var vr1 = new S0(this.F2++, ForwardSubModOpt.M23()); + return ForwardSubModOpt.s_8.F7; + } +} + +public class ForwardSubModOpt +{ + public static IRT s_rt; + public static uint s_2; + public static S0 s_8 = new S0(0, 1); + public static int Main() + { + s_rt = new C(); + S0 vr8 = default(S0); + ushort vr9 = (ushort)(vr8.F2 % (ushort)vr8.M21(s_2)); + s_rt.WriteLine(vr9); + return vr9 == 0 ? 100 : -1; + } + + public static byte M23() + { + return default(byte); + } +} + +public interface IRT +{ + void WriteLine(T value); +} + +public class C : IRT +{ + public void WriteLine(T value) + { + System.Console.WriteLine(value); + } +} diff --git a/src/tests/JIT/opt/ForwardSub/modOpt.csproj b/src/tests/JIT/opt/ForwardSub/modOpt.csproj new file mode 100644 index 00000000000000..19781e26c20d81 --- /dev/null +++ b/src/tests/JIT/opt/ForwardSub/modOpt.csproj @@ -0,0 +1,10 @@ + + + Exe + + True + + + + +