Tune CCMP for better Perf

src/coreclr/jit/compiler.h

@@ -7040,8 +7040,8 @@ class Compiler
 public:
     PhaseStatus optOptimizeBools();
     PhaseStatus optRecognizeAndOptimizeSwitchJumps();
-    bool optSwitchConvert(BasicBlock* firstBlock, int testsCount, ssize_t* testValues, weight_t falseLikelihood, GenTree* nodeToTest);
-    bool optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingForConversion = false);
+    bool optSwitchConvert(BasicBlock* firstBlock, int testsCount, ssize_t* testValues, weight_t falseLikelihood, GenTree* nodeToTest, bool testingForConversion = false, BitVec* ccmp_vec = nullptr);
+    bool optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingForConversion = false, BitVec* ccmp_vec = nullptr);
 
     PhaseStatus optInvertLoops();    // Invert loops so they're entered at top and tested at bottom.
     PhaseStatus optOptimizeFlow();   // Simplify flow graph and do tail duplication

src/coreclr/jit/lower.cpp

@@ -11589,6 +11589,39 @@ bool Lowering::TryLowerAndNegativeOne(GenTreeOp* node, GenTree** nextNode)
 }
 
 #if defined(TARGET_AMD64) || defined(TARGET_ARM64)
+//------------------------------------------------------------------------
+// CanConvertOpToCCMP : Checks whether operand can be converted to CCMP
+//
+// Arguments:
+//    operand - operand to check for CCMP conversion
+//    tree    - parent of the operand
+//
+// Return Value:
+//    true if operand can be converted to CCMP
+//
+bool Lowering::CanConvertOpToCCMP(GenTree* operand, GenTree* tree)
+{
+    return operand->OperIsCmpCompare() && varTypeIsIntegralOrI(operand->gtGetOp1()) &&
+           IsInvariantInRange(operand, tree);
+}
+
+//------------------------------------------------------------------------
+// IsOpPreferredForCCMP : Checks if operand is preferred for conversion to CCMP
+//
+// Arguments:
+//    operand - operand to check for CCMP conversion
+//
+// Return Value:
+//    true if operand is preferred for CCMP
+//
+bool Lowering::IsOpPreferredForCCMP(GenTree* operand)
+{
+    assert(operand->OperIsCmpCompare());
+    return (operand->gtGetOp1()->IsIntegralConst() || !operand->gtGetOp1()->isContained()) &&
+           (operand->gtGetOp2() == nullptr || operand->gtGetOp2()->IsIntegralConst() ||
+            !operand->gtGetOp2()->isContained());
+}
+
 //------------------------------------------------------------------------
 // TryLowerAndOrToCCMP : Lower AND/OR of two conditions into test + CCMP + SETCC nodes.
 //
@@ -11618,6 +11651,10 @@ bool Lowering::TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next)
         DISPTREERANGE(BlockRange(), tree);
         JITDUMP("\n");
     }
+    else
+    {
+        return false;
+    }
 
     // Find out whether an operand is eligible to be converted to a conditional
     // compare. It must be a normal integral relop; for example, we cannot
@@ -11630,17 +11667,15 @@ bool Lowering::TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next)
     // by TryLowerConditionToFlagsNode.
     //
     GenCondition cond1;
