From 43dad0998a4e8af57a07995707f9aa2e2419603e Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sat, 11 May 2024 01:23:40 +0200 Subject: [PATCH 01/15] Chained comparison of two integers against a constant is not coalesced --- src/coreclr/jit/optimizebools.cpp | 236 ++++++++++++++---- .../JIT/opt/OptimizeBools/optboolsreturn.cs | 102 ++++++++ 2 files changed, 293 insertions(+), 45 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index f915cf7c349481..aa252f349b5459 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -96,6 +96,7 @@ class OptBoolsDsc bool optOptimizeCompareChainCondBlock(); bool optOptimizeRangeTests(); bool optOptimizeBoolsReturnBlock(BasicBlock* b3); + bool optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3); #ifdef DEBUG void optOptimizeBoolsGcStress(); #endif @@ -106,6 +107,7 @@ class OptBoolsDsc bool optOptimizeBoolsChkTypeCostCond(); void optOptimizeBoolsUpdateTrees(); bool FindCompareChain(GenTree* condition, bool* isTestCondition); + void optOptimizeBoolsKeepFirstBlockAndFreeTheRest(GenTree* cmpOp, bool optReturnBlock); }; //----------------------------------------------------------------------------- @@ -1235,45 +1237,15 @@ bool OptBoolsDsc::optOptimizeBoolsChkTypeCostCond() } //----------------------------------------------------------------------------- -// optOptimizeBoolsUpdateTrees: Fold the trees based on fold type and comparison type, -// update the edges, and unlink removed blocks +// optOptimizeBoolsKeepFirstBlockAndFreeTheRest: update the edges, and unlink removed blocks // -void OptBoolsDsc::optOptimizeBoolsUpdateTrees() +// Arguments: +// cmpOp - comparison operation +// optReturnBlock - defines whether to make first block a return or condition block +// +void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(GenTree* cmpOp, bool optReturnBlock) { - assert(m_b1 != nullptr && m_b2 != nullptr); - - bool optReturnBlock = false; - if (m_b3 != nullptr) - { - optReturnBlock = true; - } - - assert(m_cmpOp != GT_NONE && m_c1 != nullptr && m_c2 != nullptr); - - GenTree* cmpOp1 = m_foldOp == GT_NONE ? m_c1 : m_comp->gtNewOperNode(m_foldOp, m_foldType, m_c1, m_c2); - if (m_testInfo1.isBool && m_testInfo2.isBool) - { - // When we 'OR'/'AND' two booleans, the result is boolean as well - cmpOp1->gtFlags |= GTF_BOOLEAN; - } - - GenTree* t1Comp = m_testInfo1.compTree; - t1Comp->SetOper(m_cmpOp); - t1Comp->AsOp()->gtOp1 = cmpOp1; - t1Comp->AsOp()->gtOp2->gtType = m_foldType; // Could have been varTypeIsGC() - if (optReturnBlock) - { - // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN) - t1Comp->AsOp()->gtOp2->AsIntCon()->gtIconVal = 0; - m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); - m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); - - // Update the return count of flow graph - assert(m_comp->fgReturnCount >= 2); - --m_comp->fgReturnCount; - } - -#if FEATURE_SET_FLAGS + #if FEATURE_SET_FLAGS // For comparisons against zero we will have the GTF_SET_FLAGS set // and this can cause an assert to fire in fgMoveOpsLeft(GenTree* tree) // during the CSE phase. @@ -1291,10 +1263,10 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() // we generate an instruction that sets the flags, which allows us // to omit the cmp with zero instruction. - // Request that the codegen for cmpOp1 sets the condition flags - // when it generates the code for cmpOp1. + // Request that the codegen for cmpOp sets the condition flags + // when it generates the code for cmpOp. // - cmpOp1->gtRequestSetFlags(); + cmpOp->gtRequestSetFlags(); #endif // Recost/rethread the tree if necessary @@ -1390,6 +1362,48 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() m_b1->bbCodeOffsEnd = optReturnBlock ? m_b3->bbCodeOffsEnd : m_b2->bbCodeOffsEnd; } +//----------------------------------------------------------------------------- +// optOptimizeBoolsUpdateTrees: Fold the trees based on fold type and comparison type, +// update the edges, and unlink removed blocks +// +void OptBoolsDsc::optOptimizeBoolsUpdateTrees() +{ + assert(m_b1 != nullptr && m_b2 != nullptr); + + bool optReturnBlock = false; + if (m_b3 != nullptr) + { + optReturnBlock = true; + } + + assert(m_cmpOp != GT_NONE && m_c1 != nullptr && m_c2 != nullptr); + + GenTree* cmpOp1 = m_foldOp == GT_NONE ? m_c1 : m_comp->gtNewOperNode(m_foldOp, m_foldType, m_c1, m_c2); + if (m_testInfo1.isBool && m_testInfo2.isBool) + { + // When we 'OR'/'AND' two booleans, the result is boolean as well + cmpOp1->gtFlags |= GTF_BOOLEAN; + } + + GenTree* t1Comp = m_testInfo1.compTree; + t1Comp->SetOper(m_cmpOp); + t1Comp->AsOp()->gtOp1 = cmpOp1; + t1Comp->AsOp()->gtOp2->gtType = m_foldType; // Could have been varTypeIsGC() + if (optReturnBlock) + { + // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN) + t1Comp->AsOp()->gtOp2->AsIntCon()->gtIconVal = 0; + m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); + m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); + + // Update the return count of flow graph + assert(m_comp->fgReturnCount >= 2); + --m_comp->fgReturnCount; + } + + optOptimizeBoolsKeepFirstBlockAndFreeTheRest(cmpOp1, optReturnBlock); +} + //----------------------------------------------------------------------------- // optOptimizeBoolsReturnBlock: Optimize boolean when m_b1 is BBJ_COND and m_b2 and m_b3 are BBJ_RETURN // @@ -1537,10 +1551,10 @@ bool OptBoolsDsc::optOptimizeBoolsReturnBlock(BasicBlock* b3) (it1val == 0 && it2val == 0 && it3val == 0)) { // Case: x == 0 && y == 0 - // t1:c1!=0 t2:c2==0 t3:c3==0 - // ==> true if (c1|c2)==0 - foldOp = GT_OR; - cmpOp = GT_EQ; + // t1:c1!=0 t2:c2==0 t3:c3==0 + // ==> true if (c1|c2)==0 + foldOp = GT_OR; + cmpOp = GT_EQ; } else if ((m_testInfo1.compTree->gtOper == GT_EQ && m_testInfo2.compTree->gtOper == GT_NE) && (it1val == 0 && it2val == 0 && it3val == 0)) @@ -1771,6 +1785,90 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest) return opr1; } +//----------------------------------------------------------------------------- +// optOptimizeAndConditionWithEqualityOperator: Optimize boolean when m_b1 is BBJ_COND with EQ on integers, m_b2 is BBJ_RETURN with EQ on integers and m_b3 is BBJ_RETURN of False +// +// Arguments: +// b3: Pointer to basic block b3 +// +// Returns: +// true if boolean optimization is done and m_b1, m_b2 and m_b3 are folded into m_b1, else false. +// +// Notes: +// m_b1, m_b2 and m_b3 of OptBoolsDsc are set on entry. +// +// if B1->TargetIs(b3), it transforms +// B1 : brtrue(t1, B3) +// B2 : ret(t2) +// B3 : ret(0) +// to +// B1 : ret((!t1) && t2) +// +bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) +{ + Statement* const s1 = optOptimizeBoolsChkBlkCond(); + if (s1 == nullptr) + { + return false; + } + + GenTree* cond1 = m_testInfo1.GetTestOp(); + GenTree* cond2 = m_testInfo2.GetTestOp(); + + if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE)) + { + return false; + } + + GenTree* op11 = cond1->AsOp()->gtOp1; + GenTree* op12 = cond1->AsOp()->gtOp2; + GenTree* op21 = cond2->AsOp()->gtOp1; + GenTree* op22 = cond2->AsOp()->gtOp2; + + if (!op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT)) + { + return false; + } + + GenTreeOp* xorOp1 = m_comp->gtNewOperNode(GT_XOR, TYP_INT, op11, op12); + xorOp1->gtFlags |= GTF_BOOLEAN; + xorOp1->gtPrev = op12; + op12->gtNext = xorOp1; + op12->gtPrev = op11; + op11->gtNext = op12; + op11->gtPrev = nullptr; + GenTreeOp* xorOp2 = m_comp->gtNewOperNode(GT_XOR, TYP_INT, op21, op22); + xorOp2->gtFlags |= GTF_BOOLEAN; + xorOp2->gtPrev = op22; + op22->gtNext = xorOp2; + op22->gtPrev = op21; + op21->gtNext = op22; + + GenTreeOp* branchlessOrEqOp = m_comp->gtNewOperNode(GT_OR, TYP_INT, xorOp1, xorOp2); + branchlessOrEqOp->gtFlags |= GTF_BOOLEAN; + branchlessOrEqOp->gtPrev = xorOp2; + xorOp2->gtNext = branchlessOrEqOp; + op21->gtPrev = xorOp1; + xorOp1->gtNext = op21; + + GenTreeIntCon* trueOp = m_comp->gtNewFalse(); + m_testInfo1.compTree->SetOper(cond2->OperIs(GT_EQ) ? GT_EQ : GT_NE); + m_testInfo1.compTree->gtFlags |= GTF_BOOLEAN; + m_testInfo1.compTree->gtType = TYP_INT; + m_testInfo1.compTree->AsOp()->gtOp1 = branchlessOrEqOp; + m_testInfo1.compTree->AsOp()->gtOp2 = trueOp; + m_testInfo1.compTree->gtPrev = trueOp; + trueOp->gtNext = m_testInfo1.compTree; + trueOp->gtPrev = branchlessOrEqOp; + branchlessOrEqOp->gtNext = trueOp; + + m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); + m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); + + optOptimizeBoolsKeepFirstBlockAndFreeTheRest(m_testInfo1.compTree, true); + return true; +} + //----------------------------------------------------------------------------- // optOptimizeBools: Folds boolean conditionals for GT_JTRUE/GT_RETURN/GT_SWIFT_ERROR_RET nodes // @@ -1894,6 +1992,54 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest) // +--* LCL_VAR int V00 arg0 // \--* CNS_INT int 0 // +// Case 16: (x = 128 && y = 89) => (x^128) || (y^89) +// * RETURN int $VN.Void +// \--* EQ int +// +--* OR int +// | +--* XOR int +// | | +--* LCL_VAR int V00 arg0 +// | | \--* CNS_INT int 128 $43 +// | \--* XOR int +// | +--* LCL_VAR int V01 arg1 +// | \--* CNS_INT int 89 $44 +// \--* CNS_INT int 0 +// +// Case 17: (x = y && v = z) => (x^y) || (v^z) +// * RETURN int $VN.Void +// \--* EQ int +// +--* OR int +// | +--* XOR int +// | | +--* LCL_VAR int V00 arg0 +// | | \--* LCL_VAR int V00 arg1 +// | \--* XOR int +// | +--* LCL_VAR int V01 arg2 +// | \--* LCL_VAR int V00 arg3 +// \--* CNS_INT int 0 +// +// Case 18: (x != 128 || y != 89) => !((x^128) || (y^89)) +// * RETURN int $VN.Void +// \--* NE int +// +--* OR int +// | +--* XOR int +// | | +--* LCL_VAR int V00 arg0 +// | | \--* CNS_INT int 128 $43 +// | \--* XOR int +// | +--* LCL_VAR int V01 arg1 +// | \--* CNS_INT int 89 $44 +// \--* CNS_INT int 0 +// +// Case 19: (x != y && v != z) => !((x^y) || (v^z)) +// * RETURN int $VN.Void +// \--* NE int +// +--* OR int +// | +--* XOR int +// | | +--* LCL_VAR int V00 arg0 +// | | \--* LCL_VAR int V00 arg1 +// | \--* XOR int +// | +--* LCL_VAR int V01 arg2 +// | \--* LCL_VAR int V00 arg3 +// \--* CNS_INT int 0 +// // Patterns that are not optimized include (x == 1 && y == 1), (x == 1 || y == 1), // (x == 0 || y == 0) because currently their comptree is not marked as boolean expression. // When m_foldOp == GT_AND or m_cmpOp == GT_NE, both compTrees must be boolean expression @@ -2001,7 +2147,7 @@ PhaseStatus Compiler::optOptimizeBools() continue; } - if (optBoolsDsc.optOptimizeBoolsReturnBlock(b3)) + if (optBoolsDsc.optOptimizeBoolsReturnBlock(b3) || optBoolsDsc.optOptimizeAndConditionWithEqualityOperator(b3)) { change = true; numReturn++; diff --git a/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs b/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs index 93dd6a0991b89d..9534a30276d261 100644 --- a/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs +++ b/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs @@ -327,6 +327,18 @@ private static bool IsLessThanZeroBis(int x) return b; } + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool IsAEqual128AndB89(int a, int b) => a == 128 && b == 89; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool IsAEqualBAndC89(int a, int b, int c) => a == b && c == 89; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool IsAEqualBAndCEqualD(int a, int b, int c, int d) => a == b && c == d; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool IsANeither128OrB89(int a, int b) => a != 128 || b != 89; + [Fact] public static int TestEntryPoint() { @@ -1116,6 +1128,96 @@ public static int TestEntryPoint() return 101; } + if (IsAEqual128AndB89(128, 3)) + { + Console.WriteLine($"IsAEqual128AndB89(128, 3) failed"); + return 101; + } + + if (IsAEqual128AndB89(5, 89)) + { + Console.WriteLine($"IsAEqual128AndB89(5, 89) failed"); + return 101; + } + + if (IsAEqual128AndB89(5, 3)) + { + Console.WriteLine($"IsAEqual128AndB89(5, 3) failed"); + return 101; + } + + if (!IsAEqual128AndB89(128, 89)) + { + Console.WriteLine($"IsAEqual128AndB89(128, 89) failed"); + return 101; + } + + if (IsAEqualBAndC89(128, 128, 3)) + { + Console.WriteLine($"IsAEqualBAndC89(128, 128, 3) failed"); + return 101; + } + + if (IsAEqualBAndC89(3, 128, 89)) + { + Console.WriteLine($"IsAEqualBAndC89(3, 128, 89) failed"); + return 101; + } + + if (!IsAEqualBAndC89(128, 128, 89)) + { + Console.WriteLine($"IsAEqualBAndC89(128, 128, 89) failed"); + return 101; + } + + if (IsAEqualBAndCEqualD(128, 3, 89, 89)) + { + Console.WriteLine($"IsAEqualBAndCEqualD(128, 3, 89, 89) failed"); + return 101; + } + + if (IsAEqualBAndCEqualD(128, 128, 89, 3)) + { + Console.WriteLine($"IsAEqualBAndCEqualD(128, 128, 89, 3) failed"); + return 101; + } + + if (IsAEqualBAndCEqualD(128, 5, 89, 3)) + { + Console.WriteLine($"IsAEqualBAndCEqualD(128, 5, 89, 3) failed"); + return 101; + } + + if (!IsAEqualBAndCEqualD(128, 128, 89, 89)) + { + Console.WriteLine($"IsAEqualBAndCEqualD(128, 128, 89, 89) failed"); + return 101; + } + + if (IsANeither128OrB89(128, 89)) + { + Console.WriteLine($"IsANeither128OrB89(128, 89) failed"); + return 101; + } + + if (!IsANeither128OrB89(3, 89)) + { + Console.WriteLine($"IsANeither128OrB89(3, 89) failed"); + return 101; + } + + if (!IsANeither128OrB89(128, 5)) + { + Console.WriteLine($"IsANeither128OrB89(128, 5) failed"); + return 101; + } + + if (!IsANeither128OrB89(3, 5)) + { + Console.WriteLine($"IsANeither128OrB89(3, 5) failed"); + return 101; + } + Console.WriteLine("PASSED"); return 100; } From 951ce9d7b67ebbda44b76bd376f7348f6e53328e Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sat, 11 May 2024 01:55:33 +0200 Subject: [PATCH 02/15] format file --- src/coreclr/jit/optimizebools.cpp | 51 ++++++++++++++++--------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index aa252f349b5459..9abbbb506c28cd 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1245,7 +1245,7 @@ bool OptBoolsDsc::optOptimizeBoolsChkTypeCostCond() // void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(GenTree* cmpOp, bool optReturnBlock) { - #if FEATURE_SET_FLAGS +#if FEATURE_SET_FLAGS // For comparisons against zero we will have the GTF_SET_FLAGS set // and this can cause an assert to fire in fgMoveOpsLeft(GenTree* tree) // during the CSE phase. @@ -1551,10 +1551,10 @@ bool OptBoolsDsc::optOptimizeBoolsReturnBlock(BasicBlock* b3) (it1val == 0 && it2val == 0 && it3val == 0)) { // Case: x == 0 && y == 0 - // t1:c1!=0 t2:c2==0 t3:c3==0 - // ==> true if (c1|c2)==0 - foldOp = GT_OR; - cmpOp = GT_EQ; + // t1:c1!=0 t2:c2==0 t3:c3==0 + // ==> true if (c1|c2)==0 + foldOp = GT_OR; + cmpOp = GT_EQ; } else if ((m_testInfo1.compTree->gtOper == GT_EQ && m_testInfo2.compTree->gtOper == GT_NE) && (it1val == 0 && it2val == 0 && it3val == 0)) @@ -1786,7 +1786,8 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest) } //----------------------------------------------------------------------------- -// optOptimizeAndConditionWithEqualityOperator: Optimize boolean when m_b1 is BBJ_COND with EQ on integers, m_b2 is BBJ_RETURN with EQ on integers and m_b3 is BBJ_RETURN of False +// optOptimizeAndConditionWithEqualityOperator: Optimize boolean when m_b1 is BBJ_COND with EQ on integers, m_b2 is +// BBJ_RETURN with EQ on integers and m_b3 is BBJ_RETURN of False // // Arguments: // b3: Pointer to basic block b3 @@ -1825,42 +1826,43 @@ bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) GenTree* op21 = cond2->AsOp()->gtOp1; GenTree* op22 = cond2->AsOp()->gtOp2; - if (!op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT)) + if (!op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || + !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT)) { return false; } GenTreeOp* xorOp1 = m_comp->gtNewOperNode(GT_XOR, TYP_INT, op11, op12); xorOp1->gtFlags |= GTF_BOOLEAN; - xorOp1->gtPrev = op12; - op12->gtNext = xorOp1; - op12->gtPrev = op11; - op11->gtNext = op12; - op11->gtPrev = nullptr; + xorOp1->gtPrev = op12; + op12->gtNext = xorOp1; + op12->gtPrev = op11; + op11->gtNext = op12; + op11->gtPrev = nullptr; GenTreeOp* xorOp2 = m_comp->gtNewOperNode(GT_XOR, TYP_INT, op21, op22); xorOp2->gtFlags |= GTF_BOOLEAN; xorOp2->gtPrev = op22; - op22->gtNext = xorOp2; - op22->gtPrev = op21; - op21->gtNext = op22; + op22->gtNext = xorOp2; + op22->gtPrev = op21; + op21->gtNext = op22; GenTreeOp* branchlessOrEqOp = m_comp->gtNewOperNode(GT_OR, TYP_INT, xorOp1, xorOp2); branchlessOrEqOp->gtFlags |= GTF_BOOLEAN; branchlessOrEqOp->gtPrev = xorOp2; - xorOp2->gtNext = branchlessOrEqOp; - op21->gtPrev = xorOp1; - xorOp1->gtNext = op21; + xorOp2->gtNext = branchlessOrEqOp; + op21->gtPrev = xorOp1; + xorOp1->gtNext = op21; GenTreeIntCon* trueOp = m_comp->gtNewFalse(); m_testInfo1.compTree->SetOper(cond2->OperIs(GT_EQ) ? GT_EQ : GT_NE); m_testInfo1.compTree->gtFlags |= GTF_BOOLEAN; - m_testInfo1.compTree->gtType = TYP_INT; + m_testInfo1.compTree->gtType = TYP_INT; m_testInfo1.compTree->AsOp()->gtOp1 = branchlessOrEqOp; m_testInfo1.compTree->AsOp()->gtOp2 = trueOp; - m_testInfo1.compTree->gtPrev = trueOp; - trueOp->gtNext = m_testInfo1.compTree; - trueOp->gtPrev = branchlessOrEqOp; - branchlessOrEqOp->gtNext = trueOp; + m_testInfo1.compTree->gtPrev = trueOp; + trueOp->gtNext = m_testInfo1.compTree; + trueOp->gtPrev = branchlessOrEqOp; + branchlessOrEqOp->gtNext = trueOp; m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); @@ -2147,7 +2149,8 @@ PhaseStatus Compiler::optOptimizeBools() continue; } - if (optBoolsDsc.optOptimizeBoolsReturnBlock(b3) || optBoolsDsc.optOptimizeAndConditionWithEqualityOperator(b3)) + if (optBoolsDsc.optOptimizeBoolsReturnBlock(b3) || + optBoolsDsc.optOptimizeAndConditionWithEqualityOperator(b3)) { change = true; numReturn++; From a8dbdbf0272ddbc0d20babad50697f22453d9f50 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sat, 11 May 2024 23:05:19 +0200 Subject: [PATCH 03/15] optimize only int operands --- src/coreclr/jit/optimizebools.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 9abbbb506c28cd..9ea3a9f0f6028c 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1827,7 +1827,8 @@ bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) GenTree* op22 = cond2->AsOp()->gtOp2; if (!op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || - !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT)) + !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op11->TypeIs(TYP_INT) || + !op12->TypeIs(TYP_INT) || !op21->TypeIs(TYP_INT) || !op22->TypeIs(TYP_INT)) { return false; } From 64a79f5aa90d42d9aa96e03173c2aed78e4265de Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sat, 11 May 2024 23:44:23 +0200 Subject: [PATCH 04/15] try to fix segmentation fault on arm --- src/coreclr/jit/optimizebools.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 9ea3a9f0f6028c..7795e1e3540061 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1255,8 +1255,16 @@ void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(GenTree* cmpOp, b // Make sure that the GTF_SET_FLAGS bit is cleared. // Fix 388436 ARM JitStress WP7 - m_c1->gtFlags &= ~GTF_SET_FLAGS; - m_c2->gtFlags &= ~GTF_SET_FLAGS; + + if (m_c1 != nullptr) + { + m_c1->gtFlags &= ~GTF_SET_FLAGS; + } + + if (m_c2 != nullptr) + { + m_c2->gtFlags &= ~GTF_SET_FLAGS; + } // The new top level node that we just created does feed directly into // a comparison against zero, so set the GTF_SET_FLAGS bit so that From 34c6eb720cc56bcf1efc97e89d6b7cebf16a1b97 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 12 May 2024 15:23:43 +0200 Subject: [PATCH 05/15] try fix errors in arm builds --- src/coreclr/jit/optimizebools.cpp | 65 ++++++++++++++----------------- 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 7795e1e3540061..8cb15d318e1f29 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -107,7 +107,7 @@ class OptBoolsDsc bool optOptimizeBoolsChkTypeCostCond(); void optOptimizeBoolsUpdateTrees(); bool FindCompareChain(GenTree* condition, bool* isTestCondition); - void optOptimizeBoolsKeepFirstBlockAndFreeTheRest(GenTree* cmpOp, bool optReturnBlock); + void optOptimizeBoolsKeepFirstBlockAndFreeTheRest(bool optReturnBlock); }; //----------------------------------------------------------------------------- @@ -1243,40 +1243,8 @@ bool OptBoolsDsc::optOptimizeBoolsChkTypeCostCond() // cmpOp - comparison operation // optReturnBlock - defines whether to make first block a return or condition block // -void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(GenTree* cmpOp, bool optReturnBlock) +void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(bool optReturnBlock) { -#if FEATURE_SET_FLAGS - // For comparisons against zero we will have the GTF_SET_FLAGS set - // and this can cause an assert to fire in fgMoveOpsLeft(GenTree* tree) - // during the CSE phase. - // - // So make sure to clear any GTF_SET_FLAGS bit on these operations - // as they are no longer feeding directly into a comparisons against zero - - // Make sure that the GTF_SET_FLAGS bit is cleared. - // Fix 388436 ARM JitStress WP7 - - if (m_c1 != nullptr) - { - m_c1->gtFlags &= ~GTF_SET_FLAGS; - } - - if (m_c2 != nullptr) - { - m_c2->gtFlags &= ~GTF_SET_FLAGS; - } - - // The new top level node that we just created does feed directly into - // a comparison against zero, so set the GTF_SET_FLAGS bit so that - // we generate an instruction that sets the flags, which allows us - // to omit the cmp with zero instruction. - - // Request that the codegen for cmpOp sets the condition flags - // when it generates the code for cmpOp. - // - cmpOp->gtRequestSetFlags(); -#endif - // Recost/rethread the tree if necessary // if (m_comp->fgNodeThreading != NodeThreading::None) @@ -1409,7 +1377,32 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() --m_comp->fgReturnCount; } - optOptimizeBoolsKeepFirstBlockAndFreeTheRest(cmpOp1, optReturnBlock); +#if FEATURE_SET_FLAGS + // For comparisons against zero we will have the GTF_SET_FLAGS set + // and this can cause an assert to fire in fgMoveOpsLeft(GenTree* tree) + // during the CSE phase. + // + // So make sure to clear any GTF_SET_FLAGS bit on these operations + // as they are no longer feeding directly into a comparisons against zero + + // Make sure that the GTF_SET_FLAGS bit is cleared. + // Fix 388436 ARM JitStress WP7 + + m_c1->gtFlags &= ~GTF_SET_FLAGS; + m_c2->gtFlags &= ~GTF_SET_FLAGS; + + // The new top level node that we just created does feed directly into + // a comparison against zero, so set the GTF_SET_FLAGS bit so that + // we generate an instruction that sets the flags, which allows us + // to omit the cmp with zero instruction. + + // Request that the codegen for cmpOp sets the condition flags + // when it generates the code for cmpOp. + // + cmpOp1->gtRequestSetFlags(); +#endif + + optOptimizeBoolsKeepFirstBlockAndFreeTheRest(optReturnBlock); } //----------------------------------------------------------------------------- @@ -1876,7 +1869,7 @@ bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); - optOptimizeBoolsKeepFirstBlockAndFreeTheRest(m_testInfo1.compTree, true); + optOptimizeBoolsKeepFirstBlockAndFreeTheRest(true); return true; } From 4f3700888f9d737439bbaecaeb6cfa7583d101b4 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 12 May 2024 20:08:21 +0200 Subject: [PATCH 06/15] fix regression on pattern a != || b== --- src/coreclr/jit/optimizebools.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 8cb15d318e1f29..770f4d73c61598 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1817,10 +1817,13 @@ bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) GenTree* cond1 = m_testInfo1.GetTestOp(); GenTree* cond2 = m_testInfo2.GetTestOp(); - if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE)) + ssize_t block3RetVal = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue()->AsIntCon()->IconValue(); + + if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE) || (block3RetVal != 0 && cond2->OperIs(GT_EQ)) || + (block3RetVal != 1 && cond2->OperIs(GT_NE))) { return false; - } + }; GenTree* op11 = cond1->AsOp()->gtOp1; GenTree* op12 = cond1->AsOp()->gtOp2; From 5591e9acf0568860fc4b373c8b0771ff9f6a6971 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Mon, 13 May 2024 23:21:22 +0200 Subject: [PATCH 07/15] do not optimize for arm --- src/coreclr/jit/optimizebools.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 770f4d73c61598..0d67d88e63620a 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1808,6 +1808,10 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest) // bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) { +#if defined(TARGET_ARM) || defined(TARGET_ARM64) + return false; +#endif + Statement* const s1 = optOptimizeBoolsChkBlkCond(); if (s1 == nullptr) { From c653261a5fce01d7dd096b918ad497cb702dbaa4 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Mon, 19 Aug 2024 23:12:52 +0200 Subject: [PATCH 08/15] rollback dev --- src/coreclr/jit/optimizebools.cpp | 253 ++++-------------------------- 1 file changed, 33 insertions(+), 220 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index a3eea942eea48b..cba1c020e5e1d9 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -96,7 +96,6 @@ class OptBoolsDsc bool optOptimizeCompareChainCondBlock(); bool optOptimizeRangeTests(); bool optOptimizeBoolsReturnBlock(BasicBlock* b3); - bool optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3); #ifdef DEBUG void optOptimizeBoolsGcStress(); #endif @@ -107,7 +106,6 @@ class OptBoolsDsc bool optOptimizeBoolsChkTypeCostCond(); void optOptimizeBoolsUpdateTrees(); bool FindCompareChain(GenTree* condition, bool* isTestCondition); - void optOptimizeBoolsKeepFirstBlockAndFreeTheRest(bool optReturnBlock); }; //----------------------------------------------------------------------------- @@ -1237,14 +1235,39 @@ bool OptBoolsDsc::optOptimizeBoolsChkTypeCostCond() } //----------------------------------------------------------------------------- -// optOptimizeBoolsKeepFirstBlockAndFreeTheRest: update the edges, and unlink removed blocks -// -// Arguments: -// cmpOp - comparison operation -// optReturnBlock - defines whether to make first block a return or condition block +// optOptimizeBoolsUpdateTrees: Fold the trees based on fold type and comparison type, +// update the edges, and unlink removed blocks // -void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(bool optReturnBlock) +void OptBoolsDsc::optOptimizeBoolsUpdateTrees() { + assert(m_b1 != nullptr && m_b2 != nullptr); + + bool optReturnBlock = false; + if (m_b3 != nullptr) + { + optReturnBlock = true; + } + + assert(m_cmpOp != GT_NONE && m_c1 != nullptr && m_c2 != nullptr); + + GenTree* cmpOp1 = m_foldOp == GT_NONE ? m_c1 : m_comp->gtNewOperNode(m_foldOp, m_foldType, m_c1, m_c2); + + GenTree* t1Comp = m_testInfo1.compTree; + t1Comp->SetOper(m_cmpOp); + t1Comp->AsOp()->gtOp1 = cmpOp1; + t1Comp->AsOp()->gtOp2->gtType = m_foldType; // Could have been varTypeIsGC() + if (optReturnBlock) + { + // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN) + t1Comp->AsOp()->gtOp2->AsIntCon()->gtIconVal = 0; + m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); + m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); + + // Update the return count of flow graph + assert(m_comp->fgReturnCount >= 2); + --m_comp->fgReturnCount; + } + // Recost/rethread the tree if necessary // if (m_comp->fgNodeThreading != NodeThreading::None) @@ -1337,73 +1360,6 @@ void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(bool optReturnBlo m_b1->bbCodeOffsEnd = optReturnBlock ? m_b3->bbCodeOffsEnd : m_b2->bbCodeOffsEnd; } -//----------------------------------------------------------------------------- -// optOptimizeBoolsUpdateTrees: Fold the trees based on fold type and comparison type, -// update the edges, and unlink removed blocks -// -void OptBoolsDsc::optOptimizeBoolsUpdateTrees() -{ - assert(m_b1 != nullptr && m_b2 != nullptr); - - bool optReturnBlock = false; - if (m_b3 != nullptr) - { - optReturnBlock = true; - } - - assert(m_cmpOp != GT_NONE && m_c1 != nullptr && m_c2 != nullptr); - - GenTree* cmpOp1 = m_foldOp == GT_NONE ? m_c1 : m_comp->gtNewOperNode(m_foldOp, m_foldType, m_c1, m_c2); - if (m_testInfo1.isBool && m_testInfo2.isBool) - { - // When we 'OR'/'AND' two booleans, the result is boolean as well - cmpOp1->gtFlags |= GTF_BOOLEAN; - } - - GenTree* t1Comp = m_testInfo1.compTree; - t1Comp->SetOper(m_cmpOp); - t1Comp->AsOp()->gtOp1 = cmpOp1; - t1Comp->AsOp()->gtOp2->gtType = m_foldType; // Could have been varTypeIsGC() - if (optReturnBlock) - { - // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN) - t1Comp->AsOp()->gtOp2->AsIntCon()->gtIconVal = 0; - m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); - m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); - - // Update the return count of flow graph - assert(m_comp->fgReturnCount >= 2); - --m_comp->fgReturnCount; - } - -#if FEATURE_SET_FLAGS - // For comparisons against zero we will have the GTF_SET_FLAGS set - // and this can cause an assert to fire in fgMoveOpsLeft(GenTree* tree) - // during the CSE phase. - // - // So make sure to clear any GTF_SET_FLAGS bit on these operations - // as they are no longer feeding directly into a comparisons against zero - - // Make sure that the GTF_SET_FLAGS bit is cleared. - // Fix 388436 ARM JitStress WP7 - - m_c1->gtFlags &= ~GTF_SET_FLAGS; - m_c2->gtFlags &= ~GTF_SET_FLAGS; - - // The new top level node that we just created does feed directly into - // a comparison against zero, so set the GTF_SET_FLAGS bit so that - // we generate an instruction that sets the flags, which allows us - // to omit the cmp with zero instruction. - - // Request that the codegen for cmpOp sets the condition flags - // when it generates the code for cmpOp. - // - cmpOp1->gtRequestSetFlags(); -#endif - - optOptimizeBoolsKeepFirstBlockAndFreeTheRest(optReturnBlock); -} - //----------------------------------------------------------------------------- // optOptimizeBoolsReturnBlock: Optimize boolean when m_b1 is BBJ_COND and m_b2 and m_b3 are BBJ_RETURN // @@ -1781,100 +1737,6 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest) return opr1; } -//----------------------------------------------------------------------------- -// optOptimizeAndConditionWithEqualityOperator: Optimize boolean when m_b1 is BBJ_COND with EQ on integers, m_b2 is -// BBJ_RETURN with EQ on integers and m_b3 is BBJ_RETURN of False -// -// Arguments: -// b3: Pointer to basic block b3 -// -// Returns: -// true if boolean optimization is done and m_b1, m_b2 and m_b3 are folded into m_b1, else false. -// -// Notes: -// m_b1, m_b2 and m_b3 of OptBoolsDsc are set on entry. -// -// if B1->TargetIs(b3), it transforms -// B1 : brtrue(t1, B3) -// B2 : ret(t2) -// B3 : ret(0) -// to -// B1 : ret((!t1) && t2) -// -bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) -{ -#if defined(TARGET_ARM) || defined(TARGET_ARM64) - return false; -#endif - - Statement* const s1 = optOptimizeBoolsChkBlkCond(); - if (s1 == nullptr) - { - return false; - } - - GenTree* cond1 = m_testInfo1.GetTestOp(); - GenTree* cond2 = m_testInfo2.GetTestOp(); - - ssize_t block3RetVal = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue()->AsIntCon()->IconValue(); - - if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE) || (block3RetVal != 0 && cond2->OperIs(GT_EQ)) || - (block3RetVal != 1 && cond2->OperIs(GT_NE))) - { - return false; - }; - - GenTree* op11 = cond1->AsOp()->gtOp1; - GenTree* op12 = cond1->AsOp()->gtOp2; - GenTree* op21 = cond2->AsOp()->gtOp1; - GenTree* op22 = cond2->AsOp()->gtOp2; - - if (!op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || - !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op11->TypeIs(TYP_INT) || - !op12->TypeIs(TYP_INT) || !op21->TypeIs(TYP_INT) || !op22->TypeIs(TYP_INT)) - { - return false; - } - - GenTreeOp* xorOp1 = m_comp->gtNewOperNode(GT_XOR, TYP_INT, op11, op12); - xorOp1->gtFlags |= GTF_BOOLEAN; - xorOp1->gtPrev = op12; - op12->gtNext = xorOp1; - op12->gtPrev = op11; - op11->gtNext = op12; - op11->gtPrev = nullptr; - GenTreeOp* xorOp2 = m_comp->gtNewOperNode(GT_XOR, TYP_INT, op21, op22); - xorOp2->gtFlags |= GTF_BOOLEAN; - xorOp2->gtPrev = op22; - op22->gtNext = xorOp2; - op22->gtPrev = op21; - op21->gtNext = op22; - - GenTreeOp* branchlessOrEqOp = m_comp->gtNewOperNode(GT_OR, TYP_INT, xorOp1, xorOp2); - branchlessOrEqOp->gtFlags |= GTF_BOOLEAN; - branchlessOrEqOp->gtPrev = xorOp2; - xorOp2->gtNext = branchlessOrEqOp; - op21->gtPrev = xorOp1; - xorOp1->gtNext = op21; - - GenTreeIntCon* trueOp = m_comp->gtNewFalse(); - m_testInfo1.compTree->SetOper(cond2->OperIs(GT_EQ) ? GT_EQ : GT_NE); - m_testInfo1.compTree->gtFlags |= GTF_BOOLEAN; - m_testInfo1.compTree->gtType = TYP_INT; - m_testInfo1.compTree->AsOp()->gtOp1 = branchlessOrEqOp; - m_testInfo1.compTree->AsOp()->gtOp2 = trueOp; - m_testInfo1.compTree->gtPrev = trueOp; - trueOp->gtNext = m_testInfo1.compTree; - trueOp->gtPrev = branchlessOrEqOp; - branchlessOrEqOp->gtNext = trueOp; - - m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); - m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); - - optOptimizeBoolsKeepFirstBlockAndFreeTheRest(true); - return true; -} - //----------------------------------------------------------------------------- // optOptimizeBools: Folds boolean conditionals for GT_JTRUE/GT_RETURN/GT_SWIFT_ERROR_RET nodes // @@ -1998,54 +1860,6 @@ bool OptBoolsDsc::optOptimizeAndConditionWithEqualityOperator(BasicBlock* b3) // +--* LCL_VAR int V00 arg0 // \--* CNS_INT int 0 // -// Case 16: (x = 128 && y = 89) => (x^128) || (y^89) -// * RETURN int $VN.Void -// \--* EQ int -// +--* OR int -// | +--* XOR int -// | | +--* LCL_VAR int V00 arg0 -// | | \--* CNS_INT int 128 $43 -// | \--* XOR int -// | +--* LCL_VAR int V01 arg1 -// | \--* CNS_INT int 89 $44 -// \--* CNS_INT int 0 -// -// Case 17: (x = y && v = z) => (x^y) || (v^z) -// * RETURN int $VN.Void -// \--* EQ int -// +--* OR int -// | +--* XOR int -// | | +--* LCL_VAR int V00 arg0 -// | | \--* LCL_VAR int V00 arg1 -// | \--* XOR int -// | +--* LCL_VAR int V01 arg2 -// | \--* LCL_VAR int V00 arg3 -// \--* CNS_INT int 0 -// -// Case 18: (x != 128 || y != 89) => !((x^128) || (y^89)) -// * RETURN int $VN.Void -// \--* NE int -// +--* OR int -// | +--* XOR int -// | | +--* LCL_VAR int V00 arg0 -// | | \--* CNS_INT int 128 $43 -// | \--* XOR int -// | +--* LCL_VAR int V01 arg1 -// | \--* CNS_INT int 89 $44 -// \--* CNS_INT int 0 -// -// Case 19: (x != y && v != z) => !((x^y) || (v^z)) -// * RETURN int $VN.Void -// \--* NE int -// +--* OR int -// | +--* XOR int -// | | +--* LCL_VAR int V00 arg0 -// | | \--* LCL_VAR int V00 arg1 -// | \--* XOR int -// | +--* LCL_VAR int V01 arg2 -// | \--* LCL_VAR int V00 arg3 -// \--* CNS_INT int 0 -// // Patterns that are not optimized include (x == 1 && y == 1), (x == 1 || y == 1), // (x == 0 || y == 0) because currently their comptree is not marked as boolean expression. // When m_foldOp == GT_AND or m_cmpOp == GT_NE, both compTrees must be boolean expression @@ -2153,8 +1967,7 @@ PhaseStatus Compiler::optOptimizeBools() continue; } - if (optBoolsDsc.optOptimizeBoolsReturnBlock(b3) || - optBoolsDsc.optOptimizeAndConditionWithEqualityOperator(b3)) + if (optBoolsDsc.optOptimizeBoolsReturnBlock(b3)) { change = true; numReturn++; @@ -2174,4 +1987,4 @@ PhaseStatus Compiler::optOptimizeBools() const bool modified = stress || ((numCond + numReturn) > 0); return modified ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; -} +} \ No newline at end of file From f6b4e8a286b7316a5c512b806e50933f71b2932e Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Mon, 19 Aug 2024 23:13:50 +0200 Subject: [PATCH 09/15] rollback test --- .../JIT/opt/OptimizeBools/optboolsreturn.cs | 104 +----------------- 1 file changed, 1 insertion(+), 103 deletions(-) diff --git a/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs b/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs index 9534a30276d261..4cbea34754c8af 100644 --- a/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs +++ b/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs @@ -327,18 +327,6 @@ private static bool IsLessThanZeroBis(int x) return b; } - [MethodImpl(MethodImplOptions.NoInlining)] - private static bool IsAEqual128AndB89(int a, int b) => a == 128 && b == 89; - - [MethodImpl(MethodImplOptions.NoInlining)] - private static bool IsAEqualBAndC89(int a, int b, int c) => a == b && c == 89; - - [MethodImpl(MethodImplOptions.NoInlining)] - private static bool IsAEqualBAndCEqualD(int a, int b, int c, int d) => a == b && c == d; - - [MethodImpl(MethodImplOptions.NoInlining)] - private static bool IsANeither128OrB89(int a, int b) => a != 128 || b != 89; - [Fact] public static int TestEntryPoint() { @@ -1128,97 +1116,7 @@ public static int TestEntryPoint() return 101; } - if (IsAEqual128AndB89(128, 3)) - { - Console.WriteLine($"IsAEqual128AndB89(128, 3) failed"); - return 101; - } - - if (IsAEqual128AndB89(5, 89)) - { - Console.WriteLine($"IsAEqual128AndB89(5, 89) failed"); - return 101; - } - - if (IsAEqual128AndB89(5, 3)) - { - Console.WriteLine($"IsAEqual128AndB89(5, 3) failed"); - return 101; - } - - if (!IsAEqual128AndB89(128, 89)) - { - Console.WriteLine($"IsAEqual128AndB89(128, 89) failed"); - return 101; - } - - if (IsAEqualBAndC89(128, 128, 3)) - { - Console.WriteLine($"IsAEqualBAndC89(128, 128, 3) failed"); - return 101; - } - - if (IsAEqualBAndC89(3, 128, 89)) - { - Console.WriteLine($"IsAEqualBAndC89(3, 128, 89) failed"); - return 101; - } - - if (!IsAEqualBAndC89(128, 128, 89)) - { - Console.WriteLine($"IsAEqualBAndC89(128, 128, 89) failed"); - return 101; - } - - if (IsAEqualBAndCEqualD(128, 3, 89, 89)) - { - Console.WriteLine($"IsAEqualBAndCEqualD(128, 3, 89, 89) failed"); - return 101; - } - - if (IsAEqualBAndCEqualD(128, 128, 89, 3)) - { - Console.WriteLine($"IsAEqualBAndCEqualD(128, 128, 89, 3) failed"); - return 101; - } - - if (IsAEqualBAndCEqualD(128, 5, 89, 3)) - { - Console.WriteLine($"IsAEqualBAndCEqualD(128, 5, 89, 3) failed"); - return 101; - } - - if (!IsAEqualBAndCEqualD(128, 128, 89, 89)) - { - Console.WriteLine($"IsAEqualBAndCEqualD(128, 128, 89, 89) failed"); - return 101; - } - - if (IsANeither128OrB89(128, 89)) - { - Console.WriteLine($"IsANeither128OrB89(128, 89) failed"); - return 101; - } - - if (!IsANeither128OrB89(3, 89)) - { - Console.WriteLine($"IsANeither128OrB89(3, 89) failed"); - return 101; - } - - if (!IsANeither128OrB89(128, 5)) - { - Console.WriteLine($"IsANeither128OrB89(128, 5) failed"); - return 101; - } - - if (!IsANeither128OrB89(3, 5)) - { - Console.WriteLine($"IsANeither128OrB89(3, 5) failed"); - return 101; - } - Console.WriteLine("PASSED"); return 100; } -} +} \ No newline at end of file From 0bfee18f75dd8d592d08f762689e09c1025a25f2 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Wed, 28 Aug 2024 21:49:52 +0200 Subject: [PATCH 10/15] fix remarks --- src/coreclr/jit/lower.h | 4 +- src/coreclr/jit/lowerxarch.cpp | 82 ++++++++++++++++ src/coreclr/jit/optimizebools.cpp | 153 +++++++++++++++++++++++------- 3 files changed, 205 insertions(+), 34 deletions(-) diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index c5c771167025e4..00cdeb8b63308e 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -89,8 +89,10 @@ class Lowering final : public Phase void ContainCheckReturnTrap(GenTreeOp* node); void ContainCheckLclHeap(GenTreeOp* node); void ContainCheckRet(GenTreeUnOp* ret); +#if defined(TARGET_ARM64) || defined(TARGET_XARCH) + bool TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next); +#endif #ifdef TARGET_ARM64 - bool TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next); insCflags TruthifyingFlags(GenCondition cond); void ContainCheckConditionalCompare(GenTreeCCMP* ccmp); void ContainCheckNeg(GenTreeOp* neg); diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index e2c4671974773b..fb0184e7761cd0 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -318,6 +318,17 @@ GenTree* Lowering::LowerBinaryArithmetic(GenTreeOp* binOp) } #endif +#ifdef TARGET_XARCH + if (binOp->OperIs(GT_OR, GT_AND)) + { + GenTree* next; + if (TryLowerAndOrToCCMP(binOp, &next)) + { + return next; + } + } +#endif + ContainCheckBinary(binOp); return binOp->gtNext; @@ -8642,6 +8653,77 @@ void Lowering::ContainCheckCast(GenTreeCast* node) #endif // !defined(TARGET_64BIT) } +//------------------------------------------------------------------------ +// TryLowerAndOrToCCMP : Lower AND/OR of two conditions into test + CCMP + SETCC nodes. +// +// Arguments: +// tree - pointer to the node +// next - [out] Next node to lower if this function returns true +// +// Return Value: +// false if no changes were made +// +bool Lowering::TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next) +{ + assert(tree->OperIs(GT_OR, GT_AND)); + + if (!comp->opts.OptimizationEnabled()) + { + return false; + } + + GenTree* lastNode = BlockRange().LastNode(); + + if (!lastNode->OperIs(GT_JTRUE, GT_RETURN)) + { + return false; + } + + GenTree* op1 = tree->gtGetOp1(); + GenTree* op2 = tree->gtGetOp2(); + + if ((lastNode->OperIs(GT_JTRUE) && ((tree->OperIs(GT_OR) && (!op1->OperIs(GT_NE) || !op2->OperIs(GT_NE))) || + (tree->OperIs(GT_AND) && (!op1->OperIs(GT_EQ) || !op2->OperIs(GT_EQ))))) || + (lastNode->OperIs(GT_RETURN) && ((tree->OperIs(GT_OR) && (!op1->OperIs(GT_EQ) || !op2->OperIs(GT_EQ))) || + (tree->OperIs(GT_AND) && (!op1->OperIs(GT_NE) || !op2->OperIs(GT_NE)))))) + { + return false; + } + + GenTree* op11 = op1->gtGetOp1(); + GenTree* op12 = op1->gtGetOp2(); + GenTree* op21 = op2->gtGetOp1(); + GenTree* op22 = op2->gtGetOp2(); + + if (!varTypeIsIntegralOrI(op11) || !op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !varTypeIsIntegralOrI(op12) || + !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || !varTypeIsIntegralOrI(op21) || !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || + !varTypeIsIntegralOrI(op22) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT)) + { + return false; + } + + op1->gtGetOp1()->ClearContained(); + op1->gtGetOp2()->ClearContained(); + op2->gtGetOp1()->ClearContained(); + op2->gtGetOp2()->ClearContained(); + + op1->SetOper(GT_XOR); + op2->SetOper(GT_XOR); + + if (lastNode->OperIs(GT_JTRUE) && tree->OperIs(GT_AND)) + { + tree->SetOper(GT_OR); + tree->gtNext->gtNext->SetOper(GT_EQ); + } + + JITDUMP("Conversion was legal. Result:\n"); + DISPTREERANGE(BlockRange(), tree); + JITDUMP("\n"); + + *next = tree->gtNext; + return true; +} + //------------------------------------------------------------------------ // ContainCheckCompare: determine whether the sources of a compare node should be contained. // diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index cba1c020e5e1d9..0f24e0465d3e79 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -96,6 +96,7 @@ class OptBoolsDsc bool optOptimizeCompareChainCondBlock(); bool optOptimizeRangeTests(); bool optOptimizeBoolsReturnBlock(BasicBlock* b3); + bool optOptimizeCompareChainReturnBlock(BasicBlock* b3); #ifdef DEBUG void optOptimizeBoolsGcStress(); #endif @@ -106,6 +107,7 @@ class OptBoolsDsc bool optOptimizeBoolsChkTypeCostCond(); void optOptimizeBoolsUpdateTrees(); bool FindCompareChain(GenTree* condition, bool* isTestCondition); + void optOptimizeBoolsKeepFirstBlockAndFreeTheRest(bool optReturnBlock); }; //----------------------------------------------------------------------------- @@ -398,6 +400,7 @@ bool OptBoolsDsc::FindCompareChain(GenTree* condition, bool* isTestCondition) *isTestCondition = false; +#if defined(TARGET_ARM64) || defined(TARGET_XARCH) if (condition->OperIs(GT_EQ, GT_NE) && condOp2->IsIntegralConst()) { ssize_t condOp2Value = condOp2->AsIntCon()->IconValue(); @@ -427,6 +430,7 @@ bool OptBoolsDsc::FindCompareChain(GenTree* condition, bool* isTestCondition) *isTestCondition = true; } } +#endif return false; } @@ -1235,39 +1239,13 @@ bool OptBoolsDsc::optOptimizeBoolsChkTypeCostCond() } //----------------------------------------------------------------------------- -// optOptimizeBoolsUpdateTrees: Fold the trees based on fold type and comparison type, -// update the edges, and unlink removed blocks +// optOptimizeBoolsKeepFirstBlockAndFreeTheRest: update the edges, and unlink removed blocks // -void OptBoolsDsc::optOptimizeBoolsUpdateTrees() +// Arguments: +// optReturnBlock - defines whether to make first block a return or condition block +// +void OptBoolsDsc::optOptimizeBoolsKeepFirstBlockAndFreeTheRest(bool optReturnBlock) { - assert(m_b1 != nullptr && m_b2 != nullptr); - - bool optReturnBlock = false; - if (m_b3 != nullptr) - { - optReturnBlock = true; - } - - assert(m_cmpOp != GT_NONE && m_c1 != nullptr && m_c2 != nullptr); - - GenTree* cmpOp1 = m_foldOp == GT_NONE ? m_c1 : m_comp->gtNewOperNode(m_foldOp, m_foldType, m_c1, m_c2); - - GenTree* t1Comp = m_testInfo1.compTree; - t1Comp->SetOper(m_cmpOp); - t1Comp->AsOp()->gtOp1 = cmpOp1; - t1Comp->AsOp()->gtOp2->gtType = m_foldType; // Could have been varTypeIsGC() - if (optReturnBlock) - { - // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN) - t1Comp->AsOp()->gtOp2->AsIntCon()->gtIconVal = 0; - m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); - m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); - - // Update the return count of flow graph - assert(m_comp->fgReturnCount >= 2); - --m_comp->fgReturnCount; - } - // Recost/rethread the tree if necessary // if (m_comp->fgNodeThreading != NodeThreading::None) @@ -1360,6 +1338,39 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() m_b1->bbCodeOffsEnd = optReturnBlock ? m_b3->bbCodeOffsEnd : m_b2->bbCodeOffsEnd; } +void OptBoolsDsc::optOptimizeBoolsUpdateTrees() +{ + assert(m_b1 != nullptr && m_b2 != nullptr); + + bool optReturnBlock = false; + if (m_b3 != nullptr) + { + optReturnBlock = true; + } + + assert(m_cmpOp != GT_NONE && m_c1 != nullptr && m_c2 != nullptr); + + GenTree* cmpOp1 = m_foldOp == GT_NONE ? m_c1 : m_comp->gtNewOperNode(m_foldOp, m_foldType, m_c1, m_c2); + + GenTree* t1Comp = m_testInfo1.compTree; + t1Comp->SetOper(m_cmpOp); + t1Comp->AsOp()->gtOp1 = cmpOp1; + t1Comp->AsOp()->gtOp2->gtType = m_foldType; // Could have been varTypeIsGC() + if (optReturnBlock) + { + // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN) + t1Comp->AsOp()->gtOp2->AsIntCon()->gtIconVal = 0; + m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); + m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); + + // Update the return count of flow graph + assert(m_comp->fgReturnCount >= 2); + --m_comp->fgReturnCount; + } + + optOptimizeBoolsKeepFirstBlockAndFreeTheRest(optReturnBlock); +} + //----------------------------------------------------------------------------- // optOptimizeBoolsReturnBlock: Optimize boolean when m_b1 is BBJ_COND and m_b2 and m_b3 are BBJ_RETURN // @@ -1737,6 +1748,75 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest) return opr1; } +#ifdef TARGET_XARCH +bool OptBoolsDsc::optOptimizeCompareChainReturnBlock(BasicBlock* b3) +{ + Statement* const s1 = optOptimizeBoolsChkBlkCond(); + if (s1 == nullptr) + { + return false; + } + + GenTree* cond1 = m_testInfo1.GetTestOp(); + GenTree* cond2 = m_testInfo2.GetTestOp(); + + ssize_t block3RetVal = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue()->AsIntCon()->IconValue(); + + if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE) || (block3RetVal != 0 && cond2->OperIs(GT_EQ)) || + (block3RetVal != 1 && cond2->OperIs(GT_NE))) + { + return false; + }; + + GenTree* op11 = cond1->AsOp()->gtOp1; + GenTree* op12 = cond1->AsOp()->gtOp2; + GenTree* op21 = cond2->AsOp()->gtOp1; + GenTree* op22 = cond2->AsOp()->gtOp2; + + if (!op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || + !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op11->TypeIs(TYP_INT) || + !op12->TypeIs(TYP_INT) || !op21->TypeIs(TYP_INT) || !op22->TypeIs(TYP_INT)) + { + return false; + } + + GenTreeOp* xorOp1 = m_comp->gtNewOperNode(GT_EQ, TYP_INT, op11, op12); + xorOp1->gtPrev = op12; + op12->gtNext = xorOp1; + op12->gtPrev = op11; + op11->gtNext = op12; + op11->gtPrev = nullptr; + GenTreeOp* xorOp2 = m_comp->gtNewOperNode(GT_EQ, TYP_INT, op21, op22); + xorOp2->gtPrev = op22; + op22->gtNext = xorOp2; + op22->gtPrev = op21; + op21->gtNext = op22; + + GenTreeOp* branchlessOrEqOp = m_comp->gtNewOperNode(GT_OR, TYP_INT, xorOp1, xorOp2); + branchlessOrEqOp->gtPrev = xorOp2; + xorOp2->gtNext = branchlessOrEqOp; + op21->gtPrev = xorOp1; + xorOp1->gtNext = op21; + + GenTreeIntCon* trueOp = m_comp->gtNewFalse(); + m_testInfo1.compTree->SetOper(cond2->OperIs(GT_EQ) ? GT_EQ : GT_NE); + m_testInfo1.compTree->gtType = TYP_INT; + + m_testInfo1.compTree->AsOp()->gtOp1 = branchlessOrEqOp; + m_testInfo1.compTree->AsOp()->gtOp2 = trueOp; + m_testInfo1.compTree->gtPrev = trueOp; + trueOp->gtNext = m_testInfo1.compTree; + trueOp->gtPrev = branchlessOrEqOp; + branchlessOrEqOp->gtNext = trueOp; + + m_testInfo1.testTree->gtOper = m_testInfo2.testTree->OperGet(); + m_testInfo1.testTree->gtType = m_testInfo2.testTree->TypeGet(); + + optOptimizeBoolsKeepFirstBlockAndFreeTheRest(true); + return true; +} +#endif + //----------------------------------------------------------------------------- // optOptimizeBools: Folds boolean conditionals for GT_JTRUE/GT_RETURN/GT_SWIFT_ERROR_RET nodes // @@ -1937,7 +2017,7 @@ PhaseStatus Compiler::optOptimizeBools() retry = true; numCond++; } -#ifdef TARGET_ARM64 +#if defined(TARGET_ARM64) || defined(TARGET_XARCH) else if (optBoolsDsc.optOptimizeCompareChainCondBlock()) { // The optimization will have merged b1 and b2. Retry the loop so that @@ -1972,6 +2052,13 @@ PhaseStatus Compiler::optOptimizeBools() change = true; numReturn++; } +#ifdef TARGET_XARCH + else if (optBoolsDsc.optOptimizeCompareChainReturnBlock(b3)) + { + change = true; + numReturn++; + } +#endif } else { @@ -1987,4 +2074,4 @@ PhaseStatus Compiler::optOptimizeBools() const bool modified = stress || ((numCond + numReturn) > 0); return modified ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; -} \ No newline at end of file +} From e24840dfe2d7dc5ec6a496b2b8a0be43eed2b364 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Wed, 28 Aug 2024 23:12:15 +0200 Subject: [PATCH 11/15] fix build errors 1 --- src/coreclr/jit/optimizebools.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 0f24e0465d3e79..2ccb666c162445 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1760,6 +1760,13 @@ bool OptBoolsDsc::optOptimizeCompareChainReturnBlock(BasicBlock* b3) GenTree* cond1 = m_testInfo1.GetTestOp(); GenTree* cond2 = m_testInfo2.GetTestOp(); + GenTree* block3ReturnTree = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue(); + + if (block3ReturnTree == nullptr || !block3ReturnTree->OperIs(GT_CNS_INT)) + { + return false; + } + ssize_t block3RetVal = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue()->AsIntCon()->IconValue(); if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE) || (block3RetVal != 0 && cond2->OperIs(GT_EQ)) || From 781c223de03061a705479763b7db9368dd473a10 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Wed, 28 Aug 2024 23:58:09 +0200 Subject: [PATCH 12/15] fix build errors 2 --- src/coreclr/jit/optimizebools.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 2ccb666c162445..9596dc54ccfa43 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1762,12 +1762,12 @@ bool OptBoolsDsc::optOptimizeCompareChainReturnBlock(BasicBlock* b3) GenTree* block3ReturnTree = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue(); - if (block3ReturnTree == nullptr || !block3ReturnTree->OperIs(GT_CNS_INT)) + if (block3ReturnTree == nullptr || !block3ReturnTree->OperIs(GT_CNS_INT) || !block3ReturnTree->TypeIs(TYP_INT)) { return false; } - ssize_t block3RetVal = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue()->AsIntCon()->IconValue(); + ssize_t block3RetVal = block3ReturnTree->AsIntCon()->IconValue(); if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE) || (block3RetVal != 0 && cond2->OperIs(GT_EQ)) || (block3RetVal != 1 && cond2->OperIs(GT_NE))) From 0d2ba7bac4f1f611715620863390ddfbdf679948 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sat, 31 Aug 2024 23:13:17 +0200 Subject: [PATCH 13/15] fix build errors 3 --- src/coreclr/jit/lowerxarch.cpp | 9 +++++---- src/coreclr/jit/optimizebools.cpp | 2 -- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 049c5f559b3cdd..377cc7f9d37a57 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -8678,7 +8678,8 @@ bool Lowering::TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next) GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); - if ((lastNode->OperIs(GT_JTRUE) && ((tree->OperIs(GT_OR) && (!op1->OperIs(GT_NE) || !op2->OperIs(GT_NE))) || + if (!op1->OperIs(GT_NE, GT_EQ) || !op2->OperIs(GT_NE, GT_EQ) || + (lastNode->OperIs(GT_JTRUE) && ((tree->OperIs(GT_OR) && (!op1->OperIs(GT_NE) || !op2->OperIs(GT_NE))) || (tree->OperIs(GT_AND) && (!op1->OperIs(GT_EQ) || !op2->OperIs(GT_EQ))))) || (lastNode->OperIs(GT_RETURN) && ((tree->OperIs(GT_OR) && (!op1->OperIs(GT_EQ) || !op2->OperIs(GT_EQ))) || (tree->OperIs(GT_AND) && (!op1->OperIs(GT_NE) || !op2->OperIs(GT_NE)))))) @@ -8691,9 +8692,9 @@ bool Lowering::TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next) GenTree* op21 = op2->gtGetOp1(); GenTree* op22 = op2->gtGetOp2(); - if (!varTypeIsIntegralOrI(op11) || !op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !varTypeIsIntegralOrI(op12) || - !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || !varTypeIsIntegralOrI(op21) || !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || - !varTypeIsIntegralOrI(op22) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT)) + if (!op11->TypeIs(TYP_INT) || !op11->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op12->TypeIs(TYP_INT) || + !op12->OperIs(GT_LCL_VAR, GT_CNS_INT) || !op21->TypeIs(TYP_INT) || !op21->OperIs(GT_LCL_VAR, GT_CNS_INT) || + !op22->TypeIs(TYP_INT) || !op22->OperIs(GT_LCL_VAR, GT_CNS_INT)) { return false; } diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 9596dc54ccfa43..1ccc88e0ec033f 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -400,7 +400,6 @@ bool OptBoolsDsc::FindCompareChain(GenTree* condition, bool* isTestCondition) *isTestCondition = false; -#if defined(TARGET_ARM64) || defined(TARGET_XARCH) if (condition->OperIs(GT_EQ, GT_NE) && condOp2->IsIntegralConst()) { ssize_t condOp2Value = condOp2->AsIntCon()->IconValue(); @@ -430,7 +429,6 @@ bool OptBoolsDsc::FindCompareChain(GenTree* condition, bool* isTestCondition) *isTestCondition = true; } } -#endif return false; } From 3e8b5302095ecba61574757c764799be2b4b899e Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 1 Sep 2024 00:25:42 +0200 Subject: [PATCH 14/15] fix build errors 4 --- src/coreclr/jit/optimizebools.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 1ccc88e0ec033f..d5d7d33da18797 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1765,7 +1765,7 @@ bool OptBoolsDsc::optOptimizeCompareChainReturnBlock(BasicBlock* b3) return false; } - ssize_t block3RetVal = block3ReturnTree->AsIntCon()->IconValue(); + INT64 block3RetVal = block3ReturnTree->AsIntConCommon()->IntegralValue(); if (!cond1->OperIs(GT_NE) || !cond2->OperIs(GT_EQ, GT_NE) || (block3RetVal != 0 && cond2->OperIs(GT_EQ)) || (block3RetVal != 1 && cond2->OperIs(GT_NE))) From 6ab5360637943ff35c97a9f91471dbc41c11b1f6 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 1 Sep 2024 16:45:51 +0200 Subject: [PATCH 15/15] fix build errors 5 --- src/coreclr/jit/optimizebools.cpp | 10 +++++++--- src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index d5d7d33da18797..d8665df41c786f 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -402,7 +402,8 @@ bool OptBoolsDsc::FindCompareChain(GenTree* condition, bool* isTestCondition) if (condition->OperIs(GT_EQ, GT_NE) && condOp2->IsIntegralConst()) { - ssize_t condOp2Value = condOp2->AsIntCon()->IconValue(); + INT64 condOp2Value = + condOp2->gtOper == GT_CNS_INT ? condOp2->AsIntCon()->gtIconVal : condOp2->AsLngCon()->gtLconVal; if (condOp2Value == 0) { @@ -423,7 +424,10 @@ bool OptBoolsDsc::FindCompareChain(GenTree* condition, bool* isTestCondition) *isTestCondition = true; } else if (condOp1->OperIs(GT_AND) && isPow2(static_cast(condOp2Value)) && - condOp1->gtGetOp2()->IsIntegralConst(condOp2Value)) + ((condOp1->gtGetOp2()->gtOper == GT_CNS_INT && + condOp1->gtGetOp2()->AsIntCon()->gtIconVal == condOp2Value) || + (condOp1->gtGetOp2()->gtOper == GT_CNS_LNG && + condOp1->gtGetOp2()->AsLngCon()->gtLconVal == condOp2Value))) { // Found a EQ/NE(AND(...,n),n) which will be optimized to tbz/tbnz during lowering. *isTestCondition = true; @@ -1760,7 +1764,7 @@ bool OptBoolsDsc::optOptimizeCompareChainReturnBlock(BasicBlock* b3) GenTree* block3ReturnTree = m_b3->firstStmt()->GetRootNode()->AsOp()->GetReturnValue(); - if (block3ReturnTree == nullptr || !block3ReturnTree->OperIs(GT_CNS_INT) || !block3ReturnTree->TypeIs(TYP_INT)) + if (block3ReturnTree == nullptr || !block3ReturnTree->IsIntegralConst() || !block3ReturnTree->TypeIs(TYP_INT)) { return false; } diff --git a/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs b/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs index 4cbea34754c8af..93dd6a0991b89d 100644 --- a/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs +++ b/src/tests/JIT/opt/OptimizeBools/optboolsreturn.cs @@ -1119,4 +1119,4 @@ public static int TestEntryPoint() Console.WriteLine("PASSED"); return 100; } -} \ No newline at end of file +}