Merge pull request #11884 from mikedn/arm-cmp-setcc

[WIP] Lower long compares that are materialized into a register for ARM32

8 files changed, 90 insertions, 232 deletions

diff --git a/src/jit/codegenarm.cpp b/src/jit/codegenarm.cpp
index 124e99448b..438453b41a 100644
--- a/src/jit/codegenarm.cpp
+++ b/src/jit/codegenarm.cpp
@@ -1273,7 +1273,7 @@ void CodeGen::genCkfinite(GenTreePtr treeNode)
 }
 
 //------------------------------------------------------------------------
-// genCodeForCompare: Produce code for a GT_EQ/GT_NE/GT_LT/GT_LE/GT_GE/GT_GT node.
+// genCodeForCompare: Produce code for a GT_EQ/GT_NE/GT_LT/GT_LE/GT_GE/GT_GT/GT_CMP node.
 //
 // Arguments:
 //    tree - the node
@@ -1289,51 +1289,35 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
     var_types  op1Type = op1->TypeGet();
     var_types  op2Type = op2->TypeGet();
 
-    if (varTypeIsLong(op1Type))
-    {
-#ifdef DEBUG
-        // The result of an unlowered long compare on a 32-bit target must either be
-        // a) materialized into a register, or
-        // b) unused.
-        //
-        // A long compare that has a result that is used but not materialized into a register should
-        // have been handled by Lowering::LowerCompare.
+    assert(!varTypeIsLong(op1Type));
+    assert(!varTypeIsLong(op2Type));
 
-        LIR::Use use;
-        assert((tree->gtRegNum != REG_NA) || !LIR::AsRange(compiler->compCurBB).TryGetUse(tree, &use));
-#endif
-        genCompareLong(tree);
+    regNumber targetReg = tree->gtRegNum;
+    emitter*  emit      = getEmitter();
+
+    genConsumeIfReg(op1);
+    genConsumeIfReg(op2);
+
+    if (varTypeIsFloating(op1Type))
+    {
+        assert(op1Type == op2Type);
+        assert(!tree->OperIs(GT_CMP));
+        emit->emitInsBinary(INS_vcmp, emitTypeSize(op1Type), op1, op2);
+        // vmrs with register 0xf has special meaning of transferring flags
+        emit->emitIns_R(INS_vmrs, EA_4BYTE, REG_R15);
     }
     else
     {
-        assert(!varTypeIsLong(op2Type));
-
-        regNumber targetReg = tree->gtRegNum;
-        emitter*  emit      = getEmitter();
-
-        genConsumeIfReg(op1);
-        genConsumeIfReg(op2);
-
-        if (varTypeIsFloating(op1Type))
-        {
-            assert(op1Type == op2Type);
-            emit->emitInsBinary(INS_vcmp, emitTypeSize(op1Type), op1, op2);
-            // vmrs with register 0xf has special meaning of transferring flags
-            emit->emitIns_R(INS_vmrs, EA_4BYTE, REG_R15);
-        }
-        else
-        {
-            assert(!varTypeIsFloating(op2Type));
-            var_types cmpType = (op1Type == op2Type) ? op1Type : TYP_INT;
-            emit->emitInsBinary(INS_cmp, emitTypeSize(cmpType), op1, op2);
-        }
+        assert(!varTypeIsFloating(op2Type));
+        var_types cmpType = (op1Type == op2Type) ? op1Type : TYP_INT;
+        emit->emitInsBinary(INS_cmp, emitTypeSize(cmpType), op1, op2);
+    }
 
-        // Are we evaluating this into a register?
-        if (targetReg != REG_NA)
-        {
-            genSetRegToCond(targetReg, tree);
-            genProduceReg(tree);
-        }
+    // Are we evaluating this into a register?
+    if (targetReg != REG_NA)
+    {
+        genSetRegToCond(targetReg, tree);
+        genProduceReg(tree);
     }
 }
 
@@ -1430,158 +1414,6 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree)
 }
 
 //------------------------------------------------------------------------