-    if (op2->OperIsCmpCompare() && varTypeIsIntegralOrI(op2->gtGetOp1()) && IsInvariantInRange(op2, tree) &&
-        (op2->gtGetOp1()->IsIntegralConst() || !op2->gtGetOp1()->isContained()) &&
-        (op2->gtGetOp2() == nullptr || op2->gtGetOp2()->IsIntegralConst() || !op2->gtGetOp2()->isContained()) &&
+    bool         canConvertOp2ToCCMP = CanConvertOpToCCMP(op2, tree);
+    bool         canConvertOp1ToCCMP = CanConvertOpToCCMP(op1, tree);
+
+    if (canConvertOp2ToCCMP && (!canConvertOp1ToCCMP || IsOpPreferredForCCMP(op2)) &&
         TryLowerConditionToFlagsNode(tree, op1, &cond1, false))
     {
         // Fall through, converting op2 to the CCMP
     }
-    else if (op1->OperIsCmpCompare() && varTypeIsIntegralOrI(op1->gtGetOp1()) && IsInvariantInRange(op1, tree) &&
-             (op1->gtGetOp1()->IsIntegralConst() || !op1->gtGetOp1()->isContained()) &&
-             (op1->gtGetOp2() == nullptr || op1->gtGetOp2()->IsIntegralConst() || !op1->gtGetOp2()->isContained()) &&
-             TryLowerConditionToFlagsNode(tree, op2, &cond1, false))
+    else if (canConvertOp1ToCCMP && TryLowerConditionToFlagsNode(tree, op2, &cond1, false))
     {
         std::swap(op1, op2);
     }

src/coreclr/jit/lower.h

@@ -84,6 +84,8 @@ class Lowering final : public Phase
     void ContainCheckLclHeap(GenTreeOp* node);
     void ContainCheckRet(GenTreeUnOp* ret);
 #if defined(TARGET_ARM64) || defined(TARGET_AMD64)
+    bool      IsOpPreferredForCCMP(GenTree* operand);
+    bool      CanConvertOpToCCMP(GenTree* operand, GenTree* tree);
     bool      TryLowerAndOrToCCMP(GenTreeOp* tree, GenTree** next);
     insCflags TruthifyingFlags(GenCondition cond);
     void      ContainCheckConditionalCompare(GenTreeCCMP* ccmp);

src/coreclr/jit/optimizebools.cpp

@@ -1581,6 +1581,8 @@ PhaseStatus Compiler::optOptimizeBools()
     unsigned numCond   = 0;
     unsigned numPasses = 0;
     unsigned stress    = false;
+    BitVecTraits* ccmp_traits = new BitVecTraits(fgBBNumMax + 1, this);
+    BitVec ccmp_vec = BitVecOps::MakeEmpty(ccmp_traits);
 
     do
     {
@@ -1656,7 +1658,7 @@ PhaseStatus Compiler::optOptimizeBools()
                 // trigger or not
                 // else if ((compOpportunisticallyDependsOn(InstructionSet_APX) || JitConfig.JitEnableApxIfConv()) &&
                 // optBoolsDsc.optOptimizeCompareChainCondBlock())
-                else if (JitConfig.EnableApxConditionalChaining() && !optSwitchDetectAndConvert(b1, true) &&
+                else if (JitConfig.EnableApxConditionalChaining() && !optSwitchDetectAndConvert(b1, true, &ccmp_vec) &&
                          optBoolsDsc.optOptimizeCompareChainCondBlock())
                 {
                     // The optimization will have merged b1 and b2. Retry the loop so that

src/coreclr/jit/switchrecognition.cpp

@@ -11,6 +11,9 @@
 #define SWITCH_MAX_DISTANCE ((TARGET_POINTER_SIZE * BITS_PER_BYTE) - 1)
 #define SWITCH_MIN_TESTS    3
 
+// This is a heuristics based value tuned for optimal performance
+#define CONVERT_SWITCH_TO_CCMP_MIN_TEST 5
+
 //-----------------------------------------------------------------------------
 //  optRecognizeAndOptimizeSwitchJumps: Optimize range check for `x == cns1 || x == cns2 || x == cns3 ...`
 //      pattern and convert it to a BBJ_SWITCH block (jump table), which then *might* be converted
@@ -164,14 +167,17 @@ bool IsConstantTestCondBlock(const BasicBlock* block,
 // Return Value:
 //    True if the conversion was successful, false otherwise
 //
-bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingForConversion)
+bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingForConversion, BitVec* ccmp_vec)
 {
     assert(firstBlock->KindIs(BBJ_COND));
 
     GenTree*    variableNode = nullptr;
     ssize_t     cns          = 0;
     BasicBlock* trueTarget   = nullptr;
     BasicBlock* falseTarget  = nullptr;
+    int     testValueIndex                  = 0;
+    ssize_t testValues[SWITCH_MAX_DISTANCE] = {};
+    weight_t          falseLikelihood       = firstBlock->GetFalseEdge()->getLikelihood();
 
     // The algorithm is simple - we check that the given block is a constant test block
     // and then try to accumulate as many constant test blocks as possible. Once we hit
@@ -186,16 +192,30 @@ bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingFor
             // TODO: make it more flexible and support cases like "x != cns1 && x != cns2 && ..."
             return false;
         }
+        if (testingForConversion)
+        {
+            assert(ccmp_vec != nullptr);
+            BitVecTraits* ccmp_traits = new BitVecTraits(fgBBNumMax + 1, this);
+            if (BitVecOps::IsMember(ccmp_traits, *ccmp_vec, firstBlock->bbNum))
+            {
+                BitVecOps::RemoveElemD(ccmp_traits, *ccmp_vec, firstBlock->bbNum);
+                return true;
+            }
+            else
+            {
+                *ccmp_vec = BitVecOps::MakeEmpty(ccmp_traits);
+            }
+        }
+
 
         // No more than SWITCH_MAX_TABLE_SIZE blocks are allowed (arbitrary limit in this context)
-        int     testValueIndex                  = 0;
-        ssize_t testValues[SWITCH_MAX_DISTANCE] = {};
+        testValueIndex                          = 0;
         testValues[testValueIndex]              = cns;
         testValueIndex++;
 
         // Track likelihood of reaching the false block
         //
-        weight_t          falseLikelihood = firstBlock->GetFalseEdge()->getLikelihood();
+        falseLikelihood = firstBlock->GetFalseEdge()->getLikelihood();
         const BasicBlock* prevBlock       = firstBlock;
 
         // Now walk the chain of test blocks, and see if they are basically the same type of test
@@ -209,8 +229,7 @@ bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingFor
             {
                 // Only the first conditional block can have multiple statements.
                 // Stop searching and process what we already have.
-                return !testingForConversion &&
-                       optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                goto optSwitchConvert;
             }
 
             // Inspect secondary blocks
@@ -220,29 +239,25 @@ bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingFor
                 if (currTrueTarget != trueTarget)
                 {
                     // This blocks jumps to a different target, stop searching and process what we already have.
-                    return !testingForConversion &&
-                           optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                    goto optSwitchConvert;
                 }
 
                 if (!GenTree::Compare(currVariableNode, variableNode->gtEffectiveVal()))
                 {
                     // A different variable node is used, stop searching and process what we already have.
-                    return !testingForConversion &&
-                           optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                    goto optSwitchConvert;
                 }
 
                 if (currBb->GetUniquePred(this) != prevBlock)
                 {
                     // Multiple preds in a secondary block, stop searching and process what we already have.
-                    return !testingForConversion &&
-                           optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                    goto optSwitchConvert;
                 }
 
                 if (!BasicBlock::sameEHRegion(prevBlock, currBb))
                 {
                     // Current block is in a different EH region, stop searching and process what we already have.
-                    return !testingForConversion &&
-                           optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                    goto optSwitchConvert;
                 }
 
                 // Ok we can work with that, add the test value to the list
@@ -252,32 +267,29 @@ bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingFor
                 if (testValueIndex == SWITCH_MAX_DISTANCE)
                 {
                     // Too many suitable tests found - stop and process what we already have.
-                    return !testingForConversion &&
-                           optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                    goto optSwitchConvert;
                 }
 
                 if (isReversed)
                 {
                     // We only support reversed test (GT_NE) for the last block.
-                    return !testingForConversion &&
-                           optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                    goto optSwitchConvert;
                 }
-
-                if (testingForConversion)
-                    return true;
-
                 prevBlock = currBb;
             }
             else
             {
                 // Current block is not a suitable test, stop searching and process what we already have.
-                return !testingForConversion &&
-                       optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode);
+                goto optSwitchConvert;
             }
         }
     }
 
     return false;
