[RISC-V] Move comparison transformations from codegen to lowering

src/coreclr/jit/emitriscv64.cpp

@@ -730,7 +730,7 @@ void emitter::emitIns_R_R_I(
         (INS_lb <= ins && INS_lhu >= ins) || INS_ld == ins || INS_lw == ins || INS_jalr == ins || INS_fld == ins ||
         INS_flw == ins || INS_slli_uw == ins || INS_rori == ins || INS_roriw == ins)
     {
-        assert(isGeneralRegister(reg2));
+        assert(isGeneralRegisterOrR0(reg2));
         code |= (reg1 & 0x1f) << 7; // rd
         code |= reg2 << 15;         // rs1
         code |= imm << 20;          // imm
@@ -5121,15 +5121,6 @@ regNumber emitter::emitInsTernary(instruction ins, emitAttr attr, GenTree* dst,
                     // n * n bytes will store n bytes result
                     emitIns_R_R_R(ins, attr, dstReg, src1Reg, src2Reg);
 
-                    if ((dst->gtFlags & GTF_UNSIGNED) != 0)
-                    {
-                        if (attr == EA_4BYTE)
-                        {
-                            emitIns_R_R_I(INS_slli, EA_8BYTE, dstReg, dstReg, 32);
-                            emitIns_R_R_I(INS_srli, EA_8BYTE, dstReg, dstReg, 32);
-                        }
-                    }
-
                     if (needCheckOv)
                     {
                         assert(tempReg != dstReg);

src/coreclr/jit/lower.cpp

@@ -4366,6 +4366,10 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp)
             {
                 GenTree* reversed = comp->gtReverseCond(op1);
                 assert(reversed == op1);
+#ifdef TARGET_RISCV64
+                LowerNode(op1);
+                op1 = cmp->gtGetOp1();
+#endif
             }
 
             // Relops and SETCC can be either TYP_INT or TYP_LONG typed, so we
@@ -4448,7 +4452,26 @@ GenTree* Lowering::LowerCompare(GenTree* cmp)
             cmp->gtFlags |= GTF_UNSIGNED;
         }
     }
-#endif // TARGET_XARCH
+#elif defined(TARGET_RISCV64)
+    // Branches will be lowered in LowerJTrue
+    LIR::Use cmpUse;
+    if (!BlockRange().TryGetUse(cmp, &cmpUse) || cmpUse.User()->OperIs(GT_JTRUE))
+        return cmp->gtNext;
+
+    bool isReversed =
+        varTypeIsFloating(cmp->gtGetOp1()) ? LowerAndReverseFloatingCompare(cmp) : LowerAndReverseIntegerCompare(cmp);
+    if (isReversed)
+    {
+        GenTree* one   = comp->gtNewIconNode(1);
+        GenTree* notOp = comp->gtNewOperNode(GT_XOR, cmp->gtType, cmp, one);
+        BlockRange().InsertAfter(cmp, one, notOp);
+        one->SetContained();
+        cmpUse.ReplaceWith(notOp);
+    }
+    if (!cmp->OperIsCmpCompare()) // comparison was optimized out to some other node
+        return cmp->gtNext;
+#endif // TARGET_RISCV64
+
     ContainCheckCompare(cmp->AsOp());
     return cmp->gtNext;
 }
@@ -7714,7 +7737,7 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod)
         {
             divMod->ChangeOper(GT_GE);
             divMod->gtFlags |= GTF_UNSIGNED;
-            ContainCheckNode(divMod);
+            LowerNode(divMod);
             return true;
         }
     }

src/coreclr/jit/lower.h

@@ -156,9 +156,14 @@ class Lowering final : public Phase
 #ifndef TARGET_64BIT
     GenTree* DecomposeLongCompare(GenTree* cmp);
 #endif