-// genCompareLong: Generate code for comparing two longs when the result of the compare
-// is manifested in a register.
-//
-// Arguments:
-//    treeNode - the compare tree
-//
-// Return Value:
-//    None.
-//
-// Comments:
-// For long compares, we need to compare the high parts of operands first, then the low parts.
-// If the high compare is false, we do not need to compare the low parts. For less than and
-// greater than, if the high compare is true, we can assume the entire compare is true.
-//
-void CodeGen::genCompareLong(GenTreePtr treeNode)
-{
-    assert(treeNode->OperIsCompare());
-
-    GenTreeOp* tree = treeNode->AsOp();
-    GenTreePtr op1  = tree->gtOp1;
-    GenTreePtr op2  = tree->gtOp2;
-
-    assert(varTypeIsLong(op1->TypeGet()));
-    assert(varTypeIsLong(op2->TypeGet()));
-
-    regNumber targetReg = treeNode->gtRegNum;
-
-    genConsumeOperands(tree);
-
-    GenTreePtr loOp1 = op1->gtGetOp1();
-    GenTreePtr hiOp1 = op1->gtGetOp2();
-    GenTreePtr loOp2 = op2->gtGetOp1();
-    GenTreePtr hiOp2 = op2->gtGetOp2();
-
-    // Create compare for the high parts
-    instruction ins     = INS_cmp;
-    var_types   cmpType = TYP_INT;
-    emitAttr    cmpAttr = emitTypeSize(cmpType);
-
-    // Emit the compare instruction
-    getEmitter()->emitInsBinary(ins, cmpAttr, hiOp1, hiOp2);
-
-    // If the result is not being materialized in a register, we're done.
-    if (targetReg == REG_NA)
-    {
-        return;
-    }
-
-    BasicBlock* labelTrue  = genCreateTempLabel();
-    BasicBlock* labelFalse = genCreateTempLabel();
-    BasicBlock* labelNext  = genCreateTempLabel();
-
-    genJccLongHi(tree->gtOper, labelTrue, labelFalse, tree->IsUnsigned());
-    getEmitter()->emitInsBinary(ins, cmpAttr, loOp1, loOp2);
-    genJccLongLo(tree->gtOper, labelTrue, labelFalse);
-
-    genDefineTempLabel(labelFalse);
-    getEmitter()->emitIns_R_I(INS_mov, emitActualTypeSize(tree->gtType), tree->gtRegNum, 0);
-    getEmitter()->emitIns_J(INS_b, labelNext);
-
-    genDefineTempLabel(labelTrue);
-    getEmitter()->emitIns_R_I(INS_mov, emitActualTypeSize(tree->gtType), tree->gtRegNum, 1);
-
-    genDefineTempLabel(labelNext);
-
-    genProduceReg(tree);
-}
-
-void CodeGen::genJccLongHi(genTreeOps cmp, BasicBlock* jumpTrue, BasicBlock* jumpFalse, bool isUnsigned)
-{
-    if (cmp != GT_NE)
-    {
-        jumpFalse->bbFlags |= BBF_JMP_TARGET | BBF_HAS_LABEL;
-    }
-
-    switch (cmp)
-    {
-        case GT_EQ:
-            inst_JMP(EJ_ne, jumpFalse);
-            break;
-
-        case GT_NE:
-            inst_JMP(EJ_ne, jumpTrue);
-            break;
-
-        case GT_LT:
-        case GT_LE:
-            if (isUnsigned)
-            {
-                inst_JMP(EJ_hi, jumpFalse);
-                inst_JMP(EJ_lo, jumpTrue);
-            }
-            else
-            {
-                inst_JMP(EJ_gt, jumpFalse);
-                inst_JMP(EJ_lt, jumpTrue);
-            }
-            break;
-
-        case GT_GE:
-        case GT_GT:
-            if (isUnsigned)
-            {
-                inst_JMP(EJ_lo, jumpFalse);
-                inst_JMP(EJ_hi, jumpTrue);
-            }
-            else
-            {
-                inst_JMP(EJ_lt, jumpFalse);
-                inst_JMP(EJ_gt, jumpTrue);
-            }
-            break;
-
-        default:
-            noway_assert(!"expected a comparison operator");
-    }
-}
-
-void CodeGen::genJccLongLo(genTreeOps cmp, BasicBlock* jumpTrue, BasicBlock* jumpFalse)
-{
-    switch (cmp)
-    {
-        case GT_EQ:
-            inst_JMP(EJ_eq, jumpTrue);
-            break;
-
-        case GT_NE:
-            inst_JMP(EJ_ne, jumpTrue);
-            break;
-
-        case GT_LT:
-            inst_JMP(EJ_lo, jumpTrue);
-            break;
-
-        case GT_LE:
-            inst_JMP(EJ_ls, jumpTrue);
-            break;
-
-        case GT_GE:
-            inst_JMP(EJ_hs, jumpTrue);
-            break;
-
-        case GT_GT:
-            inst_JMP(EJ_hi, jumpTrue);
-            break;
-
-        default:
-            noway_assert(!"expected comparison");
-    }
-}
-
-//------------------------------------------------------------------------
 // genSetRegToCond: Generate code to materialize a condition into a register.
 //
 // Arguments:
diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp
index 37b9026db1..cb3a1d5cb7 100644
--- a/src/jit/codegenarmarch.cpp
+++ b/src/jit/codegenarmarch.cpp
@@ -230,6 +230,7 @@ void CodeGen::genCodeForTreeNode(GenTreePtr treeNode)
         case GT_LE:
         case GT_GE:
         case GT_GT:
+        case GT_CMP:
             genCodeForCompare(treeNode->AsOp());
             break;
 
@@ -243,6 +244,10 @@ void CodeGen::genCodeForTreeNode(GenTreePtr treeNode)
             genCodeForJcc(treeNode->AsCC());
             break;
 
+        case GT_SETCC:
+            genCodeForSetcc(treeNode->AsCC());
+            break;
+
 #endif // _TARGET_ARM_
 
         case GT_RETURNTRAP:
@@ -2597,6 +2602,54 @@ void CodeGen::genCodeForJcc(GenTreeCC* tree)
     inst_JMP(jumpKind, compiler->compCurBB->bbJumpDest);
 }
 
+//------------------------------------------------------------------------
+// genCodeForSetcc: Generates code for a GT_SETCC node.
+//
+// Arguments:
+//    setcc - the GT_SETCC node
+//
+// Assumptions:
+//    The condition represents an integer comparison. This code doesn't
+//    have the necessary logic to deal with floating point comparisons,
+//    in fact it doesn't even know if the comparison is integer or floating
+//    point because SETCC nodes do not have any operands.
+//
+
+void CodeGen::genCodeForSetcc(GenTreeCC* setcc)
+{
+    regNumber    dstReg      = setcc->gtRegNum;
+    CompareKind  compareKind = setcc->IsUnsigned() ? CK_UNSIGNED : CK_SIGNED;
+    emitJumpKind jumpKind    = genJumpKindForOper(setcc->gtCondition, compareKind);
+
+    assert(genIsValidIntReg(dstReg));
+    // Make sure nobody is setting GTF_RELOP_NAN_UN on this node as it is ignored.
+    assert((setcc->gtFlags & GTF_RELOP_NAN_UN) == 0);
+
+    // Emit code like that:
+    //   ...
+    //   bgt True
+    //   movs rD, #0
+    //   b Next
+    // True:
+    //   movs rD, #1
+    // Next:
+    //   ...
+
+    BasicBlock* labelTrue = genCreateTempLabel();
+    getEmitter()->emitIns_J(emitter::emitJumpKindToIns(jumpKind), labelTrue);
+
+    getEmitter()->emitIns_R_I(INS_mov, emitActualTypeSize(setcc->TypeGet()), dstReg, 0);
+
+    BasicBlock* labelNext = genCreateTempLabel();
+    getEmitter()->emitIns_J(INS_b, labelNext);
+
+    genDefineTempLabel(labelTrue);
+    getEmitter()->emitIns_R_I(INS_mov, emitActualTypeSize(setcc->TypeGet()), dstReg, 1);
+    genDefineTempLabel(labelNext);
+
+    genProduceReg(setcc);
+}
+
 #endif // defined(_TARGET_ARM_)
 
 //------------------------------------------------------------------------
diff --git a/src/jit/codegenlinear.h b/src/jit/codegenlinear.h
index 59763bc5c8..54c6db1032 100644
--- a/src/jit/codegenlinear.h
+++ b/src/jit/codegenlinear.h
@@ -53,12 +53,6 @@ unsigned getFirstArgWithStackSlot();
 void genCompareFloat(GenTreePtr treeNode);
 void genCompareInt(GenTreePtr treeNode);
 
-#if defined(_TARGET_ARM_)
-void genCompareLong(GenTreePtr treeNode);
-void genJccLongHi(genTreeOps cmp, BasicBlock* jumpTrue, BasicBlock* jumpFalse, bool isUnsigned = false);
-void genJccLongLo(genTreeOps cmp, BasicBlock* jumpTrue, BasicBlock* jumpFalse);
-#endif // defined(_TARGET_ARM_)
-
 #ifdef FEATURE_SIMD
 enum SIMDScalarMoveType
 {
diff --git a/src/jit/emitarm.cpp b/src/jit/emitarm.cpp
index 4f3f5e629a..72ad88337d 100644
--- a/src/jit/emitarm.cpp
+++ b/src/jit/emitarm.cpp
@@ -7787,6 +7787,14 @@ regNumber emitter::emitInsTernary(instruction ins, emitAttr attr, GenTree* dst,
             assert(!"Invalid ins for overflow check");
         }
     }
