[Arm64] Use GTF_SET_FLAGS/GTF_USE_FLAGS

5 files changed, 82 insertions, 37 deletions

diff --git a/src/jit/codegenarm64.cpp b/src/jit/codegenarm64.cpp
index 20061d4beb..e5c1c46c77 100644
--- a/src/jit/codegenarm64.cpp
+++ b/src/jit/codegenarm64.cpp
@@ -1558,6 +1558,24 @@ void CodeGen::genCodeForBinary(GenTree* treeNode)
     GenTreePtr  op2 = treeNode->gtGetOp2();
     instruction ins = genGetInsForOper(treeNode->OperGet(), targetType);
 
+    if ((treeNode->gtFlags & GTF_SET_FLAGS) != 0)
+    {
+        switch (oper)
+        {
+            case GT_ADD:
+                ins = INS_adds;
+                break;
+            case GT_SUB:
+                ins = INS_subs;
+                break;
+            case GT_AND:
+                ins = INS_ands;
+                break;
+            default:
+                assert(!"Unexpected BinaryOp with GTF_SET_FLAGS set");
+        }
+    }
+
     // The arithmetic node must be sitting in a register (since it's not contained)
     assert(targetReg != REG_NA);
 
@@ -3272,10 +3290,6 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
     regNumber targetReg = tree->gtRegNum;
     emitter*  emit      = getEmitter();
 
-    // TODO-ARM64-CQ: Check if we can use the currently set flags.
-    // TODO-ARM64-CQ: Check for the case where we can simply transfer the carry bit to a register
-    //         (signed < or >= where targetReg != REG_NA)
-
     GenTreePtr op1     = tree->gtOp1;
     GenTreePtr op2     = tree->gtOp2;
     var_types  op1Type = genActualType(op1->TypeGet());
@@ -3286,42 +3300,45 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
 
     genConsumeOperands(tree);
 
-    emitAttr cmpSize = EA_ATTR(genTypeSize(op1Type));
-
-    assert(genTypeSize(op1Type) == genTypeSize(op2Type));
-
-    if (varTypeIsFloating(op1Type))
+    if ((tree->gtFlags & GTF_USE_FLAGS) == 0)
     {
-        assert(varTypeIsFloating(op2Type));
-        assert(!op1->isContained());
-        assert(op1Type == op2Type);
+        emitAttr cmpSize = EA_ATTR(genTypeSize(op1Type));
 
-        if (op2->IsIntegralConst(0))
+        assert(genTypeSize(op1Type) == genTypeSize(op2Type));
+
+        if (varTypeIsFloating(op1Type))
         {
-            emit->emitIns_R_F(INS_fcmp, cmpSize, op1->gtRegNum, 0.0);
+            assert(varTypeIsFloating(op2Type));
+            assert(!op1->isContained());
+            assert(op1Type == op2Type);
+
+            if (op2->IsIntegralConst(0))
+            {
+                emit->emitIns_R_F(INS_fcmp, cmpSize, op1->gtRegNum, 0.0);
+            }
+            else
+            {
+                assert(!op2->isContained());
+                emit->emitIns_R_R(INS_fcmp, cmpSize, op1->gtRegNum, op2->gtRegNum);
+            }
         }
         else
         {
-            assert(!op2->isContained());
-            emit->emitIns_R_R(INS_fcmp, cmpSize, op1->gtRegNum, op2->gtRegNum);
-        }
-    }
-    else
-    {
-        assert(!varTypeIsFloating(op2Type));
-        // We don't support swapping op1 and op2 to generate cmp reg, imm
-        assert(!op1->isContainedIntOrIImmed());
+            assert(!varTypeIsFloating(op2Type));
+            // We don't support swapping op1 and op2 to generate cmp reg, imm
+            assert(!op1->isContainedIntOrIImmed());
 
-        instruction ins = tree->OperIs(GT_TEST_EQ, GT_TEST_NE) ? INS_tst : INS_cmp;
+            instruction ins = tree->OperIs(GT_TEST_EQ, GT_TEST_NE) ? INS_tst : INS_cmp;
 
-        if (op2->isContainedIntOrIImmed())
-        {
-            GenTreeIntConCommon* intConst = op2->AsIntConCommon();
-            emit->emitIns_R_I(ins, cmpSize, op1->gtRegNum, intConst->IconValue());
-        }
-        else
-        {
-            emit->emitIns_R_R(ins, cmpSize, op1->gtRegNum, op2->gtRegNum);
+            if (op2->isContainedIntOrIImmed())
+            {
+                GenTreeIntConCommon* intConst = op2->AsIntConCommon();
+                emit->emitIns_R_I(ins, cmpSize, op1->gtRegNum, intConst->IconValue());
+            }
+            else
+            {
+                emit->emitIns_R_R(ins, cmpSize, op1->gtRegNum, op2->gtRegNum);
+            }
         }
     }
 
