From 8f2935dee36ac3dc856f1090c8302fbff639c3e7 Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Fri, 3 Jun 2022 23:36:10 +0300 Subject: [PATCH 01/13] Optimization for full range checks (#70145) --- src/coreclr/jit/morph.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 48b88f100d473..eed10155dadfc 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10790,6 +10790,38 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } } } + + if (op1->OperIs(GT_LT)) + { + GenTree* lNode = op1->gtGetOp1(); + GenTree* rNode = op1->gtGetOp2(); + + if ((lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(0)) || + (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MIN)) || + (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(INT64_MIN))) + { + GenTree* ret = gtNewIconNode(1, TYP_INT); + ret->SetVNsFromNode(tree); + DEBUG_DESTROY_NODE(tree); + return fgMorphTree(ret); + } + } + + if (op1->OperIs(GT_GT)) + { + GenTree* lNode = op1->gtGetOp1(); + GenTree* rNode = op1->gtGetOp2(); + + if ((lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(BYTE_MAX)) || + (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MAX)) || + (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(INT64_MAX))) + { + GenTree* ret = gtNewIconNode(1, TYP_INT); + ret->SetVNsFromNode(tree); + DEBUG_DESTROY_NODE(tree); + return fgMorphTree(ret); + } + } } } From 81523b64bbf22de56d0011c3f3247057ff840887 Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Mon, 6 Jun 2022 18:52:23 +0300 Subject: [PATCH 02/13] Fix the x86 long value error (#70145) --- src/coreclr/jit/morph.cpp | 47 +++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index eed10155dadfc..1b06a40487d8c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10791,30 +10791,43 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } } - if (op1->OperIs(GT_LT)) + if (op1->OperIs(GT_LT, GT_GT)) { + bool fold = false; GenTree* lNode = op1->gtGetOp1(); GenTree* rNode = op1->gtGetOp2(); - - if ((lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(0)) || - (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MIN)) || - (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(INT64_MIN))) +#if defined(HOST_X86) + ssize_t longMin = INT32_MIN; + ssize_t longMax = INT32_MAX; +#else + ssize_t longMin = INT64_MIN; + ssize_t longMax = INT64_MAX; +#endif + if (op1->OperIs(GT_LT)) { - GenTree* ret = gtNewIconNode(1, TYP_INT); - ret->SetVNsFromNode(tree); - DEBUG_DESTROY_NODE(tree); - return fgMorphTree(ret); + // Folds: + // 1. byte x >= 0 => true + // 2. int x >= int.MinValue => true + // 3. long x >= long.MaxValue => true + + fold = (lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(0)) || + (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MIN)) || + (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(longMin)); } - } - if (op1->OperIs(GT_GT)) - { - GenTree* lNode = op1->gtGetOp1(); - GenTree* rNode = op1->gtGetOp2(); + if (op1->OperIs(GT_GT)) + { + // Folds: + // 1. byte x <= byte.MaxValue => true + // 2. int x <= int.MaxValue => true + // 3. long x <= long.MaxValue => true + + fold = (lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(BYTE_MAX)) || + (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MAX)) || + (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(longMax)); + } - if ((lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(BYTE_MAX)) || - (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MAX)) || - (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(INT64_MAX))) + if (fold) { GenTree* ret = gtNewIconNode(1, TYP_INT); ret->SetVNsFromNode(tree); From 05022dd3947e23bcd6bd0620f30374b7bf9d5de6 Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Tue, 7 Jun 2022 09:51:53 +0300 Subject: [PATCH 03/13] Fix arm building errors (#70145) --- src/coreclr/jit/morph.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1b06a40487d8c..d8694420ee7dd 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10796,7 +10796,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) bool fold = false; GenTree* lNode = op1->gtGetOp1(); GenTree* rNode = op1->gtGetOp2(); -#if defined(HOST_X86) + +#if defined(HOST_X86) || defined(HOST_ARM) ssize_t longMin = INT32_MIN; ssize_t longMax = INT32_MAX; #else @@ -13070,7 +13071,7 @@ GenTree* Compiler::fgOptimizeMultiply(GenTreeOp* mul) // Should we try to replace integer multiplication with lea/add/shift sequences? bool mulShiftOpt = compCodeOpt() != SMALL_CODE; #else // !TARGET_XARCH - bool mulShiftOpt = false; + bool mulShiftOpt = false; #endif // !TARGET_XARCH size_t abs_mult = (mult >= 0) ? mult : -mult; From ebbaa3e2a98ef0d788ca9022bf26085527a566bd Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Wed, 8 Jun 2022 11:37:04 +0300 Subject: [PATCH 04/13] Moved code according to the comments (#70145) --- src/coreclr/jit/morph.cpp | 113 ++++++++++++++++++++++---------------- 1 file changed, 66 insertions(+), 47 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d8694420ee7dd..a9b9cca229b9b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10790,52 +10790,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } } } - - if (op1->OperIs(GT_LT, GT_GT)) - { - bool fold = false; - GenTree* lNode = op1->gtGetOp1(); - GenTree* rNode = op1->gtGetOp2(); - -#if defined(HOST_X86) || defined(HOST_ARM) - ssize_t longMin = INT32_MIN; - ssize_t longMax = INT32_MAX; -#else - ssize_t longMin = INT64_MIN; - ssize_t longMax = INT64_MAX; -#endif - if (op1->OperIs(GT_LT)) - { - // Folds: - // 1. byte x >= 0 => true - // 2. int x >= int.MinValue => true - // 3. long x >= long.MaxValue => true - - fold = (lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(0)) || - (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MIN)) || - (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(longMin)); - } - - if (op1->OperIs(GT_GT)) - { - // Folds: - // 1. byte x <= byte.MaxValue => true - // 2. int x <= int.MaxValue => true - // 3. long x <= long.MaxValue => true - - fold = (lNode->TypeIs(TYP_UBYTE) && rNode->IsIntegralConst(BYTE_MAX)) || - (lNode->TypeIs(TYP_INT) && rNode->IsIntegralConst(INT32_MAX)) || - (lNode->TypeIs(TYP_LONG) && rNode->IsIntegralConst(longMax)); - } - - if (fold) - { - GenTree* ret = gtNewIconNode(1, TYP_INT); - ret->SetVNsFromNode(tree); - DEBUG_DESTROY_NODE(tree); - return fgMorphTree(ret); - } - } } } @@ -11421,6 +11375,12 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) if (!optValnumCSE_phase && op2->IsIntegralConst()) { tree = fgOptimizeEqualityComparisonWithConst(tree->AsOp()); + + if (tree->OperIs(GT_CNS_INT, GT_COMMA)) + { + return tree; + } + assert(tree->OperIsCompare()); oper = tree->OperGet(); @@ -12524,6 +12484,65 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) op1->SetVNsFromNode(cmp); DEBUG_DESTROY_NODE(cmp); + + if (op1->OperIs(GT_GE, GT_LE) && (op1->gtGetOp2()->IsIntegralConst() || op1->gtGetOp1()->IsIntegralConst())) + { + GenTree* conNode = op1->gtGetOp2()->IsIntegralConst() ? op1->gtGetOp2() : op1->gtGetOp1(); + + IntegralRange valRange = IntegralRange::ForNode(conNode, this); + +#if defined(HOST_X86) || defined(HOST_ARM) + ssize_t valMin = + (int32_t)IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(conNode->TypeGet())); + ssize_t valMax = + (int32_t)IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(conNode->TypeGet())); +#else + ssize_t valMin = IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(conNode->TypeGet())); + ssize_t valMax = IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(conNode->TypeGet())); +#endif + // Folds + // 1. byte x <= 0 => true + // 2. int x <= int.MinValue => true + // 3. long x <= long.MaxValue => true + // 4. byte x >= byte.MaxValue => true + // 5. int x >= int.MaxValue => true + // 6. long x >= long.MaxValue => true + // + // when const is RHS: + if (op1->gtGetOp2()->IsIntegralConst()) + { + + if ((op1->OperIs(GT_GE) && conNode->IsIntegralConst(valMin)) || + (op1->OperIs(GT_LE) && conNode->IsIntegralConst(valMax))) + { + GenTree* ret = gtNewIconNode(1, TYP_INT); + ret->SetVNsFromNode(op1); + DEBUG_DESTROY_NODE(op1); + return fgMorphTree(ret); + } + } // when const is LHS: + else + { + if ((op1->OperIs(GT_GE) && conNode->IsIntegralConst(valMax)) || + (op1->OperIs(GT_LE) && conNode->IsIntegralConst(valMin))) + { + GenTree* cmpSideEffects = nullptr; + GenTree* ret = gtNewIconNode(1, TYP_INT); + + gtExtractSideEffList(op1, &cmpSideEffects); + + if (cmpSideEffects != nullptr) + { + ret = gtNewOperNode(GT_COMMA, TYP_INT, cmpSideEffects, ret); + } + + ret->SetVNsFromNode(op1); + DEBUG_DESTROY_NODE(op1); + return fgMorphTree(ret); + } + } + } + return op1; } @@ -13071,7 +13090,7 @@ GenTree* Compiler::fgOptimizeMultiply(GenTreeOp* mul) // Should we try to replace integer multiplication with lea/add/shift sequences? bool mulShiftOpt = compCodeOpt() != SMALL_CODE; #else // !TARGET_XARCH - bool mulShiftOpt = false; + bool mulShiftOpt = false; #endif // !TARGET_XARCH size_t abs_mult = (mult >= 0) ? mult : -mult; From 5548ead7ab445d151f162ba8835a26f8d06a4a52 Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Thu, 9 Jun 2022 16:30:26 +0300 Subject: [PATCH 05/13] Moved code to fgOptimizeRelationalComparisonWithConst (#70145) --- src/coreclr/jit/morph.cpp | 302 +++++++++++++-------- src/tests/JIT/opt/Compares/compares.cs | 204 ++++++++++++++ src/tests/JIT/opt/Compares/compares.csproj | 12 + 3 files changed, 400 insertions(+), 118 deletions(-) create mode 100644 src/tests/JIT/opt/Compares/compares.cs create mode 100644 src/tests/JIT/opt/Compares/compares.csproj diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index a9b9cca229b9b..846040407c652 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11376,11 +11376,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) { tree = fgOptimizeEqualityComparisonWithConst(tree->AsOp()); - if (tree->OperIs(GT_CNS_INT, GT_COMMA)) - { - return tree; - } - assert(tree->OperIsCompare()); oper = tree->OperGet(); @@ -11403,9 +11398,17 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } // op2's value may be changed, so it cannot be a CSE candidate. - if (op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2)) + // op1's value may be changed, so it cannot be a CSE candidate. + if ((op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2)) || + (op1->IsIntegralConst() && !gtIsActiveCSE_Candidate(op1))) { tree = fgOptimizeRelationalComparisonWithConst(tree->AsOp()); + + if (tree->OperIs(GT_CNS_INT, GT_COMMA)) + { + return tree; + } + oper = tree->OperGet(); assert(op1 == tree->AsOp()->gtGetOp1()); @@ -12485,64 +12488,6 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) DEBUG_DESTROY_NODE(cmp); - if (op1->OperIs(GT_GE, GT_LE) && (op1->gtGetOp2()->IsIntegralConst() || op1->gtGetOp1()->IsIntegralConst())) - { - GenTree* conNode = op1->gtGetOp2()->IsIntegralConst() ? op1->gtGetOp2() : op1->gtGetOp1(); - - IntegralRange valRange = IntegralRange::ForNode(conNode, this); - -#if defined(HOST_X86) || defined(HOST_ARM) - ssize_t valMin = - (int32_t)IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(conNode->TypeGet())); - ssize_t valMax = - (int32_t)IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(conNode->TypeGet())); -#else - ssize_t valMin = IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(conNode->TypeGet())); - ssize_t valMax = IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(conNode->TypeGet())); -#endif - // Folds - // 1. byte x <= 0 => true - // 2. int x <= int.MinValue => true - // 3. long x <= long.MaxValue => true - // 4. byte x >= byte.MaxValue => true - // 5. int x >= int.MaxValue => true - // 6. long x >= long.MaxValue => true - // - // when const is RHS: - if (op1->gtGetOp2()->IsIntegralConst()) - { - - if ((op1->OperIs(GT_GE) && conNode->IsIntegralConst(valMin)) || - (op1->OperIs(GT_LE) && conNode->IsIntegralConst(valMax))) - { - GenTree* ret = gtNewIconNode(1, TYP_INT); - ret->SetVNsFromNode(op1); - DEBUG_DESTROY_NODE(op1); - return fgMorphTree(ret); - } - } // when const is LHS: - else - { - if ((op1->OperIs(GT_GE) && conNode->IsIntegralConst(valMax)) || - (op1->OperIs(GT_LE) && conNode->IsIntegralConst(valMin))) - { - GenTree* cmpSideEffects = nullptr; - GenTree* ret = gtNewIconNode(1, TYP_INT); - - gtExtractSideEffList(op1, &cmpSideEffects); - - if (cmpSideEffects != nullptr) - { - ret = gtNewOperNode(GT_COMMA, TYP_INT, cmpSideEffects, ret); - } - - ret->SetVNsFromNode(op1); - DEBUG_DESTROY_NODE(op1); - return fgMorphTree(ret); - } - } - } - return op1; } @@ -12706,8 +12651,9 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) // cmp - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. // // Return Value: -// The "cmp" tree, possibly with a modified oper. +// 1. The "cmp" tree, possibly with a modified oper. // The second operand's constant value may be modified as well. +// 2. GT_CNS_INT node containing zero as value // // Assumptions: // The operands have been swapped so that any constants are on the right. @@ -12716,79 +12662,199 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) { assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); - assert(cmp->gtGetOp2()->IsIntegralConst()); - assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2())); - GenTree* op1 = cmp->gtGetOp1(); - GenTreeIntConCommon* op2 = cmp->gtGetOp2()->AsIntConCommon(); + if (cmp->gtGetOp2()->IsIntegralConst() && !gtIsActiveCSE_Candidate(cmp->gtGetOp2())) + { + assert(cmp->gtGetOp2()->IsIntegralConst()); + assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2())); - assert(genActualType(op1) == genActualType(op2)); + GenTree* op1 = cmp->gtGetOp1(); + GenTreeIntConCommon* op2 = cmp->gtGetOp2()->AsIntConCommon(); - genTreeOps oper = cmp->OperGet(); - int64_t op2Value = op2->IntegralValue(); + assert(genActualType(op1) == genActualType(op2)); - if (op2Value == 1) - { - // Check for "expr >= 1". - if (oper == GT_GE) + genTreeOps oper = cmp->OperGet(); + int64_t op2Value = op2->IntegralValue(); + + if (op2Value == 1) { - // Change to "expr != 0" for unsigned and "expr > 0" for signed. - oper = cmp->IsUnsigned() ? GT_NE : GT_GT; + // Check for "expr >= 1". + if (oper == GT_GE) + { + // Change to "expr != 0" for unsigned and "expr > 0" for signed. + oper = cmp->IsUnsigned() ? GT_NE : GT_GT; + } + // Check for "expr < 1". + else if (oper == GT_LT) + { + // Change to "expr == 0" for unsigned and "expr <= 0". + oper = cmp->IsUnsigned() ? GT_EQ : GT_LE; + } } - // Check for "expr < 1". - else if (oper == GT_LT) + // Check for "expr relop -1". + else if (!cmp->IsUnsigned() && (op2Value == -1)) { - // Change to "expr == 0" for unsigned and "expr <= 0". - oper = cmp->IsUnsigned() ? GT_EQ : GT_LE; + // Check for "expr <= -1". + if (oper == GT_LE) + { + // Change to "expr < 0". + oper = GT_LT; + } + // Check for "expr > -1". + else if (oper == GT_GT) + { + // Change to "expr >= 0". + oper = GT_GE; + } } - } - // Check for "expr relop -1". - else if (!cmp->IsUnsigned() && (op2Value == -1)) - { - // Check for "expr <= -1". - if (oper == GT_LE) + else if (cmp->IsUnsigned()) { - // Change to "expr < 0". - oper = GT_LT; + if ((oper == GT_LE) || (oper == GT_GT)) + { + if (op2Value == 0) + { + // IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT + // recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails + // if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0) + // and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes + // occurs as a result of branch inversion. + oper = (oper == GT_LE) ? GT_EQ : GT_NE; + cmp->gtFlags &= ~GTF_UNSIGNED; + } + // LE_UN/GT_UN(expr, int/long.MaxValue) => GE/LT(expr, 0). + else if (((op1->TypeIs(TYP_LONG) && (op2Value == INT64_MAX))) || + ((genActualType(op1) == TYP_INT) && (op2Value == INT32_MAX))) + { + oper = (oper == GT_LE) ? GT_GE : GT_LT; + cmp->gtFlags &= ~GTF_UNSIGNED; + } + } } - // Check for "expr > -1". - else if (oper == GT_GT) + + if (!cmp->OperIs(oper)) { - // Change to "expr >= 0". - oper = GT_GE; + // Keep the old ValueNumber for 'tree' as the new expr + // will still compute the same value as before. + cmp->SetOper(oper, GenTree::PRESERVE_VN); + op2->SetIntegralValue(0); + fgUpdateConstTreeValueNumber(op2); } } - else if (cmp->IsUnsigned()) + + if (cmp->OperIs(GT_LT, GT_GT)) { - if ((oper == GT_LE) || (oper == GT_GT)) + assert(cmp->gtGetOp2()->IsIntegralConst() || cmp->gtGetOp1()->IsIntegralConst()); + assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2()) || !gtIsActiveCSE_Candidate(cmp->gtGetOp1())); + + GenTree* conNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp2() : cmp->gtGetOp1(); + GenTree* varNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp1() : cmp->gtGetOp2(); + var_types limitType = varNode->TypeGet(); + + if (varNode->OperIs(GT_CAST)) + { + limitType = varNode->AsCast()->gtCastType; + } + + switch (limitType) + { + case TYP_UBYTE: + case TYP_SHORT: + case TYP_USHORT: + case TYP_INT: + case TYP_UINT: + case TYP_LONG: + break; + default: + return cmp; + } + + IntegralRange valRange = IntegralRange::ForNode(varNode, this); + +#if defined(HOST_X86) || defined(HOST_ARM) + ssize_t valMin = (int32_t)IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(limitType)); + ssize_t valMax = (int32_t)IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(limitType)); +#else + ssize_t valMax = IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(limitType)); + ssize_t valMin = IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(limitType)); + +#endif + + bool fold = false; + // Folds + // 1. byte x <= 0 => false + // 2. short x <= short.MinValue => false + // 3. ushort x <= ushort.MinValue => false + // 4. int x <= int.MinValue => false + // 5. uint x <= uint.MinValue => false + // 6. long x <= long.MinValue => false + // 7. ulong x <= ulong.MinValue => false + // + // 8. byte x >= byte.MaxValue => false + // 9. short x <= short.MinValue => false + // 10. ushort x <= ushort.MinValue => false + // 11. int x >= int.MaxValue => false + // 12. int x >= int.MaxValue => false + // 13. long x >= long.MaxValue => false + // 14. long x >= long.MaxValue => false + // + // when const is RHS: + if (cmp->gtGetOp2()->IsIntegralConst()) { - if (op2Value == 0) + if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMax)) || + (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMin))) { - // IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT - // recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails - // if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0) - // and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes - // occurs as a result of branch inversion. - oper = (oper == GT_LE) ? GT_EQ : GT_NE; - cmp->gtFlags &= ~GTF_UNSIGNED; + fold = true; } - // LE_UN/GT_UN(expr, int/long.MaxValue) => GE/LT(expr, 0). - else if (((op1->TypeIs(TYP_LONG) && (op2Value == INT64_MAX))) || - ((genActualType(op1) == TYP_INT) && (op2Value == INT32_MAX))) + + // when const is RHS and cmp is unsigned + if (cmp->IsUnsigned()) { - oper = (oper == GT_LE) ? GT_GE : GT_LT; - cmp->gtFlags &= ~GTF_UNSIGNED; + if (cmp->OperIs(GT_GT) && conNode->IsIntegralConst(-1)) + { + fold = true; + } } } - } + // when const is LHS: + else if (cmp->gtGetOp1()->IsIntegralConst()) + { + if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMin)) || + (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMax))) + { + fold = true; + } - if (!cmp->OperIs(oper)) - { - // Keep the old ValueNumber for 'tree' as the new expr - // will still compute the same value as before. - cmp->SetOper(oper, GenTree::PRESERVE_VN); - op2->SetIntegralValue(0); - fgUpdateConstTreeValueNumber(op2); + // when const is LHS and cmp is unsigned + if (cmp->IsUnsigned()) + { + if (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(-1)) + { + fold = true; + } + } + } + + if (fold) + { + GenTree* ret = gtNewIconNode(0); + + ret->SetVNsFromNode(cmp); + + GenTree* cmpSideEffects = nullptr; + + gtExtractSideEffList(cmp, &cmpSideEffects); + + if (cmpSideEffects != nullptr) + { + ret = gtNewOperNode(GT_COMMA, TYP_INT, cmpSideEffects, ret); + } + + DEBUG_DESTROY_NODE(cmp); + + INDEBUG(ret->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + + return ret; + } } return cmp; @@ -13090,7 +13156,7 @@ GenTree* Compiler::fgOptimizeMultiply(GenTreeOp* mul) // Should we try to replace integer multiplication with lea/add/shift sequences? bool mulShiftOpt = compCodeOpt() != SMALL_CODE; #else // !TARGET_XARCH - bool mulShiftOpt = false; + bool mulShiftOpt = false; #endif // !TARGET_XARCH size_t abs_mult = (mult >= 0) ? mult : -mult; diff --git a/src/tests/JIT/opt/Compares/compares.cs b/src/tests/JIT/opt/Compares/compares.cs new file mode 100644 index 0000000000000..7d914254ee7ae --- /dev/null +++ b/src/tests/JIT/opt/Compares/compares.cs @@ -0,0 +1,204 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// unit test for the full range comparison optimization + +using System; +using System.Runtime.CompilerServices; + +public class FullRangeComparisonTest +{ + // Class for testing side effects promotion + public class SideEffects + { + public byte B; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MinValue_RHSConst_Byte(byte b) => b >= 0; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MinValue_RHSConst_Short(short s) => s >= short.MinValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MinValue_RHSConst_Int(int i) => i >= int.MinValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MinValue_RHSConst_Long(long l) => l >= long.MinValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MaxValue_RHSConst_Byte(byte b) => b <= byte.MaxValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MaxValue_RHSConst_Short(short s) => s <= short.MaxValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MaxValue_RHSConst_Int(int i) => i <= int.MaxValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MaxValue_RHSConst_Long(long l) => l <= long.MaxValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MaxValue_RHSConst_UShort(ushort us) => us <= ushort.MaxValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MaxValue_RHSConst_UInt(uint ui) => ui <= uint.MaxValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MaxValue_RHSConst_ULong(ulong ul) => ul <= uint.MaxValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MinValue_LHSConst_Byte(byte b) => 0 <= b; + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MinValue_LHSConst_Short(short i) => short.MinValue <= i; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MinValue_LHSConst_Int(int i) => int.MinValue <= i; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrLessThan_MinValue_LHSConst_Long(long l) => long.MinValue <= l; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MaxValue_LHSConst_Byte(byte b) => byte.MaxValue >= b; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MaxValue_LHSConst_Short(short s) => short.MaxValue >= s; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MaxValue_LHSConst_Int(int i) => int.MaxValue >= i; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MaxValue_LHSConst_Long(long l) => long.MaxValue >= l; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool EqualsOrGreaterThan_MaxValue_LHSConst_SideEffects(SideEffects c) => c.B <= 255; + public static int Main() + { + // Optimize comparison with full range values + // RHS Const Optimization + if (!EqualsOrGreaterThan_MinValue_RHSConst_Byte(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MinValue_RHSConst_Byte(10) failed"); + return 101; + } + + if (!EqualsOrGreaterThan_MinValue_RHSConst_Short(-10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MinValue_RHSConst_Short(10) failed"); + return 101; + } + + if (!EqualsOrGreaterThan_MinValue_RHSConst_Int(-10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MinValue_RHSConst_Int(10) failed"); + return 101; + } + + if (!EqualsOrGreaterThan_MinValue_RHSConst_Long(-10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MinValue_RHSConst_Long(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MaxValue_RHSConst_Byte(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MaxValue_RHSConst_Byte(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MaxValue_RHSConst_Short(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MaxValue_RHSConst_Short(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MaxValue_RHSConst_Int(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MaxValue_RHSConst_Int(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MaxValue_RHSConst_Long(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MaxValue_RHSConst_Long(10) failed"); + return 101; + } + + // LHS Const Optimization + if (!EqualsOrLessThan_MinValue_LHSConst_Byte(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MinValue_LHSConst_Byte(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MinValue_LHSConst_Int(-10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MinValue_LHSConst_Int(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MinValue_LHSConst_Long(-10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MinValue_LHSConst_Long(10) failed"); + return 101; + } + + if (!EqualsOrGreaterThan_MaxValue_LHSConst_Byte(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MaxValue_LHSConst_Byte(10) failed"); + return 101; + } + + if (!EqualsOrGreaterThan_MaxValue_LHSConst_Int(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MaxValue_LHSConst_Int(10) failed"); + return 101; + } + + if (!EqualsOrGreaterThan_MaxValue_LHSConst_Long(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MaxValue_LHSConst_Long(10) failed"); + return 101; + } + + + // Unsigned values + if (!EqualsOrLessThan_MaxValue_RHSConst_UShort(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MaxValue_RHSConst_UShort(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MaxValue_RHSConst_UInt(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MaxValue_RHSConst_UInt(10) failed"); + return 101; + } + + if (!EqualsOrLessThan_MaxValue_RHSConst_ULong(10)) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrLessThan_MaxValue_RHSConst_ULong(10) failed"); + return 101; + } + + // Side effects persist + try + { + EqualsOrGreaterThan_MaxValue_LHSConst_SideEffects(null); + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MaxValue_LHSConst_SideEffects(null) failed"); + return 101; + } + catch (NullReferenceException ex) + { + + } + catch (Exception ex) + { + Console.WriteLine("FullRangeComparisonTest:EqualsOrGreaterThan_MaxValue_LHSConst_SideEffects(null) failed"); + return 101; + } + + Console.WriteLine("PASSED"); + return 100; + } +} diff --git a/src/tests/JIT/opt/Compares/compares.csproj b/src/tests/JIT/opt/Compares/compares.csproj new file mode 100644 index 0000000000000..5e5fbae5cb863 --- /dev/null +++ b/src/tests/JIT/opt/Compares/compares.csproj @@ -0,0 +1,12 @@ + + + Exe + + + PdbOnly + True + + + + + From 4d7aaecc889b56106a45a5b8567f5869abe8a755 Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Fri, 10 Jun 2022 15:21:54 +0300 Subject: [PATCH 06/13] Moved logic to separate functions (#70145) --- src/coreclr/jit/assertionprop.cpp | 44 ++++ src/coreclr/jit/compiler.h | 3 + src/coreclr/jit/morph.cpp | 365 +++++++++++++++--------------- 3 files changed, 232 insertions(+), 180 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index b80c42875f3bb..5d3d937299542 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -68,6 +68,50 @@ bool IntegralRange::Contains(int64_t value) const return SymbolicToRealMap[static_cast(value)]; } +//------------------------------------------------------------------------ +// GetUpperBound: Get the largest possible value "type" can represent +// +// Arguments: +// node - the node, of an integral type, in question +// type - the integral type in question +// compiler - the Compiler, used to retrieve additional info +// +// Return Value: +// the largest possible value "type" can represent. +// +/* static */ size_t IntegralRange::GetUpperBound(GenTree* node, var_types type, Compiler* compiler) +{ + IntegralRange valRange = IntegralRange::ForNode(node, compiler); +#if defined(HOST_X86) || defined(HOST_ARM) + return (int32_t)IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(type)); +#else + return IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(type)); +#endif + +} + +//------------------------------------------------------------------------ +// GetUpperBound: Get the smallest possible value "type" can represent +// +// Arguments: +// node - the node, of an integral type, in question +// type - the integral type in question +// compiler - the Compiler, used to retrieve additional info +// +// Return Value: +// the smallest possible value "type" can represent. +// +/* static */ size_t IntegralRange::GetLowerBound(GenTree* node, var_types type, Compiler* compiler) +{ + IntegralRange valRange = IntegralRange::ForNode(node, compiler); +#if defined(HOST_X86) || defined(HOST_ARM) + return (int32_t)IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(type)); +#else + return IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(type)); + +#endif +} + //------------------------------------------------------------------------ // LowerBoundForType: Get the symbolic lower bound for a type. // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index ed2f3f55ce905..c13e465333348 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1260,6 +1260,8 @@ class IntegralRange } static int64_t SymbolicToRealValue(SymbolicIntegerValue value); + static size_t GetUpperBound(GenTree* node, var_types type, Compiler* compiler); + static size_t GetLowerBound(GenTree* node, var_types type, Compiler* compiler); static SymbolicIntegerValue LowerBoundForType(var_types type); static SymbolicIntegerValue UpperBoundForType(var_types type); @@ -5690,6 +5692,7 @@ class Compiler GenTree* fgOptimizeCast(GenTreeCast* cast); GenTree* fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp); GenTree* fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp); + GenTree* fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* cmp); #ifdef FEATURE_HW_INTRINSICS GenTree* fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node); #endif diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 846040407c652..26c4c589d27a8 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11375,7 +11375,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) if (!optValnumCSE_phase && op2->IsIntegralConst()) { tree = fgOptimizeEqualityComparisonWithConst(tree->AsOp()); - assert(tree->OperIsCompare()); oper = tree->OperGet(); @@ -11398,23 +11397,30 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } // op2's value may be changed, so it cannot be a CSE candidate. - // op1's value may be changed, so it cannot be a CSE candidate. - if ((op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2)) || - (op1->IsIntegralConst() && !gtIsActiveCSE_Candidate(op1))) + if (op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2)) { tree = fgOptimizeRelationalComparisonWithConst(tree->AsOp()); - - if (tree->OperIs(GT_CNS_INT, GT_COMMA)) - { - return tree; - } - oper = tree->OperGet(); assert(op1 == tree->AsOp()->gtGetOp1()); assert(op2 == tree->AsOp()->gtGetOp2()); } + if (opts.OptimizationEnabled() && fgGlobalMorph) + { + if (op2->IsIntegralConst() || op1->IsIntegralConst()) + { + if (tree->OperIs(GT_GT, GT_LT)) + { + tree = fgOptimizeRelationalComparisonWithFullRangeConst(tree->AsOp()); + if (tree->OperIs(GT_CNS_INT, GT_COMMA)) + { + return tree; + } + } + } + } + COMPARE: noway_assert(tree->OperIsCompare()); @@ -12487,7 +12493,6 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) op1->SetVNsFromNode(cmp); DEBUG_DESTROY_NODE(cmp); - return op1; } @@ -12642,219 +12647,219 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) } //------------------------------------------------------------------------ -// fgOptimizeRelationalComparisonWithConst: optimizes a comparison operation. +// fgOptimizeRelationalComparisonWithFullRangeConst: optimizes a comparison operation. // -// Recognizes comparisons against various constant operands and morphs -// them, if possible, into comparisons against zero. +// Recognizes "Always false" comparisons against various full range constant operands and morphs +// them into zero. // // Arguments: -// cmp - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. +// cmp - the GT_LT/GT_GT tree to morph. // // Return Value: -// 1. The "cmp" tree, possibly with a modified oper. -// The second operand's constant value may be modified as well. -// 2. GT_CNS_INT node containing zero as value -// +// 1. The unmodified "cmp" tree. +// 2. A CNS_INT node containing zero. +// 3. A GT_COMMA node containing side effects along with a CNS_INT node containing zero // Assumptions: -// The operands have been swapped so that any constants are on the right. -// The second operand is an integral constant. +// The second operand is an integral constant or the first operand is an integral constant. // -GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) +GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* cmp) { - assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); - - if (cmp->gtGetOp2()->IsIntegralConst() && !gtIsActiveCSE_Candidate(cmp->gtGetOp2())) - { - assert(cmp->gtGetOp2()->IsIntegralConst()); - assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2())); + assert(cmp->OperIs(GT_LT, GT_GT)); + assert(cmp->gtGetOp2()->IsIntegralConst() || cmp->gtGetOp1()->IsIntegralConst()); - GenTree* op1 = cmp->gtGetOp1(); - GenTreeIntConCommon* op2 = cmp->gtGetOp2()->AsIntConCommon(); + GenTree* conNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp2() : cmp->gtGetOp1(); + GenTree* varNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp1() : cmp->gtGetOp2(); + var_types limitType = varNode->TypeGet(); - assert(genActualType(op1) == genActualType(op2)); + if (varNode->OperIs(GT_CAST)) + { + limitType = varNode->CastToType(); + switch (limitType) + { + case TYP_UBYTE: + case TYP_SHORT: + case TYP_USHORT: + default: + return cmp; + } + } - genTreeOps oper = cmp->OperGet(); - int64_t op2Value = op2->IntegralValue(); + ssize_t valMin = IntegralRange::GetLowerBound(varNode, limitType, this); + ssize_t valMax = IntegralRange::GetUpperBound(varNode, limitType, this); - if (op2Value == 1) + bool fold = false; + // Folds + // 1. byte x <= 0 => false + // 2. short x <= short.MinValue => false + // 3. ushort x <= ushort.MinValue => false + // 4. int x <= int.MinValue => false + // 5. uint x <= uint.MinValue => false + // 6. long x <= long.MinValue => false + // 7. ulong x <= ulong.MinValue => false + // + // 8. byte x >= byte.MaxValue => false + // 9. short x <= short.MinValue => false + // 10. ushort x <= ushort.MinValue => false + // 11. int x >= int.MaxValue => false + // 12. int x >= int.MaxValue => false + // 13. long x >= long.MaxValue => false + // 14. long x >= long.MaxValue => false + // + // when const is RHS: + if (cmp->gtGetOp2()->IsIntegralConst()) + { + if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMax)) || + (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMin))) { - // Check for "expr >= 1". - if (oper == GT_GE) - { - // Change to "expr != 0" for unsigned and "expr > 0" for signed. - oper = cmp->IsUnsigned() ? GT_NE : GT_GT; - } - // Check for "expr < 1". - else if (oper == GT_LT) - { - // Change to "expr == 0" for unsigned and "expr <= 0". - oper = cmp->IsUnsigned() ? GT_EQ : GT_LE; - } + fold = true; } - // Check for "expr relop -1". - else if (!cmp->IsUnsigned() && (op2Value == -1)) + + // when const is RHS and cmp is unsigned + if (cmp->IsUnsigned()) { - // Check for "expr <= -1". - if (oper == GT_LE) + if (cmp->OperIs(GT_GT) && conNode->IsIntegralConst(-1)) { - // Change to "expr < 0". - oper = GT_LT; - } - // Check for "expr > -1". - else if (oper == GT_GT) - { - // Change to "expr >= 0". - oper = GT_GE; + fold = true; } } - else if (cmp->IsUnsigned()) + } + // when const is LHS: + else if (cmp->gtGetOp1()->IsIntegralConst()) + { + if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMin)) || + (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMax))) { - if ((oper == GT_LE) || (oper == GT_GT)) - { - if (op2Value == 0) - { - // IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT - // recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails - // if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0) - // and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes - // occurs as a result of branch inversion. - oper = (oper == GT_LE) ? GT_EQ : GT_NE; - cmp->gtFlags &= ~GTF_UNSIGNED; - } - // LE_UN/GT_UN(expr, int/long.MaxValue) => GE/LT(expr, 0). - else if (((op1->TypeIs(TYP_LONG) && (op2Value == INT64_MAX))) || - ((genActualType(op1) == TYP_INT) && (op2Value == INT32_MAX))) - { - oper = (oper == GT_LE) ? GT_GE : GT_LT; - cmp->gtFlags &= ~GTF_UNSIGNED; - } - } + fold = true; } - if (!cmp->OperIs(oper)) + // when const is LHS and cmp is unsigned + if (cmp->IsUnsigned()) { - // Keep the old ValueNumber for 'tree' as the new expr - // will still compute the same value as before. - cmp->SetOper(oper, GenTree::PRESERVE_VN); - op2->SetIntegralValue(0); - fgUpdateConstTreeValueNumber(op2); + if (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(-1)) + { + fold = true; + } } } - if (cmp->OperIs(GT_LT, GT_GT)) + if (fold) { - assert(cmp->gtGetOp2()->IsIntegralConst() || cmp->gtGetOp1()->IsIntegralConst()); - assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2()) || !gtIsActiveCSE_Candidate(cmp->gtGetOp1())); + GenTree* ret = gtNewZeroConNode(TYP_INT); - GenTree* conNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp2() : cmp->gtGetOp1(); - GenTree* varNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp1() : cmp->gtGetOp2(); - var_types limitType = varNode->TypeGet(); + ret->SetVNsFromNode(cmp); - if (varNode->OperIs(GT_CAST)) - { - limitType = varNode->AsCast()->gtCastType; - } + GenTree* cmpSideEffects = nullptr; - switch (limitType) + gtExtractSideEffList(cmp, &cmpSideEffects); + + if (cmpSideEffects != nullptr) { - case TYP_UBYTE: - case TYP_SHORT: - case TYP_USHORT: - case TYP_INT: - case TYP_UINT: - case TYP_LONG: - break; - default: - return cmp; + ret = gtNewOperNode(GT_COMMA, TYP_INT, cmpSideEffects, ret); } - IntegralRange valRange = IntegralRange::ForNode(varNode, this); + DEBUG_DESTROY_NODE(cmp); -#if defined(HOST_X86) || defined(HOST_ARM) - ssize_t valMin = (int32_t)IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(limitType)); - ssize_t valMax = (int32_t)IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(limitType)); -#else - ssize_t valMax = IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(limitType)); - ssize_t valMin = IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(limitType)); + INDEBUG(ret->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); -#endif + return ret; + } - bool fold = false; - // Folds - // 1. byte x <= 0 => false - // 2. short x <= short.MinValue => false - // 3. ushort x <= ushort.MinValue => false - // 4. int x <= int.MinValue => false - // 5. uint x <= uint.MinValue => false - // 6. long x <= long.MinValue => false - // 7. ulong x <= ulong.MinValue => false - // - // 8. byte x >= byte.MaxValue => false - // 9. short x <= short.MinValue => false - // 10. ushort x <= ushort.MinValue => false - // 11. int x >= int.MaxValue => false - // 12. int x >= int.MaxValue => false - // 13. long x >= long.MaxValue => false - // 14. long x >= long.MaxValue => false - // - // when const is RHS: - if (cmp->gtGetOp2()->IsIntegralConst()) - { - if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMax)) || - (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMin))) - { - fold = true; - } + return cmp; +} - // when const is RHS and cmp is unsigned - if (cmp->IsUnsigned()) - { - if (cmp->OperIs(GT_GT) && conNode->IsIntegralConst(-1)) - { - fold = true; - } - } +//------------------------------------------------------------------------ +// fgOptimizeRelationalComparisonWithConst: optimizes a comparison operation. +// +// Recognizes comparisons against various constant operands and morphs +// them, if possible, into comparisons against zero. +// +// Arguments: +// cmp - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. +// +// Return Value: +// The "cmp" tree, possibly with a modified oper. +// The second operand's constant value may be modified as well. +// +// Assumptions: +// The operands have been swapped so that any constants are on the right. +// The second operand is an integral constant. +// +GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) +{ + assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); + assert(cmp->gtGetOp2()->IsIntegralConst()); + assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2())); + + GenTree* op1 = cmp->gtGetOp1(); + GenTreeIntConCommon* op2 = cmp->gtGetOp2()->AsIntConCommon(); + + assert(genActualType(op1) == genActualType(op2)); + + genTreeOps oper = cmp->OperGet(); + int64_t op2Value = op2->IntegralValue(); + + if (op2Value == 1) + { + // Check for "expr >= 1". + if (oper == GT_GE) + { + // Change to "expr != 0" for unsigned and "expr > 0" for signed. + oper = cmp->IsUnsigned() ? GT_NE : GT_GT; } - // when const is LHS: - else if (cmp->gtGetOp1()->IsIntegralConst()) + // Check for "expr < 1". + else if (oper == GT_LT) { - if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMin)) || - (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMax))) + // Change to "expr == 0" for unsigned and "expr <= 0". + oper = cmp->IsUnsigned() ? GT_EQ : GT_LE; + } + } + // Check for "expr relop -1". + else if (!cmp->IsUnsigned() && (op2Value == -1)) + { + // Check for "expr <= -1". + if (oper == GT_LE) + { + // Change to "expr < 0". + oper = GT_LT; + } + // Check for "expr > -1". + else if (oper == GT_GT) + { + // Change to "expr >= 0". + oper = GT_GE; + } + } + else if (cmp->IsUnsigned()) + { + if ((oper == GT_LE) || (oper == GT_GT)) + { + if (op2Value == 0) { - fold = true; + // IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT + // recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails + // if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0) + // and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes + // occurs as a result of branch inversion. + oper = (oper == GT_LE) ? GT_EQ : GT_NE; + cmp->gtFlags &= ~GTF_UNSIGNED; } - - // when const is LHS and cmp is unsigned - if (cmp->IsUnsigned()) + // LE_UN/GT_UN(expr, int/long.MaxValue) => GE/LT(expr, 0). + else if (((op1->TypeIs(TYP_LONG) && (op2Value == INT64_MAX))) || + ((genActualType(op1) == TYP_INT) && (op2Value == INT32_MAX))) { - if (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(-1)) - { - fold = true; - } + oper = (oper == GT_LE) ? GT_GE : GT_LT; + cmp->gtFlags &= ~GTF_UNSIGNED; } } + } - if (fold) - { - GenTree* ret = gtNewIconNode(0); - - ret->SetVNsFromNode(cmp); - - GenTree* cmpSideEffects = nullptr; - - gtExtractSideEffList(cmp, &cmpSideEffects); - - if (cmpSideEffects != nullptr) - { - ret = gtNewOperNode(GT_COMMA, TYP_INT, cmpSideEffects, ret); - } - - DEBUG_DESTROY_NODE(cmp); - - INDEBUG(ret->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - - return ret; - } + if (!cmp->OperIs(oper)) + { + // Keep the old ValueNumber for 'tree' as the new expr + // will still compute the same value as before. + cmp->SetOper(oper, GenTree::PRESERVE_VN); + op2->SetIntegralValue(0); + fgUpdateConstTreeValueNumber(op2); } return cmp; From 2da383cc6f85d95b53b7040cddb81f1eb0b4d56b Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Fri, 10 Jun 2022 16:03:06 +0300 Subject: [PATCH 07/13] Simplified determining Lower/Upper bounds (#70145) --- src/coreclr/jit/assertionprop.cpp | 44 ------------------------------- src/coreclr/jit/compiler.h | 12 +++++++-- src/coreclr/jit/morph.cpp | 29 ++++++++------------ 3 files changed, 21 insertions(+), 64 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 5d3d937299542..b80c42875f3bb 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -68,50 +68,6 @@ bool IntegralRange::Contains(int64_t value) const return SymbolicToRealMap[static_cast(value)]; } -//------------------------------------------------------------------------ -// GetUpperBound: Get the largest possible value "type" can represent -// -// Arguments: -// node - the node, of an integral type, in question -// type - the integral type in question -// compiler - the Compiler, used to retrieve additional info -// -// Return Value: -// the largest possible value "type" can represent. -// -/* static */ size_t IntegralRange::GetUpperBound(GenTree* node, var_types type, Compiler* compiler) -{ - IntegralRange valRange = IntegralRange::ForNode(node, compiler); -#if defined(HOST_X86) || defined(HOST_ARM) - return (int32_t)IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(type)); -#else - return IntegralRange::SymbolicToRealValue(valRange.UpperBoundForType(type)); -#endif - -} - -//------------------------------------------------------------------------ -// GetUpperBound: Get the smallest possible value "type" can represent -// -// Arguments: -// node - the node, of an integral type, in question -// type - the integral type in question -// compiler - the Compiler, used to retrieve additional info -// -// Return Value: -// the smallest possible value "type" can represent. -// -/* static */ size_t IntegralRange::GetLowerBound(GenTree* node, var_types type, Compiler* compiler) -{ - IntegralRange valRange = IntegralRange::ForNode(node, compiler); -#if defined(HOST_X86) || defined(HOST_ARM) - return (int32_t)IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(type)); -#else - return IntegralRange::SymbolicToRealValue(valRange.LowerBoundForType(type)); - -#endif -} - //------------------------------------------------------------------------ // LowerBoundForType: Get the symbolic lower bound for a type. // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index c13e465333348..285b5591f2880 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1242,6 +1242,16 @@ class IntegralRange assert(lowerBound <= upperBound); } + SymbolicIntegerValue GetLowerBound() + { + return m_lowerBound; + } + + SymbolicIntegerValue GetUpperBound() + { + return m_upperBound; + } + bool Contains(int64_t value) const; bool Contains(IntegralRange other) const @@ -1260,8 +1270,6 @@ class IntegralRange } static int64_t SymbolicToRealValue(SymbolicIntegerValue value); - static size_t GetUpperBound(GenTree* node, var_types type, Compiler* compiler); - static size_t GetLowerBound(GenTree* node, var_types type, Compiler* compiler); static SymbolicIntegerValue LowerBoundForType(var_types type); static SymbolicIntegerValue UpperBoundForType(var_types type); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 26c4c589d27a8..1beaaf0043634 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12667,25 +12667,18 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* c assert(cmp->OperIs(GT_LT, GT_GT)); assert(cmp->gtGetOp2()->IsIntegralConst() || cmp->gtGetOp1()->IsIntegralConst()); - GenTree* conNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp2() : cmp->gtGetOp1(); - GenTree* varNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp1() : cmp->gtGetOp2(); - var_types limitType = varNode->TypeGet(); + GenTree* conNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp2() : cmp->gtGetOp1(); + GenTree* varNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp1() : cmp->gtGetOp2(); - if (varNode->OperIs(GT_CAST)) - { - limitType = varNode->CastToType(); - switch (limitType) - { - case TYP_UBYTE: - case TYP_SHORT: - case TYP_USHORT: - default: - return cmp; - } - } + IntegralRange valRange = IntegralRange::ForNode(varNode, this); - ssize_t valMin = IntegralRange::GetLowerBound(varNode, limitType, this); - ssize_t valMax = IntegralRange::GetUpperBound(varNode, limitType, this); +#if defined(HOST_X86) || defined(HOST_ARM) + ssize_t valMin = (int32_t)IntegralRange::SymbolicToRealValue(valRange.GetLowerBound()); + ssize_t valMax = (int32_t)IntegralRange::SymbolicToRealValue(valRange.GetUpperBound()); +#else + ssize_t valMin = IntegralRange::SymbolicToRealValue(valRange.GetLowerBound()); + ssize_t valMax = IntegralRange::SymbolicToRealValue(valRange.GetUpperBound()); +#endif bool fold = false; // Folds @@ -13161,7 +13154,7 @@ GenTree* Compiler::fgOptimizeMultiply(GenTreeOp* mul) // Should we try to replace integer multiplication with lea/add/shift sequences? bool mulShiftOpt = compCodeOpt() != SMALL_CODE; #else // !TARGET_XARCH - bool mulShiftOpt = false; + bool mulShiftOpt = false; #endif // !TARGET_XARCH size_t abs_mult = (mult >= 0) ? mult : -mult; From 01d7d4869801de4d20da30a8d6c3f85070d6116c Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Sat, 11 Jun 2022 21:41:27 +0300 Subject: [PATCH 08/13] Generalized both sides to intervals (#70145) --- src/coreclr/jit/morph.cpp | 112 ++++++++++++++------------------------ 1 file changed, 41 insertions(+), 71 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1beaaf0043634..844ab1bad610c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11410,7 +11410,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) { if (op2->IsIntegralConst() || op1->IsIntegralConst()) { - if (tree->OperIs(GT_GT, GT_LT)) + if (tree->OperIs(GT_GT, GT_LT, GT_LE, GT_GE)) { tree = fgOptimizeRelationalComparisonWithFullRangeConst(tree->AsOp()); if (tree->OperIs(GT_CNS_INT, GT_COMMA)) @@ -12664,92 +12664,62 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) // GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* cmp) { - assert(cmp->OperIs(GT_LT, GT_GT)); - assert(cmp->gtGetOp2()->IsIntegralConst() || cmp->gtGetOp1()->IsIntegralConst()); - GenTree* conNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp2() : cmp->gtGetOp1(); - GenTree* varNode = cmp->gtGetOp2()->IsIntegralConst() ? cmp->gtGetOp1() : cmp->gtGetOp2(); - - IntegralRange valRange = IntegralRange::ForNode(varNode, this); - -#if defined(HOST_X86) || defined(HOST_ARM) - ssize_t valMin = (int32_t)IntegralRange::SymbolicToRealValue(valRange.GetLowerBound()); - ssize_t valMax = (int32_t)IntegralRange::SymbolicToRealValue(valRange.GetUpperBound()); -#else - ssize_t valMin = IntegralRange::SymbolicToRealValue(valRange.GetLowerBound()); - ssize_t valMax = IntegralRange::SymbolicToRealValue(valRange.GetUpperBound()); -#endif + int64_t lhsMin; + int64_t lhsMax; + if (cmp->gtGetOp1()->IsIntegralConst()) + { + lhsMin = cmp->gtGetOp1()->AsIntConCommon()->IntegralValue(); + lhsMax = lhsMin; + } + else + { + IntegralRange lhsRange = IntegralRange::ForNode(cmp->gtGetOp1(), this); + lhsMin = IntegralRange::SymbolicToRealValue(lhsRange.GetLowerBound()); + lhsMax = IntegralRange::SymbolicToRealValue(lhsRange.GetUpperBound()); + } - bool fold = false; - // Folds - // 1. byte x <= 0 => false - // 2. short x <= short.MinValue => false - // 3. ushort x <= ushort.MinValue => false - // 4. int x <= int.MinValue => false - // 5. uint x <= uint.MinValue => false - // 6. long x <= long.MinValue => false - // 7. ulong x <= ulong.MinValue => false - // - // 8. byte x >= byte.MaxValue => false - // 9. short x <= short.MinValue => false - // 10. ushort x <= ushort.MinValue => false - // 11. int x >= int.MaxValue => false - // 12. int x >= int.MaxValue => false - // 13. long x >= long.MaxValue => false - // 14. long x >= long.MaxValue => false - // - // when const is RHS: + int64_t rhsMin; + int64_t rhsMax; if (cmp->gtGetOp2()->IsIntegralConst()) { - if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMax)) || - (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMin))) - { - fold = true; - } - - // when const is RHS and cmp is unsigned - if (cmp->IsUnsigned()) - { - if (cmp->OperIs(GT_GT) && conNode->IsIntegralConst(-1)) - { - fold = true; - } - } + rhsMin = cmp->gtGetOp2()->AsIntConCommon()->IntegralValue(); + rhsMax = rhsMin; } - // when const is LHS: - else if (cmp->gtGetOp1()->IsIntegralConst()) + else { - if ((cmp->OperIs(GT_GT) && conNode->IsIntegralConst(valMin)) || - (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(valMax))) - { - fold = true; - } - - // when const is LHS and cmp is unsigned - if (cmp->IsUnsigned()) - { - if (cmp->OperIs(GT_LT) && conNode->IsIntegralConst(-1)) - { - fold = true; - } - } + IntegralRange rhsRange = IntegralRange::ForNode(cmp->gtGetOp2(), this); + rhsMin = IntegralRange::SymbolicToRealValue(rhsRange.GetLowerBound()); + rhsMax = IntegralRange::SymbolicToRealValue(rhsRange.GetUpperBound()); } - if (fold) + genTreeOps op = cmp->gtOper; + if ((op != GT_LT) && (op != GT_LE)) { - GenTree* ret = gtNewZeroConNode(TYP_INT); + op = GenTree::SwapRelop(op); + std::swap(lhsMin, rhsMin); + std::swap(lhsMax, rhsMax); + } - ret->SetVNsFromNode(cmp); + int64_t lefOpValue = lhsMin; + int64_t rightOpValue = rhsMax; - GenTree* cmpSideEffects = nullptr; + if (cmp->IsUnsigned() && lefOpValue == -1 && lhsMax == -1) + { + rightOpValue = -1; + } - gtExtractSideEffList(cmp, &cmpSideEffects); + if ((op == GT_LT && lefOpValue == rightOpValue) || (op == GT_LE && lefOpValue > rightOpValue)) + { + GenTree* ret = gtNewZeroConNode(TYP_INT); - if (cmpSideEffects != nullptr) + if (gtTreeHasSideEffects(cmp, GTF_SIDE_EFFECT)) { - ret = gtNewOperNode(GT_COMMA, TYP_INT, cmpSideEffects, ret); + return cmp; } + ret->SetVNsFromNode(cmp); + DEBUG_DESTROY_NODE(cmp); INDEBUG(ret->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); From 8acf1cc32e21156aca38cccecea7ba5bdab0407a Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Tue, 14 Jun 2022 16:53:37 +0300 Subject: [PATCH 09/13] Refactoring of conditions (#70145) --- src/coreclr/jit/morph.cpp | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 844ab1bad610c..1eaa565ac1ce2 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12664,6 +12664,10 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) // GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* cmp) { + if (gtTreeHasSideEffects(cmp, GTF_SIDE_EFFECT)) + { + return cmp; + } int64_t lhsMin; int64_t lhsMax; @@ -12701,23 +12705,23 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* c std::swap(lhsMax, rhsMax); } - int64_t lefOpValue = lhsMin; - int64_t rightOpValue = rhsMax; - - if (cmp->IsUnsigned() && lefOpValue == -1 && lhsMax == -1) + GenTree* ret = nullptr; + bool isLhsConst = lhsMin == lhsMax; + // [x0, x1] < [y0, y1] is false if x0 >= y1 + // [x0, x1] <= [y0, y1] is false if x0 > y1 + if (((op == GT_LT) && (lhsMin >= rhsMax)) || (((op == GT_LE) && (lhsMin > rhsMax))) || + (cmp->IsUnsigned() && (op == GT_LT) && ((rhsMin < 0 && !isLhsConst) || (lhsMin < 0 && isLhsConst)))) { - rightOpValue = -1; + ret = gtNewZeroConNode(TYP_INT); } - - if ((op == GT_LT && lefOpValue == rightOpValue) || (op == GT_LE && lefOpValue > rightOpValue)) + // [x0, x1] < [y0, y1] is true if x1 < y0 + else if ((op == GT_LT) && lhsMax < rhsMin) { - GenTree* ret = gtNewZeroConNode(TYP_INT); - - if (gtTreeHasSideEffects(cmp, GTF_SIDE_EFFECT)) - { - return cmp; - } + ret = gtNewOneConNode(TYP_INT); + } + if (ret != nullptr) + { ret->SetVNsFromNode(cmp); DEBUG_DESTROY_NODE(cmp); From 9a851854dc8b5cb21daba00cec0e01a0be8f9cbb Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Tue, 14 Jun 2022 18:36:58 +0300 Subject: [PATCH 10/13] Cut out unsigned types (#70145) Cut out the unsigned types to make sure the rest works fine --- src/coreclr/jit/morph.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1eaa565ac1ce2..c7e6ea9fabc84 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12706,11 +12706,10 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* c } GenTree* ret = nullptr; - bool isLhsConst = lhsMin == lhsMax; + //bool isLhsConst = lhsMin == lhsMax; // [x0, x1] < [y0, y1] is false if x0 >= y1 // [x0, x1] <= [y0, y1] is false if x0 > y1 - if (((op == GT_LT) && (lhsMin >= rhsMax)) || (((op == GT_LE) && (lhsMin > rhsMax))) || - (cmp->IsUnsigned() && (op == GT_LT) && ((rhsMin < 0 && !isLhsConst) || (lhsMin < 0 && isLhsConst)))) + if (((op == GT_LT) && (lhsMin >= rhsMax)) || (((op == GT_LE) && (lhsMin > rhsMax)))) { ret = gtNewZeroConNode(TYP_INT); } From d11970c9261c4e45da9f38a5012b5538de3f625f Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Thu, 16 Jun 2022 13:41:42 +0300 Subject: [PATCH 11/13] Refactoring according to the comments (#70145) --- src/coreclr/jit/morph.cpp | 50 +++++++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index c7e6ea9fabc84..fa838a4cfc003 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11413,7 +11413,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) if (tree->OperIs(GT_GT, GT_LT, GT_LE, GT_GE)) { tree = fgOptimizeRelationalComparisonWithFullRangeConst(tree->AsOp()); - if (tree->OperIs(GT_CNS_INT, GT_COMMA)) + if (tree->OperIs(GT_CNS_INT)) { return tree; } @@ -12705,18 +12705,48 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* c std::swap(lhsMax, rhsMax); } - GenTree* ret = nullptr; - //bool isLhsConst = lhsMin == lhsMax; - // [x0, x1] < [y0, y1] is false if x0 >= y1 - // [x0, x1] <= [y0, y1] is false if x0 > y1 - if (((op == GT_LT) && (lhsMin >= rhsMax)) || (((op == GT_LE) && (lhsMin > rhsMax)))) + GenTree* ret = nullptr; + + if (cmp->IsUnsigned()) { - ret = gtNewZeroConNode(TYP_INT); + if ((lhsMin < 0) && (lhsMax >= 0)) + { + // [0, (uint64_t)lhsMax] U [(uint64_t)lhsMin, MaxValue] + lhsMin = 0; + lhsMax = -1; + } + + if ((rhsMin < 0) && (rhsMax >= 0)) + { + // [0, (uint64_t)rhsMax] U [(uint64_t)rhsMin, MaxValue] + rhsMin = 0; + rhsMax = -1; + } + + if ((op == GT_LT && ((uint64_t)lhsMax < (uint64_t)rhsMin) || + (op == GT_LE && ((uint64_t)lhsMax <= (uint64_t)rhsMin)))) + { + ret = gtNewOneConNode(TYP_INT); + } + else if ((op == GT_LT && ((uint64_t)lhsMin >= (uint64_t)rhsMax) || + (op == GT_LE && ((uint64_t)lhsMin > (uint64_t)rhsMax)))) + { + ret = gtNewZeroConNode(TYP_INT); + } } - // [x0, x1] < [y0, y1] is true if x1 < y0 - else if ((op == GT_LT) && lhsMax < rhsMin) + else { - ret = gtNewOneConNode(TYP_INT); + // [x0, x1] < [y0, y1] is false if x0 >= y1 + // [x0, x1] <= [y0, y1] is false if x0 > y1 + if (((op == GT_LT) && (lhsMin >= rhsMax)) || (((op == GT_LE) && (lhsMin > rhsMax)))) + { + ret = gtNewZeroConNode(TYP_INT); + } + // [x0, x1] < [y0, y1] is true if x1 < y0 + else if ((op == GT_LT) && lhsMax < rhsMin) + { + ret = gtNewOneConNode(TYP_INT); + } } if (ret != nullptr) From 7c2cbd5f51ab31b6e2d680f676a4ae328c37281d Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Thu, 16 Jun 2022 14:09:42 +0300 Subject: [PATCH 12/13] Fix parentheses (#70145) --- src/coreclr/jit/morph.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index fa838a4cfc003..dfeab1749d8d2 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12723,13 +12723,13 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* c rhsMax = -1; } - if ((op == GT_LT && ((uint64_t)lhsMax < (uint64_t)rhsMin) || - (op == GT_LE && ((uint64_t)lhsMax <= (uint64_t)rhsMin)))) + if (((op == GT_LT) && ((uint64_t)lhsMax < (uint64_t)rhsMin)) || + ((op == GT_LE) && ((uint64_t)lhsMax <= (uint64_t)rhsMin))) { ret = gtNewOneConNode(TYP_INT); } - else if ((op == GT_LT && ((uint64_t)lhsMin >= (uint64_t)rhsMax) || - (op == GT_LE && ((uint64_t)lhsMin > (uint64_t)rhsMax)))) + else if (((op == GT_LT) && ((uint64_t)lhsMin >= (uint64_t)rhsMax)) || + ((op == GT_LE) && ((uint64_t)lhsMin > (uint64_t)rhsMax))) { ret = gtNewZeroConNode(TYP_INT); } @@ -12743,7 +12743,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* c ret = gtNewZeroConNode(TYP_INT); } // [x0, x1] < [y0, y1] is true if x1 < y0 - else if ((op == GT_LT) && lhsMax < rhsMin) + else if ((op == GT_LT) && (lhsMax < rhsMin)) { ret = gtNewOneConNode(TYP_INT); } From 4faafae567b2dc121af96c9f758ffc6c18cd11aa Mon Sep 17 00:00:00 2001 From: Vadim Sychev Date: Fri, 17 Jun 2022 09:59:45 +0300 Subject: [PATCH 13/13] Fix summary (#70145) --- src/coreclr/jit/morph.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index dfeab1749d8d2..addca6c8b6c8c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12649,8 +12649,8 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) //------------------------------------------------------------------------ // fgOptimizeRelationalComparisonWithFullRangeConst: optimizes a comparison operation. // -// Recognizes "Always false" comparisons against various full range constant operands and morphs -// them into zero. +// Recognizes "Always false"/"Always true" comparisons against various full range constant operands and morphs +// them into zero/one. // // Arguments: // cmp - the GT_LT/GT_GT tree to morph. @@ -12658,7 +12658,7 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp) // Return Value: // 1. The unmodified "cmp" tree. // 2. A CNS_INT node containing zero. -// 3. A GT_COMMA node containing side effects along with a CNS_INT node containing zero +// 3. A CNS_INT node containing one. // Assumptions: // The second operand is an integral constant or the first operand is an integral constant. // @@ -12751,7 +12751,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* c if (ret != nullptr) { - ret->SetVNsFromNode(cmp); + fgUpdateConstTreeValueNumber(ret); DEBUG_DESTROY_NODE(cmp);