+
+    if (dst->gtSetFlags())
+    {
+        assert((ins == INS_add) || (ins == INS_adc) || (ins == INS_sub) || (ins == INS_sbc) || (ins == INS_and) ||
+               (ins == INS_orr) || (ins == INS_eor) || (ins == INS_orn));
+        flags = INS_FLAGS_SET;
+    }
+
     if (intConst != nullptr)
     {
         emitIns_R_R_I(ins, attr, dst->gtRegNum, nonIntReg->gtRegNum, intConst->IconValue(), flags);
diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp
index 6fc6646547..5c46d3d235 100644
--- a/src/jit/lower.cpp
+++ b/src/jit/lower.cpp
@@ -2027,18 +2027,8 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget
 void Lowering::LowerCompare(GenTree* cmp)
 {
 #ifndef _TARGET_64BIT_
-
-#ifdef _TARGET_ARM_
-    // TODO-ARM: Later ARM32 should make use of same condition of x86 once support for GT_CMP and GT_SETCC is added.
-    LIR::Use cmpUse;
-    if ((cmp->gtGetOp1()->TypeGet() == TYP_LONG) && BlockRange().TryGetUse(cmp, &cmpUse) &&
-        cmpUse.User()->OperIs(GT_JTRUE))
-#elif defined(_TARGET_X86_)
     if (cmp->gtGetOp1()->TypeGet() == TYP_LONG)
-#endif
     {
-// TODO-ARM: This code should be enabled for ARM32 once support for GT_CMP and GT_SETCC is added.
-#if _TARGET_X86_
         // Currently this handles only relops that produce a value or aren't used.
         // The same approach can be used for relops that feed a GT_JTRUE, see the #if 0
         // below and its associated comment.
@@ -2167,7 +2157,6 @@ void Lowering::LowerCompare(GenTree* cmp)
 
             return;
         }
-#endif
 
         // For 32-bit targets any comparison that feeds a `GT_JTRUE` node must be lowered such that
         // the liveness of the operands to the comparison is properly visible to the rest of the
diff --git a/src/jit/lowerarmarch.cpp b/src/jit/lowerarmarch.cpp
index daaedc4cf8..ae29cf21d5 100644
--- a/src/jit/lowerarmarch.cpp
+++ b/src/jit/lowerarmarch.cpp
@@ -132,6 +132,7 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode)
             case GT_LE:
             case GT_GE:
             case GT_GT:
+            case GT_CMP:
             case GT_AND:
             case GT_OR:
             case GT_XOR:
diff --git a/src/jit/lsraarm.cpp b/src/jit/lsraarm.cpp
index 0e033cb661..4fade03ef4 100644
--- a/src/jit/lsraarm.cpp
+++ b/src/jit/lsraarm.cpp
@@ -639,6 +639,7 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
         case GT_LE:
         case GT_GE:
         case GT_GT:
+        case GT_CMP:
             TreeNodeInfoInitCmp(tree);
             break;
 
@@ -739,6 +740,7 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
         case GT_LABEL:
         case GT_PINVOKE_PROLOG:
         case GT_JCC:
+        case GT_SETCC:
         case GT_MEMORYBARRIER:
         case GT_OBJ:
             info->dstCount = tree->IsValue() ? 1 : 0;
diff --git a/src/jit/lsraarmarch.cpp b/src/jit/lsraarmarch.cpp
index d33eb30f8c..e8620475fa 100644
--- a/src/jit/lsraarmarch.cpp
+++ b/src/jit/lsraarmarch.cpp
@@ -79,28 +79,7 @@ void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree)
     TreeNodeInfo* info = &(tree->gtLsraInfo);
 
     info->srcCount = 2;
-    info->dstCount = 1;
-
-#ifdef _TARGET_ARM_
-
-    GenTreePtr op1     = tree->gtOp.gtOp1;
-    GenTreePtr op2     = tree->gtOp.gtOp2;
-    var_types  op1Type = op1->TypeGet();
-    var_types  op2Type = op2->TypeGet();
-
-    // Long compares will consume GT_LONG nodes, each of which produces two results.
-    // Thus for each long operand there will be an additional source.
-    // TODO-ARM-CQ: Mark hiOp2 and loOp2 as contained if it is a constant.
-    if (varTypeIsLong(op1Type))
-    {
-        info->srcCount++;
-    }
-    if (varTypeIsLong(op2Type))
-    {
-        info->srcCount++;
-    }
-
-#endif // _TARGET_ARM_
+    info->dstCount = tree->OperIs(GT_CMP) ? 0 : 1;
 
     CheckImmedAndMakeContained(tree, tree->gtOp.gtOp2);
 }