Use cinc instead of csel when possible (#82031)

* Use cinc instead of csel when possible * Add a separate node for cinc * Remove rebase leftover * Make GT_CINC as a BINOP * Remove unnecessary assertion * Consume register holding cinc operand * Fix cleanup of cinc nodes and incorporate review comments * Revert building RefPositions for cinc node * Clean GTF flags from the prev node correctly
dotnet · Apr 12, 2023 · 851eb3d · 851eb3d
1 parent 314362e
commit 851eb3d
Show file tree

Hide file tree

Showing 16 changed files with 378 additions and 14 deletions.
diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h
@@ -895,6 +895,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     void genCodeForCompare(GenTreeOp* tree);
 #ifdef TARGET_ARM64
     void genCodeForCCMP(GenTreeCCMP* ccmp);
+    void genCodeForCinc(GenTreeOp* cinc);
 #endif
     void genCodeForSelect(GenTreeOp* select);
     void genIntrinsic(GenTreeIntrinsic* treeNode);

diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
@@ -4680,6 +4680,66 @@ void CodeGen::genCodeForSelect(GenTreeOp* tree)
     genProduceReg(tree);
 }
 
+//------------------------------------------------------------------------
+// genCodeForCinc: Produce code for a GT_CINC/GT_CINCCC node.
+//
+// Arguments:
+//    tree - the node
+//
+void CodeGen::genCodeForCinc(GenTreeOp* cinc)
+{
+    assert(cinc->OperIs(GT_CINC, GT_CINCCC));
+
+    GenTree* opcond = nullptr;
+    GenTree* op     = cinc->gtOp1;
+    if (cinc->OperIs(GT_CINC))
+    {
+        opcond = cinc->gtOp1;
+        op     = cinc->gtOp2;
+        genConsumeRegs(opcond);
+    }
+
+    emitter*  emit   = GetEmitter();
+    var_types opType = genActualType(op->TypeGet());
+    emitAttr  attr   = emitActualTypeSize(cinc->TypeGet());
+
+    assert(!op->isUsedFromMemory());
+    genConsumeRegs(op);
+
+    GenCondition cond;
+
+    if (cinc->OperIs(GT_CINC))
+    {
+        assert(!opcond->isContained());
+        // Condition has been generated into a register - move it into flags.
+        emit->emitIns_R_I(INS_cmp, emitActualTypeSize(opcond), opcond->GetRegNum(), 0);
+        cond = GenCondition::NE;
+    }
+    else
+    {
+        assert(cinc->OperIs(GT_CINCCC));
+        cond = cinc->AsOpCC()->gtCondition;
+    }
+    const GenConditionDesc& prevDesc  = GenConditionDesc::Get(cond);
+    regNumber               targetReg = cinc->GetRegNum();
+    regNumber               srcReg;
+
+    if (op->isContained())
+    {
+        assert(op->IsIntegralConst(0));
+        srcReg = REG_ZR;
+    }
+    else
+    {
+        srcReg = op->GetRegNum();
+    }
+
+    assert(prevDesc.oper != GT_OR && prevDesc.oper != GT_AND);
+    emit->emitIns_R_R_COND(INS_cinc, attr, targetReg, srcReg, JumpKindToInsCond(prevDesc.jumpKind1));
+    regSet.verifyRegUsed(targetReg);
+    genProduceReg(cinc);
+}
+
 //------------------------------------------------------------------------
 // genCodeForJumpCompare: Generates code for jmpCompare statement.
 //

diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp
@@ -359,12 +359,15 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
             genCodeForSelect(treeNode->AsConditional());
             break;
 
+        case GT_CINC:
+        case GT_CINCCC:
+            genCodeForCinc(treeNode->AsOp());
+            break;
+
         case GT_SELECTCC:
             genCodeForSelect(treeNode->AsOp());
             break;
-#endif
 
-#ifdef TARGET_ARM64
         case GT_JCMP:
             genCodeForJumpCompare(treeNode->AsOp());
             break;

diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp
@@ -4154,6 +4154,7 @@ void GenTree::VisitOperands(TVisitor visitor)
 // Standard unary operators
 #ifdef TARGET_ARM64
         case GT_CNEG_LT:
+        case GT_CINCCC:
 #endif // TARGET_ARM64
         case GT_STORE_LCL_VAR:
         case GT_STORE_LCL_FLD:

diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp
@@ -444,7 +444,7 @@ void emitter::emitInsSanityCheck(instrDesc* id)
         case IF_DR_2D: // DR_2D   X..........nnnnn cccc..nnnnnmmmmm      Rd Rn    cond
             assert(isValidGeneralDatasize(id->idOpSize()));
             assert(isGeneralRegister(id->idReg1()));
-            assert(isGeneralRegister(id->idReg2()));
+            assert(isGeneralRegisterOrZR(id->idReg2()));
             assert(isValidImmCond(emitGetInsSC(id)));
             break;
 