-    GenTree*   OptimizeConstCompare(GenTree* cmp);
-    GenTree*   LowerCompare(GenTree* cmp);
-    GenTree*   LowerJTrue(GenTreeOp* jtrue);
+    GenTree* OptimizeConstCompare(GenTree* cmp);
+    GenTree* LowerCompare(GenTree* cmp);
+    GenTree* LowerJTrue(GenTreeOp* jtrue);
+#ifdef TARGET_RISCV64
+    bool LowerAndReverseFloatingCompare(GenTree* cmp);
+    bool LowerAndReverseIntegerCompare(GenTree* cmp);
+    void SignExtendIfNecessary(GenTree** arg);
+#endif
     GenTree*   LowerSelect(GenTreeConditional* cond);
     bool       TryLowerConditionToFlagsNode(GenTree*      parent,
                                             GenTree*      condition,

src/coreclr/jit/lowerriscv64.cpp

@@ -64,24 +64,14 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const
 
         switch (parentNode->OperGet())
         {
-            case GT_EQ:
-            case GT_NE:
-                return emitter::isValidSimm12(-immVal) || (immVal == -2048);
-
-            case GT_LE: // a <= N  ->  a < N+1
-            case GT_GT: // a > N  ->  !(a <= N)  ->  !(a < N+1)
-                immVal += 1;
-                FALLTHROUGH;
             case GT_LT:
-            case GT_GE:
             case GT_ADD:
             case GT_AND:
             case GT_OR:
             case GT_XOR:
                 return emitter::isValidSimm12(immVal);
-            case GT_JCMP:
-                return true;
 
+            case GT_JCMP:
             case GT_CMPXCHG:
             case GT_XORR:
             case GT_XAND:
@@ -133,57 +123,218 @@ GenTree* Lowering::LowerMul(GenTreeOp* mul)
 //
 GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue)
 {
-    GenTree*     op = jtrue->gtGetOp1();
-    GenCondition cond;
-    GenTree*     cmpOp1;
-    GenTree*     cmpOp2;
-
-    assert(!op->OperIsCompare() || op->OperIsCmpCompare()); // We do not expect any other relops on RISCV64
+    GenTree* cmp = jtrue->gtGetOp1();
+    assert(!cmp->OperIsCompare() || cmp->OperIsCmpCompare()); // We do not expect any other relops on RISCV64
 
-    if (op->OperIsCompare() && !varTypeIsFloating(op->gtGetOp1()))
+    // for RISCV64's compare and condition-branch instructions, it's very similar to the IL instructions.
+    jtrue->ChangeOper(GT_JCMP);
+    GenTreeOpCC* jcmp = jtrue->AsOpCC();
+    if (cmp->OperIsCompare() && !varTypeIsFloating(cmp->gtGetOp1()))
     {
-        cond = GenCondition::FromRelop(op);
-
-        cmpOp1 = op->gtGetOp1();
-        cmpOp2 = op->gtGetOp2();
-
-        // We will fall through and turn this into a JCMP(op1, op2, kind), but need to remove the relop here.
-        BlockRange().Remove(op);
+        jcmp->gtCondition = GenCondition::FromIntegralRelop(cmp);
+        jcmp->gtOp1       = cmp->gtGetOp1();
+        jcmp->gtOp2       = cmp->gtGetOp2();
+        if (cmp->OperIs(GT_GT, GT_LE))
+        {
+            // ">" and "<=" are achieved by swapping inputs for "<" and ">="
+            jcmp->gtCondition = GenCondition::Swap(jcmp->gtCondition);
+            std::swap(jcmp->gtOp1, jcmp->gtOp2);
+        }
+        BlockRange().Remove(cmp);
     }
     else
     {
+        // branch if (cond)  --->  branch if (cond != 0)
         GenCondition::Code code = GenCondition::NE;
-        if (op->OperIsCompare() && varTypeIsFloating(op->gtGetOp1()) && (op->gtFlags & GTF_RELOP_NAN_UN) != 0)
+        if (cmp->OperIsCompare() && varTypeIsFloating(cmp->gtGetOp1()))
         {
-            // Unordered floating-point comparisons are achieved by neg'ing the ordered counterparts. Avoid that by
-            // reversing both the FP comparison and the zero-comparison fused with the branch.
-            op->ChangeOper(GenTree::ReverseRelop(op->OperGet()));
-            op->gtFlags &= ~GTF_RELOP_NAN_UN;
-            code = GenCondition::EQ;
+            if (LowerAndReverseFloatingCompare(cmp))
+                code = GenCondition::EQ;
         }
-        cond = GenCondition(code);
+        jcmp->gtCondition = GenCondition(code);
+        jcmp->gtOp1       = cmp;
+        jcmp->gtOp2       = comp->gtNewZeroConNode(cmp->TypeGet());
+        BlockRange().InsertBefore(jcmp, jcmp->gtOp2);
+    }
 
-        cmpOp1 = op;
-        cmpOp2 = comp->gtNewZeroConNode(cmpOp1->TypeGet());
+    for (GenTree** op : {&jcmp->gtOp1, &jcmp->gtOp2})
+    {
+        SignExtendIfNecessary(op);
+        CheckImmedAndMakeContained(jcmp, *op);
+    }
 
-        BlockRange().InsertBefore(jtrue, cmpOp2);
+    return jcmp->gtNext;
+}
 
-        // Fall through and turn this into a JCMP(op1, 0, NE).
+//------------------------------------------------------------------------
+// LowerAndReverseFloatingCompare: lowers and reverses a floating-point comparison so that it matches the
+// available instructions: "<", "<=", "==" (ordered only, register only)
+//
+// Arguments:
+//    cmp - the floating-point comparison to lower
+//
+// Returns:
+//    Whether the comparison was reversed.
+//
+bool Lowering::LowerAndReverseFloatingCompare(GenTree* cmp)
+{
+    assert(cmp->OperIsCmpCompare() && varTypeIsFloating(cmp->gtGetOp1()));
+    bool isUnordered = (cmp->gtFlags & GTF_RELOP_NAN_UN) != 0;
+    if (isUnordered)
+    {
+        // a CMP b (unordered)  --->   !(a reversedCMP b (ordered))
+        cmp->SetOperRaw(GenTree::ReverseRelop(cmp->OperGet()));
+        cmp->gtFlags &= ~GTF_RELOP_NAN_UN;
     }
+    if (cmp->OperIs(GT_GT, GT_GE))
+    {
+        cmp->SetOperRaw(GenTree::SwapRelop(cmp->OperGet()));
+        std::swap(cmp->AsOp()->gtOp1, cmp->AsOp()->gtOp2);
+    }
+    assert(cmp->OperIs(GT_EQ, GT_LT, GT_LE));
+    return isUnordered;
+}
 
-    // for RISCV64's compare and condition-branch instructions,
-    // it's very similar to the IL instructions.
-    jtrue->ChangeOper(GT_JCMP);
-    jtrue->gtOp1                 = cmpOp1;
-    jtrue->gtOp2                 = cmpOp2;
-    jtrue->AsOpCC()->gtCondition = cond;
+//------------------------------------------------------------------------
+// LowerAndReverseIntegerCompare: lowers and reverses an integer comparison so that it matches the available
+// instructions: only "<" (with unsigned and immediate variants)
+//
+// Arguments:
+//    cmp - the integer comparison to lower
+//
+// Returns:
+//    Whether the comparison was reversed.
+//
+bool Lowering::LowerAndReverseIntegerCompare(GenTree* cmp)
+{
+    GenTree*& left  = cmp->AsOp()->gtOp1;
+    GenTree*& right = cmp->AsOp()->gtOp2;
+    assert(cmp->OperIsCmpCompare() && varTypeUsesIntReg(left));
+    bool isReversed = false;
 
-    if (cmpOp2->IsCnsIntOrI())
+    if (left->IsIntegralConst() && !right->IsIntegralConst())
     {
-        cmpOp2->SetContained();
+        // Normally const operand is ordered on the right. If we're here, it means we're lowering the second time
+        cmp->SetOperRaw(GenTree::SwapRelop(cmp->OperGet()));
+        std::swap(left, right);
     }
 
-    return jtrue->gtNext;
+    if (cmp->OperIs(GT_EQ, GT_NE))
+    {
+        if (!right->IsIntegralConst(0))
+        {
+            // a == b  --->  (a - b) == 0
+            var_types  type = genActualTypeIsInt(left) ? TYP_INT : TYP_I_IMPL;
+            genTreeOps oper = GT_SUB;
+            if (right->IsIntegralConst() && !right->AsIntCon()->ImmedValNeedsReloc(comp))
+            {
+                INT64 value  = right->AsIntConCommon()->IntegralValue();
+                INT64 minVal = (type == TYP_INT) ? INT_MIN : SSIZE_T_MIN;
+
+                const INT64 min12BitImm = -2048;
+                if (value == min12BitImm)
+                {
+                    // (a - C) == 0 ---> (a ^ C) == 0
+                    oper = GT_XOR;
+                }
+                else if (value != minVal)
+                {
+                    // a - C  --->  a + (-C)
+                    oper = GT_ADD;
+                    right->AsIntConCommon()->SetIntegralValue(-value);
+                }
+            }
+            left  = comp->gtNewOperNode(oper, type, left, right);
+            right = comp->gtNewZeroConNode(type);
+            BlockRange().InsertBefore(cmp, left, right);
+            ContainCheckBinary(left->AsOp());
+        }
+        // a != 0  --->  a > 0 (unsigned)
+        cmp->SetOperRaw(cmp->OperIs(GT_EQ) ? GT_LE : GT_GT);
+        cmp->SetUnsigned();
+    }
+
+    if (cmp->OperIs(GT_LE, GT_GE))
+    {
+        if (right->IsIntegralConst() && !right->AsIntCon()->ImmedValNeedsReloc(comp))
+        {
+            // a <= C  --->  a < C+1
+            INT64 value = right->AsIntConCommon()->IntegralValue();
+
+            bool isOverflow;
+            if (cmp->OperIs(GT_LE))
+            {
+                isOverflow = genActualTypeIsInt(left)
+                                 ? CheckedOps::AddOverflows((INT32)value, (INT32)1, cmp->IsUnsigned())
+                                 : CheckedOps::AddOverflows((INT64)value, (INT64)1, cmp->IsUnsigned());
+            }
+            else
+            {
+                isOverflow = genActualTypeIsInt(left)
+                                 ? CheckedOps::SubOverflows((INT32)value, (INT32)1, cmp->IsUnsigned())
+                                 : CheckedOps::SubOverflows((INT64)value, (INT64)1, cmp->IsUnsigned());
+            }
+            if (isOverflow)
+            {
+                BlockRange().Remove(left);
+                BlockRange().Remove(right);
+                cmp->BashToConst(1);
+                return false;
+            }
+            right->AsIntConCommon()->SetIntegralValue(cmp->OperIs(GT_LE) ? value + 1 : value - 1);
+            cmp->SetOperRaw(cmp->OperIs(GT_LE) ? GT_LT : GT_GT);
+        }
+        else
+        {
+            // a <= b  --->  !(a > b)
+            isReversed = true;
+            cmp->SetOperRaw(GenTree::ReverseRelop(cmp->OperGet()));
+        }
+    }
+
+    if (cmp->OperIs(GT_GT))
+    {
+        cmp->SetOperRaw(GenTree::SwapRelop(cmp->OperGet()));
+        std::swap(left, right);
+    }
+    assert(cmp->OperIs(GT_LT));
+
+    if (right->IsIntegralConst(0) && !right->AsIntCon()->ImmedValNeedsReloc(comp) && !cmp->IsUnsigned())
+    {
+        // a < 0 (signed)  --->  shift the sign bit into the lowest bit
+        cmp->SetOperRaw(GT_RSZ);
+        cmp->ChangeType(genActualType(left));
+        right->AsIntConCommon()->SetIntegralValue(genTypeSize(cmp) * BITS_PER_BYTE - 1);
+        right->SetContained();
+        return isReversed;
+    }
+
+    SignExtendIfNecessary(&left);
+    SignExtendIfNecessary(&right);
+    return isReversed;
+}
+
+//------------------------------------------------------------------------
+// SignExtendIfNecessary: inserts a 32-bit sign extension unless the argument is full-register or is known to be
+// implemented with a sign-extending instruction.
+//
+// Arguments:
+//    arg - the argument to sign-extend
+//
+void Lowering::SignExtendIfNecessary(GenTree** arg)
+{
+    assert(varTypeUsesIntReg(*arg));
+    if (!genActualTypeIsInt(*arg))
+        return;
+
+    if ((*arg)->OperIs(GT_ADD, GT_SUB, GT_MUL, GT_MOD, GT_UMOD, GT_DIV, GT_UDIV, GT_CNS_INT))
+        return;
+
+    if ((*arg)->OperIsShiftOrRotate() || (*arg)->OperIsCmpCompare() || (*arg)->OperIsAtomicOp())
+        return;
+
+    *arg = comp->gtNewCastNode(TYP_I_IMPL, *arg, false, TYP_I_IMPL);
+    BlockRange().InsertAfter((*arg)->gtGetOp1(), *arg);
 }
 
 //------------------------------------------------------------------------
@@ -1140,6 +1291,9 @@ void Lowering::ContainCheckCast(GenTreeCast* node)
 //
 void Lowering::ContainCheckCompare(GenTreeOp* cmp)
 {
+    if (cmp->gtOp1->IsIntegralConst(0) && !cmp->gtOp1->AsIntCon()->ImmedValNeedsReloc(comp))
+        MakeSrcContained(cmp, cmp->gtOp1);
+
     CheckImmedAndMakeContained(cmp, cmp->gtOp2);
 }