diff --git a/src/jit/emitarm64.cpp b/src/jit/emitarm64.cpp
index 0feb5e38c6..d901f8a8cf 100644
--- a/src/jit/emitarm64.cpp
+++ b/src/jit/emitarm64.cpp
@@ -11448,11 +11448,11 @@ regNumber emitter::emitInsTernary(instruction ins, emitAttr attr, GenTree* dst,
     bool isMulOverflow = false;
     if (dst->gtOverflowEx())
     {
-        if (ins == INS_add)
+        if ((ins == INS_add) || (ins == INS_adds))
         {
             ins = INS_adds;
         }
-        else if (ins == INS_sub)
+        else if ((ins == INS_sub) || (ins == INS_subs))
         {
             ins = INS_subs;
         }
diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 17d86f805f..390aac1eed 100644
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -8296,7 +8296,7 @@ bool GenTree::gtSetFlags() const
 #endif
 
 #else // !LEGACY_BACKEND
-#ifdef _TARGET_XARCH_
+#if defined(_TARGET_XARCH_) || defined(_TARGET_ARM64_)
     if (((gtFlags & GTF_SET_FLAGS) != 0) && (gtOper != GT_IND))
     {
         // GTF_SET_FLAGS is not valid on GT_IND and is overlaid with GTF_NONFAULTING_IND
diff --git a/src/jit/lowerarmarch.cpp b/src/jit/lowerarmarch.cpp
index 4cee63b006..65db5cd752 100644
--- a/src/jit/lowerarmarch.cpp
+++ b/src/jit/lowerarmarch.cpp
@@ -690,7 +690,35 @@ void Lowering::ContainCheckCast(GenTreeCast* node)
 //
 void Lowering::ContainCheckCompare(GenTreeOp* cmp)
 {
-    ContainCheckBinary(cmp);
+    if (CheckImmedAndMakeContained(cmp, cmp->gtOp2))
+    {
+#ifdef _TARGET_ARM64_
+        GenTreePtr op1 = cmp->gtOp.gtOp1;
+        GenTreePtr op2 = cmp->gtOp.gtOp2;
+
+        // If op1 codegen can set flags op2 is an immediate 0
+        // we don't need to generate cmp instruction,
+        // provided we don't have another GenTree node between op1
+        // and cmp that could potentially modify flags.
+        //
+        // TODO-CQ: right now the below peep is inexpensive and
+        // gets the benefit in most of cases because in majority
+        // of cases op1, op2 and cmp would be in that order in
+        // execution.  In general we should be able to check that all
+        // the nodes that come after op1 in execution order do not
+        // modify the flags so that it is safe to avoid generating a
+        // test instruction.  Such a check requires that on each
+        // GenTree node we need to set the info whether its codegen
+        // will modify flags.
+        if (op2->IsIntegralConst(0) && (op1->gtNext == op2) && (op2->gtNext == cmp) &&
+            cmp->OperIs(GT_EQ, GT_NE, GT_GT, GT_GE, GT_LT, GT_LE) && op1->OperIs(GT_ADD, GT_AND, GT_SUB))
+        {
+            assert(!op1->gtSetFlags());
+            op1->gtFlags |= GTF_SET_FLAGS;
+            cmp->gtFlags |= GTF_USE_FLAGS;
+        }
+#endif // _TARGET_ARM64_
+    }
 }
 
 //------------------------------------------------------------------------
diff --git a/src/jit/target.h b/src/jit/target.h
index 59bbdb492b..fd28796bd2 100644
--- a/src/jit/target.h
+++ b/src/jit/target.h
@@ -1533,7 +1533,7 @@ typedef unsigned short regPairNoSmall; // arm: need 12 bits
   #define FEATURE_MULTIREG_STRUCT_PROMOTE 1  // True when we want to promote fields of a multireg struct into registers
   #define FEATURE_FASTTAILCALL     1       // Tail calls made as epilog+jmp
   #define FEATURE_TAILCALL_OPT     1       // opportunistic Tail calls (i.e. without ".tail" prefix) made as fast tail calls.
-  #define FEATURE_SET_FLAGS        1       // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
+  #define FEATURE_SET_FLAGS        0       // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
   #define FEATURE_MULTIREG_ARGS_OR_RET  1  // Support for passing and/or returning single values in more than one register  
   #define FEATURE_MULTIREG_ARGS         1  // Support for passing a single argument in more than one register  
   #define FEATURE_MULTIREG_RET          1  // Support for returning a single value in more than one register