@@ -7315,7 +7315,7 @@ void emitter::emitIns_R_R_COND(instruction ins, emitAttr attr, regNumber reg1, r
         case INS_cinv:
         case INS_cneg:
             assert(isGeneralRegister(reg1));
-            assert(isGeneralRegister(reg2));
+            assert(isGeneralRegisterOrZR(reg2));
             cfi.cond = cond;
             fmt      = IF_DR_2D;
             break;

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
@@ -6257,6 +6257,7 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse)
 // Standard unary operators
 #ifdef TARGET_ARM64
         case GT_CNEG_LT:
+        case GT_CINCCC:
 #endif // TARGET_ARM64
         case GT_STORE_LCL_VAR:
         case GT_STORE_LCL_FLD:
@@ -9507,6 +9508,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
 // Standard unary operators
 #ifdef TARGET_ARM64
         case GT_CNEG_LT:
+        case GT_CINCCC:
 #endif // TARGET_ARM64
         case GT_STORE_LCL_VAR:
         case GT_STORE_LCL_FLD:
@@ -12098,6 +12100,10 @@ void Compiler::gtDispTree(GenTree*     tree,
             printf(" cond=%s", tree->AsOpCC()->gtCondition.Name());
         }
 #ifdef TARGET_ARM64
+        else if (tree->OperIs(GT_CINCCC))
+        {
+            printf(" cond=%s", tree->AsOpCC()->gtCondition.Name());
+        }
         else if (tree->OperIs(GT_CCMP))
         {
             printf(" cond=%s flags=%s", tree->AsCCMP()->gtCondition.Name(),

diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
@@ -1691,7 +1691,7 @@ struct GenTree
         }
 #endif
 #if defined(TARGET_ARM64)
-        if (OperIs(GT_CCMP))
+        if (OperIs(GT_CCMP, GT_CINCCC))
         {
             return true;
         }

diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h
@@ -245,9 +245,13 @@ GTNODE(SELECTCC         , GenTreeOpCC        ,0,GTK_BINOP|DBK_NOTHIR)
 // operands and sets the condition flags according to the result. Otherwise
 // sets the condition flags to the specified immediate value.
 GTNODE(CCMP             , GenTreeCCMP        ,0,GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR)
+// Maps to arm64 cinc instruction. It returns the operand incremented by one when the condition is true.
+// Otherwise returns the unchanged operand. Optimises for patterns such as, result = condition ? op1 + 1 : op1
+GTNODE(CINC             , GenTreeOp          ,0,GTK_BINOP|DBK_NOTHIR)
+// Variant of CINC that reuses flags computed by a previous node with the specified condition.
+GTNODE(CINCCC           , GenTreeOpCC        ,0,GTK_UNOP|DBK_NOTHIR)
 #endif
 
-
 //-----------------------------------------------------------------------------
 //  Other nodes that look like unary/binary operators:
 //-----------------------------------------------------------------------------

diff --git a/src/coreclr/jit/gtstructs.h b/src/coreclr/jit/gtstructs.h
@@ -111,9 +111,11 @@ GTSTRUCT_1(AllocObj    , GT_ALLOCOBJ)
 GTSTRUCT_1(RuntimeLookup, GT_RUNTIMELOOKUP)
 GTSTRUCT_1(ArrAddr     , GT_ARR_ADDR)
 GTSTRUCT_2(CC          , GT_JCC, GT_SETCC)
-GTSTRUCT_1(OpCC        , GT_SELECTCC)
 #ifdef TARGET_ARM64
 GTSTRUCT_1(CCMP        , GT_CCMP)
+GTSTRUCT_2(OpCC        , GT_SELECTCC, GT_CINCCC)
+#else
+GTSTRUCT_1(OpCC        , GT_SELECTCC)
 #endif
 #if defined(TARGET_X86)
 GTSTRUCT_1(MultiRegOp  , GT_MUL_LONG)

diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp
@@ -2219,9 +2219,8 @@ bool Compiler::fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange)
                 return GenTree::VisitResult::Continue;
             });
 
-            if (node->OperIs(GT_SELECTCC, GT_SETCC))
+            if (node->OperConsumesFlags() && node->gtPrev->gtSetFlags())
             {
-                assert((node->gtPrev->gtFlags & GTF_SET_FLAGS) != 0);
                 node->gtPrev->gtFlags &= ~GTF_SET_FLAGS;
             }
 

diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
@@ -3830,20 +3830,29 @@ GenTree* Lowering::LowerSelect(GenTreeConditional* select)
     // we could do this on x86. We currently disable if-conversion for TYP_LONG
     // on 32-bit architectures because of this.
     GenCondition selectCond;
+    GenTreeOpCC* newSelect = nullptr;
     if (((select->gtFlags & GTF_SET_FLAGS) == 0) && TryLowerConditionToFlagsNode(select, cond, &selectCond))
     {
         select->SetOper(GT_SELECTCC);
-        GenTreeOpCC* newSelect = select->AsOpCC();
+        newSelect              = select->AsOpCC();
         newSelect->gtCondition = selectCond;
         ContainCheckSelect(newSelect);
         JITDUMP("Converted to SELECTCC:\n");
         DISPTREERANGE(BlockRange(), newSelect);
         JITDUMP("\n");
-        return newSelect->gtNext;
+    }
+    else
+    {
+        ContainCheckSelect(select);
     }
 
