From 20f9bd0dfed718de3f27c2bfacd438ac47411b60 Mon Sep 17 00:00:00 2001 From: TIHan Date: Tue, 1 Nov 2022 17:55:37 -0700 Subject: [PATCH 01/23] Optimize a % 1 to Zero --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/morph.cpp | 61 +++++++++++++++++++ src/tests/JIT/opt/Remainder/IntRemainder.cs | 25 ++++++++ .../JIT/opt/Remainder/IntRemainder.csproj | 17 ++++++ 4 files changed, 104 insertions(+) create mode 100644 src/tests/JIT/opt/Remainder/IntRemainder.cs create mode 100644 src/tests/JIT/opt/Remainder/IntRemainder.csproj diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index eef9a7a01f928..6b6fb781bdedd 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5792,6 +5792,7 @@ class Compiler GenTree* fgOptimizeBitwiseXor(GenTreeOp* xorOp); GenTree* fgPropagateCommaThrow(GenTree* parent, GenTreeOp* commaThrow, GenTreeFlags precedingSideEffects); GenTree* fgMorphRetInd(GenTreeUnOp* tree); + GenTree* fgMorphModToZero(GenTreeOp* tree); GenTree* fgMorphModToSubMulDiv(GenTreeOp* tree); GenTree* fgMorphUModToAndSub(GenTreeOp* tree); GenTree* fgMorphSmpOpOptional(GenTreeOp* tree, bool* optAssertionPropDone); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 18ba00e9add32..7d2f6db04e174 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9912,6 +9912,23 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (!optValnumCSE_phase) { + if (tree->OperIs(GT_MOD, GT_UMOD) && op2->IsIntegralConst(1)) + { + // Transformation: a % 1 = 0 + tree = fgMorphModToZero(tree->AsOp()); + if (tree->OperIsBinary()) + { + op1 = tree->gtGetOp1(); + op2 = tree->gtGetOp2(); + } + else + { + assert(tree->IsIntegralConst(0)); + op1 = nullptr; + op2 = nullptr; + } + } + if (tree->OperIs(GT_UMOD) && op2->IsIntegralConstUnsignedPow2()) { // Transformation: a % b = a & (b - 1); @@ -13160,6 +13177,50 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) } #endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS) +//------------------------------------------------------------------------ +// fgMorphModToZero: Transform a % 1 into the equivalent 0. +// +// Arguments: +// tree - The GT_MOD/GT_UMOD tree to morph +// +// Returns: +// The morphed tree, will be a GT_COMMA or a zero constant node. +// +GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) +{ + JITDUMP("\nMorphing MOD/UMOD [%06u] to Zero\n", dspTreeID(tree)); + + assert(tree->OperIs(GT_MOD, GT_UMOD)); + assert(tree->gtOp2->IsIntegralConst(1)); + + GenTree* op1 = tree->gtGetOp1(); + GenTree* op2 = tree->gtGetOp2(); + + const var_types type = tree->TypeGet(); + + GenTree* zero = gtNewZeroConNode(type); + + if (op1->IsInvariant() || op1->IsLocal()) + { + INDEBUG(zero->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + + DEBUG_DESTROY_NODE(tree->gtOp2); + DEBUG_DESTROY_NODE(tree->gtOp1); + DEBUG_DESTROY_NODE(tree); + + return zero; + } + + GenTree* comma = gtNewOperNode(GT_COMMA, tree->gtType, op1, zero); + + INDEBUG(comma->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + + DEBUG_DESTROY_NODE(tree->gtOp2); + DEBUG_DESTROY_NODE(tree); + + return comma; +} + //------------------------------------------------------------------------ // fgMorphModToSubMulDiv: Transform a % b into the equivalent a - (a / b) * b // (see ECMA III 3.55 and III.3.56). diff --git a/src/tests/JIT/opt/Remainder/IntRemainder.cs b/src/tests/JIT/opt/Remainder/IntRemainder.cs new file mode 100644 index 0000000000000..28fbc8f80c4d8 --- /dev/null +++ b/src/tests/JIT/opt/Remainder/IntRemainder.cs @@ -0,0 +1,25 @@ +// 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; + +namespace CodeGenTests +{ + static class IntRemainder + { + [MethodImpl(MethodImplOptions.NoInlining)] + static int Int32_RemainderByOne(int value) + { + return value % 1; + } + + static int Main() + { + if (Int32_RemainderByOne(-123) != 0) + return 0; + + return 100; + } + } +} diff --git a/src/tests/JIT/opt/Remainder/IntRemainder.csproj b/src/tests/JIT/opt/Remainder/IntRemainder.csproj new file mode 100644 index 0000000000000..42a89c8384d74 --- /dev/null +++ b/src/tests/JIT/opt/Remainder/IntRemainder.csproj @@ -0,0 +1,17 @@ + + + Exe + + + None + True + + + + true + + + + + + From b16ce6f6616ee7832dcc68ce51eb72a6a1fda49c Mon Sep 17 00:00:00 2001 From: TIHan Date: Tue, 1 Nov 2022 17:59:55 -0700 Subject: [PATCH 02/23] Update disasm tests --- src/tests/JIT/opt/Remainder/IntRemainder.cs | 25 ++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/tests/JIT/opt/Remainder/IntRemainder.cs b/src/tests/JIT/opt/Remainder/IntRemainder.cs index 28fbc8f80c4d8..184faebe59248 100644 --- a/src/tests/JIT/opt/Remainder/IntRemainder.cs +++ b/src/tests/JIT/opt/Remainder/IntRemainder.cs @@ -8,18 +8,37 @@ namespace CodeGenTests { static class IntRemainder { + static int _fieldValue = 123; + [MethodImpl(MethodImplOptions.NoInlining)] - static int Int32_RemainderByOne(int value) + static int Int32_RemainderByOne() { + // X64: call CORINFO + // X64-NEXT: xor [[REG0:[a-z]+]], [[REG0]] + + // ARM64: bl CORINFO + // ARM64-NEXT: mov [[REG0:[a-z0-9]+]], wzr + return _fieldValue % 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Int32_RemainderByOneWithValue(int value) + { + // X64: xor [[REG0:[a-z]+]], [[REG0]] + + // ARM64: mov [[REG0:[a-z0-9]+]], wzr return value % 1; } static int Main() { - if (Int32_RemainderByOne(-123) != 0) + if (Int32_RemainderByOne() != 0) + return 0; + + if (Int32_RemainderByOneWithValue(-123) != 0) return 0; return 100; } } -} +} \ No newline at end of file From 71c19d1dfb5ad082f58b499ad6de28b63ae835ba Mon Sep 17 00:00:00 2001 From: TIHan Date: Tue, 1 Nov 2022 18:28:29 -0700 Subject: [PATCH 03/23] Minor change --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 7d2f6db04e174..4bf52667cbc1f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13211,7 +13211,7 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) return zero; } - GenTree* comma = gtNewOperNode(GT_COMMA, tree->gtType, op1, zero); + GenTree* comma = gtNewOperNode(GT_COMMA, type, op1, zero); INDEBUG(comma->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); From ad1fab77ad4c50d2ce7d8d23dfa3f60689690c2f Mon Sep 17 00:00:00 2001 From: TIHan Date: Wed, 2 Nov 2022 00:06:40 -0700 Subject: [PATCH 04/23] Remove duplicate opt --- src/coreclr/jit/morph.cpp | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 4bf52667cbc1f..b7d4a9713854c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9869,23 +9869,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA ASSIGN_HELPER_FOR_MOD: - // For "val % 1", return 0 if op1 doesn't have any side effects - // and we are not in the CSE phase, we cannot discard 'tree' - // because it may contain CSE expressions that we haven't yet examined. - // - if (((op1->gtFlags & GTF_SIDE_EFFECT) == 0) && !optValnumCSE_phase) - { - if (op2->IsIntegralConst(1)) - { - GenTree* zeroNode = gtNewZeroConNode(typ); -#ifdef DEBUG - zeroNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; -#endif - DEBUG_DESTROY_NODE(tree); - return zeroNode; - } - } - #ifndef TARGET_64BIT if (typ == TYP_LONG) { From daaa0c4dc021256b0e50bcc5f8b0c0b36a1b6e0c Mon Sep 17 00:00:00 2001 From: TIHan Date: Wed, 2 Nov 2022 09:39:21 -0700 Subject: [PATCH 05/23] Use extract side effects --- src/coreclr/jit/morph.cpp | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b7d4a9713854c..d3539a06261e5 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9922,7 +9922,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA #ifdef TARGET_ARM64 // ARM64 architecture manual suggests this transformation // for the mod operator. - else + else if (tree->OperIs(GT_MOD, GT_UMOD)) #else // XARCH only applies this transformation if we know // that magic division will be used - which is determined @@ -13183,7 +13183,20 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) GenTree* zero = gtNewZeroConNode(type); - if (op1->IsInvariant() || op1->IsLocal()) + GenTree* op1SideEffects = nullptr; + gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); + if (op1SideEffects != nullptr) + { + GenTree* comma = gtNewOperNode(GT_COMMA, type, op1SideEffects, zero); + + INDEBUG(comma->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + + DEBUG_DESTROY_NODE(tree->gtOp2); + DEBUG_DESTROY_NODE(tree); + + return comma; + } + else { INDEBUG(zero->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); @@ -13193,15 +13206,6 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) return zero; } - - GenTree* comma = gtNewOperNode(GT_COMMA, type, op1, zero); - - INDEBUG(comma->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - - DEBUG_DESTROY_NODE(tree->gtOp2); - DEBUG_DESTROY_NODE(tree); - - return comma; } //------------------------------------------------------------------------ From 79aff96190a7b3d311d4692651aab56fffb72199 Mon Sep 17 00:00:00 2001 From: TIHan Date: Wed, 2 Nov 2022 16:54:10 -0700 Subject: [PATCH 06/23] Re-arranging mod optimizations --- src/coreclr/jit/morph.cpp | 83 ++++++++++++++------------------------- 1 file changed, 30 insertions(+), 53 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d3539a06261e5..dc2af1bb46fbb 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9869,55 +9869,18 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA ASSIGN_HELPER_FOR_MOD: -#ifndef TARGET_64BIT - if (typ == TYP_LONG) - { - helper = (oper == GT_UMOD) ? CORINFO_HELP_ULMOD : CORINFO_HELP_LMOD; - goto USE_HELPER_FOR_ARITH; - } - -#if USE_HELPERS_FOR_INT_DIV - if (typ == TYP_INT) - { - if (oper == GT_UMOD) - { - helper = CORINFO_HELP_UMOD; - goto USE_HELPER_FOR_ARITH; - } - else if (oper == GT_MOD) - { - helper = CORINFO_HELP_MOD; - goto USE_HELPER_FOR_ARITH; - } - } -#endif -#endif // !TARGET_64BIT - if (!optValnumCSE_phase) { if (tree->OperIs(GT_MOD, GT_UMOD) && op2->IsIntegralConst(1)) { // Transformation: a % 1 = 0 - tree = fgMorphModToZero(tree->AsOp()); - if (tree->OperIsBinary()) - { - op1 = tree->gtGetOp1(); - op2 = tree->gtGetOp2(); - } - else - { - assert(tree->IsIntegralConst(0)); - op1 = nullptr; - op2 = nullptr; - } + return fgMorphModToZero(tree->AsOp()); } if (tree->OperIs(GT_UMOD) && op2->IsIntegralConstUnsignedPow2()) { // Transformation: a % b = a & (b - 1); - tree = fgMorphUModToAndSub(tree->AsOp()); - op1 = tree->AsOp()->gtOp1; - op2 = tree->AsOp()->gtOp2; + return fgMorphUModToAndSub(tree->AsOp()); } #ifdef TARGET_ARM64 // ARM64 architecture manual suggests this transformation @@ -9933,11 +9896,33 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA #endif { // Transformation: a % b = a - (a / b) * b; - tree = fgMorphModToSubMulDiv(tree->AsOp()); - op1 = tree->AsOp()->gtOp1; - op2 = tree->AsOp()->gtOp2; + return fgMorphModToSubMulDiv(tree->AsOp()); } } + +#ifndef TARGET_64BIT + if (typ == TYP_LONG) + { + helper = (oper == GT_UMOD) ? CORINFO_HELP_ULMOD : CORINFO_HELP_LMOD; + goto USE_HELPER_FOR_ARITH; + } + +#if USE_HELPERS_FOR_INT_DIV + if (typ == TYP_INT) + { + if (oper == GT_UMOD) + { + helper = CORINFO_HELP_UMOD; + goto USE_HELPER_FOR_ARITH; + } + else if (oper == GT_MOD) + { + helper = CORINFO_HELP_MOD; + goto USE_HELPER_FOR_ARITH; + } + } +#endif +#endif // !TARGET_64BIT break; USE_HELPER_FOR_ARITH: @@ -13189,12 +13174,10 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) { GenTree* comma = gtNewOperNode(GT_COMMA, type, op1SideEffects, zero); - INDEBUG(comma->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - DEBUG_DESTROY_NODE(tree->gtOp2); DEBUG_DESTROY_NODE(tree); - return comma; + return fgMorphTree(comma); } else { @@ -13330,13 +13313,9 @@ GenTree* Compiler::fgMorphModToSubMulDiv(GenTreeOp* tree) result = gtNewOperNode(GT_COMMA, type, tempInfos[i].asg, result); } -#ifdef DEBUG - result->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; -#endif - div->CheckDivideByConstOptimized(this); - return result; + return fgMorphTree(result); } //------------------------------------------------------------------------ @@ -13366,12 +13345,10 @@ GenTree* Compiler::fgMorphUModToAndSub(GenTreeOp* tree) const size_t cnsValue = (static_cast(tree->gtOp2->AsIntConCommon()->IntegralValue())) - 1; GenTree* const newTree = gtNewOperNode(GT_AND, type, tree->gtOp1, gtNewIconNode(cnsValue, type)); - INDEBUG(newTree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - DEBUG_DESTROY_NODE(tree->gtOp2); DEBUG_DESTROY_NODE(tree); - return newTree; + return fgMorphTree(newTree); } //------------------------------------------------------------------------------ From 3428879b76ae9bc19acf0f0be1f4dd119449818f Mon Sep 17 00:00:00 2001 From: TIHan Date: Wed, 2 Nov 2022 17:29:08 -0700 Subject: [PATCH 07/23] Re-arranging mod optimizations --- src/coreclr/jit/morph.cpp | 46 ++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index dc2af1bb46fbb..302ad5ddec94e 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9876,28 +9876,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA // Transformation: a % 1 = 0 return fgMorphModToZero(tree->AsOp()); } - - if (tree->OperIs(GT_UMOD) && op2->IsIntegralConstUnsignedPow2()) - { - // Transformation: a % b = a & (b - 1); - return fgMorphUModToAndSub(tree->AsOp()); - } -#ifdef TARGET_ARM64 - // ARM64 architecture manual suggests this transformation - // for the mod operator. - else if (tree->OperIs(GT_MOD, GT_UMOD)) -#else - // XARCH only applies this transformation if we know - // that magic division will be used - which is determined - // when 'b' is not a power of 2 constant and mod operator is signed. - // Lowering for XARCH does this optimization already, - // but is also done here to take advantage of CSE. - else if (tree->OperIs(GT_MOD) && op2->IsIntegralConst() && !op2->IsIntegralConstAbsPow2()) -#endif - { - // Transformation: a % b = a - (a / b) * b; - return fgMorphModToSubMulDiv(tree->AsOp()); - } } #ifndef TARGET_64BIT @@ -9923,6 +9901,30 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } #endif #endif // !TARGET_64BIT + if (!optValnumCSE_phase) + { + if (tree->OperIs(GT_UMOD) && op2->IsIntegralConstUnsignedPow2()) + { + // Transformation: a % b = a & (b - 1); + return fgMorphUModToAndSub(tree->AsOp()); + } +#ifdef TARGET_ARM64 + // ARM64 architecture manual suggests this transformation + // for the mod operator. + else if (tree->OperIs(GT_MOD, GT_UMOD)) +#else + // XARCH only applies this transformation if we know + // that magic division will be used - which is determined + // when 'b' is not a power of 2 constant and mod operator is signed. + // Lowering for XARCH does this optimization already, + // but is also done here to take advantage of CSE. + else if (tree->OperIs(GT_MOD) && op2->IsIntegralConst() && !op2->IsIntegralConstAbsPow2()) +#endif + { + // Transformation: a % b = a - (a / b) * b; + return fgMorphModToSubMulDiv(tree->AsOp()); + } + } break; USE_HELPER_FOR_ARITH: From a6af61e05ee9bae32e76723c459bb9ae5cab40d0 Mon Sep 17 00:00:00 2001 From: TIHan Date: Fri, 4 Nov 2022 19:19:05 -0700 Subject: [PATCH 08/23] Extended mod optimization with -1. Unsetting debug node morphed flag. --- src/coreclr/jit/morph.cpp | 13 +++++++--- src/tests/JIT/opt/Remainder/IntRemainder.cs | 26 +++++++++++++++++++ .../Regressions/Regression1/Regression1.cs | 16 ++++++++++++ .../Regression1/Regression1.csproj | 12 +++++++++ 4 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs create mode 100644 src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 302ad5ddec94e..b90c80da5bd4d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9871,9 +9871,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (!optValnumCSE_phase) { - if (tree->OperIs(GT_MOD, GT_UMOD) && op2->IsIntegralConst(1)) + if (tree->OperIs(GT_MOD, GT_UMOD) && (op2->IsIntegralConst(1) || op2->IsIntegralConst(-1))) { // Transformation: a % 1 = 0 + // Transformation: a % -1 = 0 return fgMorphModToZero(tree->AsOp()); } } @@ -13148,7 +13149,7 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) #endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS) //------------------------------------------------------------------------ -// fgMorphModToZero: Transform a % 1 into the equivalent 0. +// fgMorphModToZero: Transform 'a % 1' or 'a % -1' into the equivalent '0'. // // Arguments: // tree - The GT_MOD/GT_UMOD tree to morph @@ -13161,7 +13162,7 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) JITDUMP("\nMorphing MOD/UMOD [%06u] to Zero\n", dspTreeID(tree)); assert(tree->OperIs(GT_MOD, GT_UMOD)); - assert(tree->gtOp2->IsIntegralConst(1)); + assert(tree->gtOp2->IsIntegralConst(1) || tree->gtOp2->IsIntegralConst(-1)); GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); @@ -13176,9 +13177,15 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) { GenTree* comma = gtNewOperNode(GT_COMMA, type, op1SideEffects, zero); +#ifdef DEBUG + // op1 may already have been morphed, so unset this bit. + op1SideEffects->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; +#endif // DEBUG + DEBUG_DESTROY_NODE(tree->gtOp2); DEBUG_DESTROY_NODE(tree); + return fgMorphTree(comma); } else diff --git a/src/tests/JIT/opt/Remainder/IntRemainder.cs b/src/tests/JIT/opt/Remainder/IntRemainder.cs index 184faebe59248..c2910da686b54 100644 --- a/src/tests/JIT/opt/Remainder/IntRemainder.cs +++ b/src/tests/JIT/opt/Remainder/IntRemainder.cs @@ -30,6 +30,26 @@ static int Int32_RemainderByOneWithValue(int value) return value % 1; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int Int32_RemainderByNegativeOne() + { + // X64: call CORINFO + // X64-NEXT: xor [[REG0:[a-z]+]], [[REG0]] + + // ARM64: bl CORINFO + // ARM64-NEXT: mov [[REG0:[a-z0-9]+]], wzr + return _fieldValue % -1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Int32_RemainderByNegativeOneWithValue(int value) + { + // X64: xor [[REG0:[a-z]+]], [[REG0]] + + // ARM64: mov [[REG0:[a-z0-9]+]], wzr + return value % -1; + } + static int Main() { if (Int32_RemainderByOne() != 0) @@ -38,6 +58,12 @@ static int Main() if (Int32_RemainderByOneWithValue(-123) != 0) return 0; + if (Int32_RemainderByNegativeOne() != 0) + return 0; + + if (Int32_RemainderByNegativeOneWithValue(-123) != 0) + return 0; + return 100; } } diff --git a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs new file mode 100644 index 0000000000000..8fbe4f8cbb9ca --- /dev/null +++ b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +public class Program +{ + public static ulong[,] s_1; + public static int Main() + { + // This should not assert. + + ushort vr10 = default(ushort); + bool vr11 = 0 < ((s_1[0, 0] * (uint)(0 / vr10)) % 1); + + return 100; + } +} \ No newline at end of file diff --git a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj new file mode 100644 index 0000000000000..f3e1cbd44b404 --- /dev/null +++ b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + From ea19049798d8802a9ccaf82218731f54d8baf0de Mon Sep 17 00:00:00 2001 From: TIHan Date: Fri, 4 Nov 2022 19:28:28 -0700 Subject: [PATCH 09/23] Remove space --- src/coreclr/jit/morph.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b90c80da5bd4d..30033434a722d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13185,7 +13185,6 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) DEBUG_DESTROY_NODE(tree->gtOp2); DEBUG_DESTROY_NODE(tree); - return fgMorphTree(comma); } else From 9c1d6792baf03d91d3d0cd233665049403bdbc97 Mon Sep 17 00:00:00 2001 From: TIHan Date: Mon, 7 Nov 2022 11:10:43 -0800 Subject: [PATCH 10/23] Try catch the regression --- .../opt/Remainder/Regressions/Regression1/Regression1.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs index 8fbe4f8cbb9ca..1cb829af3de5b 100644 --- a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs +++ b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.cs @@ -7,9 +7,12 @@ public class Program public static int Main() { // This should not assert. - - ushort vr10 = default(ushort); - bool vr11 = 0 < ((s_1[0, 0] * (uint)(0 / vr10)) % 1); + try + { + ushort vr10 = default(ushort); + bool vr11 = 0 < ((s_1[0, 0] * (uint)(0 / vr10)) % 1); + } + catch {} return 100; } From 8b11f51ec1a39b02e1a5e3877efa6aa683352ed6 Mon Sep 17 00:00:00 2001 From: TIHan Date: Mon, 7 Nov 2022 12:48:22 -0800 Subject: [PATCH 11/23] Use FULL-LINE for tests --- src/tests/JIT/opt/Remainder/IntRemainder.cs | 28 ++++++++++++--------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/tests/JIT/opt/Remainder/IntRemainder.cs b/src/tests/JIT/opt/Remainder/IntRemainder.cs index c2910da686b54..1582dce63fc9b 100644 --- a/src/tests/JIT/opt/Remainder/IntRemainder.cs +++ b/src/tests/JIT/opt/Remainder/IntRemainder.cs @@ -13,40 +13,44 @@ static class IntRemainder [MethodImpl(MethodImplOptions.NoInlining)] static int Int32_RemainderByOne() { - // X64: call CORINFO - // X64-NEXT: xor [[REG0:[a-z]+]], [[REG0]] + // X64-FULL-LINE: call CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE + // X64-FULL-LINE-NEXT: xor [[REG0:[a-z]+]], [[REG0]] - // ARM64: bl CORINFO - // ARM64-NEXT: mov [[REG0:[a-z0-9]+]], wzr + // ARM64-FULL-LINE: bl CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE + // ARM64-FULL-LINE-NEXT: mov [[REG0:[a-z0-9]+]], wzr + return _fieldValue % 1; } [MethodImpl(MethodImplOptions.NoInlining)] static int Int32_RemainderByOneWithValue(int value) { - // X64: xor [[REG0:[a-z]+]], [[REG0]] + // X64-FULL-LINE: xor [[REG0:[a-z]+]], [[REG0]] + + // ARM64-FULL-LINE: mov [[REG0:[a-z0-9]+]], wzr - // ARM64: mov [[REG0:[a-z0-9]+]], wzr return value % 1; } [MethodImpl(MethodImplOptions.NoInlining)] static int Int32_RemainderByNegativeOne() { - // X64: call CORINFO - // X64-NEXT: xor [[REG0:[a-z]+]], [[REG0]] + // X64-FULL-LINE: call CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE + // X64-FULL-LINE-NEXT: xor [[REG0:[a-z]+]], [[REG0]] + + // ARM64-FULL-LINE: bl CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE + // ARM64-FULL-LINE-NEXT: mov [[REG0:[a-z0-9]+]], wzr - // ARM64: bl CORINFO - // ARM64-NEXT: mov [[REG0:[a-z0-9]+]], wzr return _fieldValue % -1; } [MethodImpl(MethodImplOptions.NoInlining)] static int Int32_RemainderByNegativeOneWithValue(int value) { - // X64: xor [[REG0:[a-z]+]], [[REG0]] + // X64-FULL-LINE: xor [[REG0:[a-z]+]], [[REG0]] + + // ARM64-FULL-LINE: mov [[REG0:[a-z0-9]+]], wzr - // ARM64: mov [[REG0:[a-z0-9]+]], wzr return value % -1; } From c7579250ca13aba94a9cc8a86cbe273ed5487cbf Mon Sep 17 00:00:00 2001 From: TIHan Date: Mon, 7 Nov 2022 13:17:05 -0800 Subject: [PATCH 12/23] Remove '% -1' opt --- src/coreclr/jit/morph.cpp | 7 +++-- src/tests/JIT/opt/Remainder/IntRemainder.cs | 30 +-------------------- 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 30033434a722d..2b8436f8a26cc 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9871,10 +9871,9 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (!optValnumCSE_phase) { - if (tree->OperIs(GT_MOD, GT_UMOD) && (op2->IsIntegralConst(1) || op2->IsIntegralConst(-1))) + if (tree->OperIs(GT_MOD, GT_UMOD) && (op2->IsIntegralConst(1))) { // Transformation: a % 1 = 0 - // Transformation: a % -1 = 0 return fgMorphModToZero(tree->AsOp()); } } @@ -13149,7 +13148,7 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) #endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS) //------------------------------------------------------------------------ -// fgMorphModToZero: Transform 'a % 1' or 'a % -1' into the equivalent '0'. +// fgMorphModToZero: Transform 'a % 1' into the equivalent '0'. // // Arguments: // tree - The GT_MOD/GT_UMOD tree to morph @@ -13162,7 +13161,7 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) JITDUMP("\nMorphing MOD/UMOD [%06u] to Zero\n", dspTreeID(tree)); assert(tree->OperIs(GT_MOD, GT_UMOD)); - assert(tree->gtOp2->IsIntegralConst(1) || tree->gtOp2->IsIntegralConst(-1)); + assert(tree->gtOp2->IsIntegralConst(1)); GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); diff --git a/src/tests/JIT/opt/Remainder/IntRemainder.cs b/src/tests/JIT/opt/Remainder/IntRemainder.cs index 1582dce63fc9b..0783fae3067f8 100644 --- a/src/tests/JIT/opt/Remainder/IntRemainder.cs +++ b/src/tests/JIT/opt/Remainder/IntRemainder.cs @@ -18,7 +18,7 @@ static int Int32_RemainderByOne() // ARM64-FULL-LINE: bl CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE // ARM64-FULL-LINE-NEXT: mov [[REG0:[a-z0-9]+]], wzr - + return _fieldValue % 1; } @@ -32,28 +32,6 @@ static int Int32_RemainderByOneWithValue(int value) return value % 1; } - [MethodImpl(MethodImplOptions.NoInlining)] - static int Int32_RemainderByNegativeOne() - { - // X64-FULL-LINE: call CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE - // X64-FULL-LINE-NEXT: xor [[REG0:[a-z]+]], [[REG0]] - - // ARM64-FULL-LINE: bl CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE - // ARM64-FULL-LINE-NEXT: mov [[REG0:[a-z0-9]+]], wzr - - return _fieldValue % -1; - } - - [MethodImpl(MethodImplOptions.NoInlining)] - static int Int32_RemainderByNegativeOneWithValue(int value) - { - // X64-FULL-LINE: xor [[REG0:[a-z]+]], [[REG0]] - - // ARM64-FULL-LINE: mov [[REG0:[a-z0-9]+]], wzr - - return value % -1; - } - static int Main() { if (Int32_RemainderByOne() != 0) @@ -62,12 +40,6 @@ static int Main() if (Int32_RemainderByOneWithValue(-123) != 0) return 0; - if (Int32_RemainderByNegativeOne() != 0) - return 0; - - if (Int32_RemainderByNegativeOneWithValue(-123) != 0) - return 0; - return 100; } } From 87fdc1d0c53f3e1dcc745a16902b0d4a056523f3 Mon Sep 17 00:00:00 2001 From: TIHan Date: Tue, 8 Nov 2022 11:32:57 -0800 Subject: [PATCH 13/23] Updated vn for constant. Re-using op2 for zero constant. --- src/coreclr/jit/morph.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 2b8436f8a26cc..f045990094b30 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13166,22 +13166,22 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); - const var_types type = tree->TypeGet(); + op2->AsIntConCommon()->SetIconValue(0); + fgUpdateConstTreeValueNumber(op2); - GenTree* zero = gtNewZeroConNode(type); + GenTree* const zero = op2; GenTree* op1SideEffects = nullptr; gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); if (op1SideEffects != nullptr) { - GenTree* comma = gtNewOperNode(GT_COMMA, type, op1SideEffects, zero); + GenTree* comma = gtNewOperNode(GT_COMMA, zero->TypeGet(), op1SideEffects, zero); #ifdef DEBUG // op1 may already have been morphed, so unset this bit. op1SideEffects->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; #endif // DEBUG - DEBUG_DESTROY_NODE(tree->gtOp2); DEBUG_DESTROY_NODE(tree); return fgMorphTree(comma); @@ -13190,7 +13190,6 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) { INDEBUG(zero->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - DEBUG_DESTROY_NODE(tree->gtOp2); DEBUG_DESTROY_NODE(tree->gtOp1); DEBUG_DESTROY_NODE(tree); From 05086f907b2dffe4186995e90f169c6566f38f4f Mon Sep 17 00:00:00 2001 From: TIHan Date: Mon, 14 Nov 2022 10:40:46 -0800 Subject: [PATCH 14/23] Do not optimize part of this if not global morph --- src/coreclr/jit/morph.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 5a858b145fc6a..e3cf38c87b7c8 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9500,7 +9500,11 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (tree->OperIs(GT_MOD, GT_UMOD) && (op2->IsIntegralConst(1))) { // Transformation: a % 1 = 0 - return fgMorphModToZero(tree->AsOp()); + GenTree* optimizedTree = fgMorphModToZero(tree->AsOp()); + if (optimizedTree != nullptr) + { + return optimizedTree; + } } } @@ -12769,6 +12773,7 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) // // Returns: // The morphed tree, will be a GT_COMMA or a zero constant node. +// Can return null if the transformation cannot happen. // GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) { @@ -12789,6 +12794,11 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); if (op1SideEffects != nullptr) { + // Do not transform this if there are side effects and we are not in global morph. + // If we want to allow this, we need to update value numbers for the GT_COMMA. + if (!fgGlobalMorph) + return nullptr; + GenTree* comma = gtNewOperNode(GT_COMMA, zero->TypeGet(), op1SideEffects, zero); #ifdef DEBUG From 84f12f462b924680d97263f32f1a36a25b3c9d4f Mon Sep 17 00:00:00 2001 From: TIHan Date: Tue, 15 Nov 2022 11:04:30 -0800 Subject: [PATCH 15/23] Checking side effect flag early --- src/coreclr/jit/morph.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index e3cf38c87b7c8..c55c17b5d15eb 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12777,11 +12777,16 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) // GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) { - JITDUMP("\nMorphing MOD/UMOD [%06u] to Zero\n", dspTreeID(tree)); - assert(tree->OperIs(GT_MOD, GT_UMOD)); assert(tree->gtOp2->IsIntegralConst(1)); + // Do not transform this if there are side effects and we are not in global morph. + // If we want to allow this, we need to update value numbers for the GT_COMMA. + if (!fgGlobalMorph && ((tree->gtGetOp1()->gtFlags & GTF_SIDE_EFFECT) != 0)) + return nullptr; + + JITDUMP("\nMorphing MOD/UMOD [%06u] to Zero\n", dspTreeID(tree)); + GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); @@ -12794,11 +12799,6 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); if (op1SideEffects != nullptr) { - // Do not transform this if there are side effects and we are not in global morph. - // If we want to allow this, we need to update value numbers for the GT_COMMA. - if (!fgGlobalMorph) - return nullptr; - GenTree* comma = gtNewOperNode(GT_COMMA, zero->TypeGet(), op1SideEffects, zero); #ifdef DEBUG From 41f3a56ac97010748c157d9e0b683eadc22ace03 Mon Sep 17 00:00:00 2001 From: TIHan Date: Mon, 28 Nov 2022 12:27:57 -0800 Subject: [PATCH 16/23] Check for GT_CNS_LNG --- src/coreclr/jit/morph.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index c55c17b5d15eb..fc001cc477cd8 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12790,7 +12790,15 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); - op2->AsIntConCommon()->SetIconValue(0); + if (op2->OperIs(GT_CNS_LNG)) + { + op2->AsIntConCommon()->SetLngValue(0); + } + else + { + op2->AsIntConCommon()->SetIconValue(0); + } + fgUpdateConstTreeValueNumber(op2); GenTree* const zero = op2; From 4b8bccc3cf2834f0f7d47191c4ed20f6efb36b9b Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 29 Nov 2022 10:42:28 -0800 Subject: [PATCH 17/23] Update Regression1.csproj --- .../JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj index f3e1cbd44b404..675d36d50e8db 100644 --- a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj +++ b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj @@ -1,6 +1,7 @@ Exe + true None From eb6f8609d2f50d44d202ed8fad3be68b1236aeb1 Mon Sep 17 00:00:00 2001 From: TIHan Date: Tue, 29 Nov 2022 18:54:20 -0800 Subject: [PATCH 18/23] Recording issue for mono --- .../opt/Remainder/Regressions/Regression1/Regression1.csproj | 1 - src/tests/issues.targets | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj index 675d36d50e8db..f3e1cbd44b404 100644 --- a/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj +++ b/src/tests/JIT/opt/Remainder/Regressions/Regression1/Regression1.csproj @@ -1,7 +1,6 @@ Exe - true None diff --git a/src/tests/issues.targets b/src/tests/issues.targets index f39b00887245e..8491d9a32bbd1 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1354,6 +1354,9 @@ + + https://github.com/dotnet/runtime/issues/79022 + https://github.com/dotnet/runtime/issues/74223 From 6504bcda9382c1ab200ea1058e1457458e54e35c Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 1 Dec 2022 12:53:59 -0800 Subject: [PATCH 19/23] Update issues.targets --- src/tests/issues.targets | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 8491d9a32bbd1..049bcda91a4f2 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1354,7 +1354,7 @@ - + https://github.com/dotnet/runtime/issues/79022 @@ -4052,4 +4052,4 @@ Condition="'%(ExcludeList.Extension)' == '.OutOfProcessTest'" /> - \ No newline at end of file + From 7bc349d8e93895b5d45563d38dca98ffda870ce8 Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 1 Dec 2022 14:23:12 -0800 Subject: [PATCH 20/23] Using SetIntegralValue, removed unrelated changes, transformation will not happen if optimizations are disabled --- src/coreclr/jit/morph.cpp | 52 ++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index fc001cc477cd8..66c09a3cf5a62 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9503,7 +9503,20 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA GenTree* optimizedTree = fgMorphModToZero(tree->AsOp()); if (optimizedTree != nullptr) { - return optimizedTree; + tree = optimizedTree; + + if (tree->OperIs(GT_COMMA)) + { + op1 = tree->gtGetOp1(); + op2 = tree->gtGetOp2(); + } + else + { + assert(tree->IsCnsIntOrI()); + op1 = nullptr; + op2 = nullptr; + } + break; } } } @@ -9536,12 +9549,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (tree->OperIs(GT_UMOD) && op2->IsIntegralConstUnsignedPow2()) { // Transformation: a % b = a & (b - 1); - return fgMorphUModToAndSub(tree->AsOp()); + tree = fgMorphUModToAndSub(tree->AsOp()); + op1 = tree->AsOp()->gtOp1; + op2 = tree->AsOp()->gtOp2; } #ifdef TARGET_ARM64 // ARM64 architecture manual suggests this transformation // for the mod operator. - else if (tree->OperIs(GT_MOD, GT_UMOD)) + else #else // XARCH only applies this transformation if we know // that magic division will be used - which is determined @@ -9552,7 +9567,9 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA #endif { // Transformation: a % b = a - (a / b) * b; - return fgMorphModToSubMulDiv(tree->AsOp()); + tree = fgMorphModToSubMulDiv(tree->AsOp()); + op1 = tree->AsOp()->gtOp1; + op2 = tree->AsOp()->gtOp2; } } break; @@ -12773,13 +12790,16 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) // // Returns: // The morphed tree, will be a GT_COMMA or a zero constant node. -// Can return null if the transformation cannot happen. +// Can return null if the transformation did not happen. // GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) { assert(tree->OperIs(GT_MOD, GT_UMOD)); assert(tree->gtOp2->IsIntegralConst(1)); + if (opts.OptimizationDisabled()) + return nullptr; + // Do not transform this if there are side effects and we are not in global morph. // If we want to allow this, we need to update value numbers for the GT_COMMA. if (!fgGlobalMorph && ((tree->gtGetOp1()->gtFlags & GTF_SIDE_EFFECT) != 0)) @@ -12790,15 +12810,7 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); - if (op2->OperIs(GT_CNS_LNG)) - { - op2->AsIntConCommon()->SetLngValue(0); - } - else - { - op2->AsIntConCommon()->SetIconValue(0); - } - + op2->AsIntConCommon()->SetIntegralValue(0); fgUpdateConstTreeValueNumber(op2); GenTree* const zero = op2; @@ -12816,7 +12828,7 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) DEBUG_DESTROY_NODE(tree); - return fgMorphTree(comma); + return comma; } else { @@ -12951,9 +12963,13 @@ GenTree* Compiler::fgMorphModToSubMulDiv(GenTreeOp* tree) result = gtNewOperNode(GT_COMMA, type, tempInfos[i].asg, result); } +#ifdef DEBUG + result->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; +#endif + div->CheckDivideByConstOptimized(this); - return fgMorphTree(result); + return result; } //------------------------------------------------------------------------ @@ -12983,10 +12999,12 @@ GenTree* Compiler::fgMorphUModToAndSub(GenTreeOp* tree) const size_t cnsValue = (static_cast(tree->gtOp2->AsIntConCommon()->IntegralValue())) - 1; GenTree* const newTree = gtNewOperNode(GT_AND, type, tree->gtOp1, gtNewIconNode(cnsValue, type)); + INDEBUG(newTree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + DEBUG_DESTROY_NODE(tree->gtOp2); DEBUG_DESTROY_NODE(tree); - return fgMorphTree(newTree); + return newTree; } //------------------------------------------------------------------------------ From 0c225eeaaab7eab30c4d1d073c61554d08d379dd Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 1 Dec 2022 14:24:19 -0800 Subject: [PATCH 21/23] Minor change --- src/coreclr/jit/morph.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 66c09a3cf5a62..6dcc9c44f7a48 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9544,6 +9544,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } #endif #endif // !TARGET_64BIT + if (!optValnumCSE_phase) { if (tree->OperIs(GT_UMOD) && op2->IsIntegralConstUnsignedPow2()) From 475e4d6c446c60af76d76135ae1e8a4eec2aa218 Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 1 Dec 2022 14:25:36 -0800 Subject: [PATCH 22/23] Minor change --- src/coreclr/jit/morph.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 6dcc9c44f7a48..6df7fc1c5ae7c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12827,6 +12827,8 @@ GenTree* Compiler::fgMorphModToZero(GenTreeOp* tree) op1SideEffects->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; #endif // DEBUG + INDEBUG(comma->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + DEBUG_DESTROY_NODE(tree); return comma; From 2c37388df14e85198fabfa16a512313748e2d9b6 Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 1 Dec 2022 17:20:57 -0800 Subject: [PATCH 23/23] Use IsIntegralConst --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 6df7fc1c5ae7c..2e35e2d4d7e61 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9512,7 +9512,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } else { - assert(tree->IsCnsIntOrI()); + assert(tree->IsIntegralConst()); op1 = nullptr; op2 = nullptr; }