+
+optSwitchConvert:
+    return optSwitchConvert(firstBlock, testValueIndex, testValues, falseLikelihood, variableNode,
+                            testingForConversion);
 }
 
 //------------------------------------------------------------------------------
@@ -296,12 +308,22 @@ bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock, bool testingFor
 // Return Value:
 //    True if the conversion was successful, false otherwise
 //
-bool Compiler::optSwitchConvert(
-    BasicBlock* firstBlock, int testsCount, ssize_t* testValues, weight_t falseLikelihood, GenTree* nodeToTest)
+bool Compiler::optSwitchConvert(BasicBlock* firstBlock,
+                                int         testsCount,
+                                ssize_t*    testValues,
+                                weight_t    falseLikelihood,
+                                GenTree*    nodeToTest,
+                                bool        testingForConversion,
+                                BitVec*     ccmp_vec)
 {
     assert(firstBlock->KindIs(BBJ_COND));
     assert(!varTypeIsSmall(nodeToTest));
 
+    if (testingForConversion && (testsCount < CONVERT_SWITCH_TO_CCMP_MIN_TEST))
+    {
+        return false;
+    }
+
     if (testsCount < SWITCH_MIN_TESTS)
     {
         // Early out - short chains.
@@ -376,6 +398,20 @@ bool Compiler::optSwitchConvert(
     FlowEdge* const trueEdge  = firstBlock->GetTrueEdge();
     FlowEdge* const falseEdge = firstBlock->GetFalseEdge();
 
+    if (testingForConversion)
+    {
+        assert(ccmp_vec != nullptr);
+        BitVecTraits* ccmp_traits = new BitVecTraits(fgBBNumMax + 1, this);
+        // Return if we are just checking for a possibility of a switch convert and not actually making the conversion
+        // to switch here.
+        BasicBlock* iterBlock = firstBlock;
+        for (int i = 0; i < testsCount; i++)
+        {
+            BitVecOps::AddElemD(ccmp_traits, *ccmp_vec, iterBlock->bbNum);
+            iterBlock = iterBlock->GetFalseTarget();
+        }
+        return true;
+    }
     // Convert firstBlock to a switch block
     firstBlock->SetSwitch(new (this, CMK_BasicBlock) BBswtDesc);
     firstBlock->bbCodeOffsEnd = lastBlock->bbCodeOffsEnd;