-    ContainCheckSelect(select);
-    return select->gtNext;
+#ifdef TARGET_ARM64
+    if (trueVal->IsCnsIntOrI() && falseVal->IsCnsIntOrI())
+    {
+        TryLowerCselToCinc(select, cond);
+    }
+#endif
+    return newSelect != nullptr ? newSelect->gtNext : select->gtNext;
 }
 
 //----------------------------------------------------------------------------------------------

diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h
@@ -89,6 +89,7 @@ class Lowering final : public Phase
     insCflags TruthifyingFlags(GenCondition cond);
     void ContainCheckConditionalCompare(GenTreeCCMP* ccmp);
     void ContainCheckNeg(GenTreeOp* neg);
+    void TryLowerCselToCinc(GenTreeOp* select, GenTree* cond);
 #endif
     void ContainCheckSelect(GenTreeOp* select);
     void ContainCheckBitCast(GenTree* node);

diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp
@@ -2495,6 +2495,67 @@ void Lowering::ContainCheckNeg(GenTreeOp* neg)
         }
     }
 }
+
+//----------------------------------------------------------------------------------------------
+// Try converting SELECT/SELECTCC to CINC/CINCCC. Conversion is possible only if
+// both the trueVal and falseVal are integral constants and abs(trueVal - falseVal) = 1.
+//
+// Arguments:
+//     select - The select node that is now SELECT or SELECTCC
+//     cond   - The condition node that SELECT or SELECTCC uses
+//
+void Lowering::TryLowerCselToCinc(GenTreeOp* select, GenTree* cond)
+{
+    assert(select->OperIs(GT_SELECT, GT_SELECTCC));
+
+    GenTree* trueVal  = select->gtOp1;
+    GenTree* falseVal = select->gtOp2;
+    size_t   op1Val   = (size_t)trueVal->AsIntCon()->IconValue();
+    size_t   op2Val   = (size_t)falseVal->AsIntCon()->IconValue();
+
+    if (op1Val + 1 == op2Val || op2Val + 1 == op1Val)
+    {
+        const bool shouldReverseCondition = op1Val + 1 == op2Val;
+
+        // Create a cinc node, insert it and update the use.
+        if (select->OperIs(GT_SELECT))
+        {
+            if (shouldReverseCondition)
+            {
+                // Reverse the condition so that op2 will be selected
+                if (!cond->OperIsCompare())
+                {
+                    // Non-compare nodes add additional GT_NOT node after reversing.
+                    // This would remove gains from this optimisation so don't proceed.
+                    return;
+                }
+                select->gtOp2    = select->gtOp1;
+                GenTree* revCond = comp->gtReverseCond(cond);
+                assert(cond == revCond); // Ensure `gtReverseCond` did not create a new node.
+            }
+            select->gtOp1 = cond->AsOp();
+            select->SetOper(GT_CINC);
+            DISPTREERANGE(BlockRange(), select);
+        }
+        else
+        {
+            GenTreeOpCC* selectcc   = select->AsOpCC();
+            GenCondition selectCond = selectcc->gtCondition;
+            if (shouldReverseCondition)
+            {
+                // Reverse the condition so that op2 will be selected
+                selectcc->gtCondition = GenCondition::Reverse(selectCond);
+            }
+            else
+            {
+                std::swap(selectcc->gtOp1, selectcc->gtOp2);
+            }
+            BlockRange().Remove(selectcc->gtOp2);
+            selectcc->SetOper(GT_CINCCC);
+            DISPTREERANGE(BlockRange(), selectcc);
+        }
+    }
+}
 #endif // TARGET_ARM64
 
 //------------------------------------------------------------------------

diff --git a/src/tests/JIT/opt/Compares/compareAnd3Chains.cs b/src/tests/JIT/opt/Compares/compareAnd3Chains.cs
@@ -219,7 +219,7 @@ public static void Eq_else_long_3_consume(long a1, long a2, long a3) {
         //ARM64-FULL-LINE-NEXT: ccmp {{x[0-9]+}}, #21, 0, {{eq|ne}}
         //ARM64-FULL-LINE-NEXT: ccmp {{x[0-9]+}}, #19, z, {{eq|ne}}
         //ARM64-FULL-LINE-NEXT: csel {{x[0-9]+}}, {{x[0-9]+}}, {{x[0-9]+}}, {{eq|ne}}
-        if (a1 == 19 || a2 == 20 && a3 == 21) { a1 = 10; } else { a1 = 11; }
+        if (a1 == 19 || a2 == 20 && a3 == 21) { a1 = 10; } else { a1 = 12; }
         consume<long>(a1, a2, a3);
     }