Reimplement compare flags reuse using SETCC/JCC

Author: mikedn

src/jit/codegenxarch.cpp

@@ -6244,42 +6244,6 @@ void CodeGen::genCompareInt(GenTreePtr treeNode)
     var_types  op2Type   = op2->TypeGet();
     regNumber  targetReg = tree->gtRegNum;
 
-    // Case of op1 == 0 or op1 != 0:
-    // Optimize generation of 'test' instruction if op1 sets flags.
-    //
-    // Note that if LSRA has inserted any GT_RELOAD/GT_COPY before
-    // op1, it will not modify the flags set by codegen of op1.
-    // Similarly op1 could also be reg-optional at its use and
-    // it was spilled after producing its result in a register.
-    // Spill code too will not modify the flags set by op1.
-    GenTree* realOp1 = op1->gtSkipReloadOrCopy();
-    if ((tree->gtFlags & GTF_USE_FLAGS) != 0)
-    {
-        // op1 must set ZF and SF flags
-        assert(realOp1->gtSetZSFlags());
-        assert(realOp1->gtSetFlags());
-
-        // Must be (in)equality against zero.
-        assert(tree->OperIs(GT_EQ, GT_NE));
-        assert(op2->IsIntegralConst(0));
-        assert(op2->isContained());
-
-        // Just consume the operands
-        genConsumeOperands(tree);
-
-        // No need to generate test instruction since
-        // op1 sets flags
-
-        // Are we evaluating this into a register?
-        if (targetReg != REG_NA)
-        {
-            genSetRegToCond(targetReg, tree);
-            genProduceReg(tree);
-        }
-
-        return;
-    }
-
     genConsumeOperands(tree);
 
     // TODO-CQ: We should be able to support swapping op1 and op2 to generate cmp reg, imm.

src/jit/lower.cpp

@@ -188,8 +188,7 @@ GenTree* Lowering::LowerNode(GenTree* node)
         case GT_TEST_EQ:
         case GT_TEST_NE:
         case GT_CMP:
-            LowerCompare(node);
-            break;
+            return LowerCompare(node);
 
         case GT_JTRUE:
             ContainCheckJTrue(node->AsOp());
@@ -2138,7 +2137,7 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget
 //      be used for ARM as well if support for GT_TEST_EQ/GT_TEST_NE is added).
 //    - Transform TEST(x, LSH(1, y)) into BT(x, y) (XARCH specific)
 
-void Lowering::LowerCompare(GenTree* cmp)
+GenTree* Lowering::LowerCompare(GenTree* cmp)
 {
 #ifndef _TARGET_64BIT_
     if (cmp->gtGetOp1()->TypeGet() == TYP_LONG)
@@ -2344,7 +2343,7 @@ void Lowering::LowerCompare(GenTree* cmp)
             cmp->AsCC()->gtCondition = condition;
         }
 
-        return;
+        return cmp->gtNext;
     }
 #endif
 
@@ -2578,7 +2577,42 @@ void Lowering::LowerCompare(GenTree* cmp)
 
             cc->gtFlags |= GTF_USE_FLAGS | GTF_UNSIGNED;
 
-            return;
+            return cmp->gtNext;
+        }
+    }
+    else if (cmp->OperIs(GT_EQ, GT_NE))
+    {
+        GenTree* op1 = cmp->gtGetOp1();
+        GenTree* op2 = cmp->gtGetOp2();
+        LIR::Use cmpUse;
+
+        if (op1->gtSetZSFlags() && op2->IsIntegralConst(0) && (op1->gtNext == op2) && (op2->gtNext == cmp) &&
+            BlockRange().TryGetUse(cmp, &cmpUse))
+        {
+            genTreeOps condition = cmp->OperGet();
+            GenTree*   next      = cmp->gtNext;
+            GenTreeCC* cc;
+
+            if (cmpUse.User()->OperIs(GT_JTRUE))
+            {
+                cmpUse.User()->ChangeOper(GT_JCC);
+                cc = cmpUse.User()->AsCC();
+                BlockRange().Remove(cmp);
+            }
+            else
+            {
+                cmp->ChangeOper(GT_SETCC);
+                cc = cmp->AsCC();
+            }
+
+            cc->gtCondition = condition;
+            cc->gtFlags |= GTF_USE_FLAGS;
+            op1->gtFlags |= GTF_SET_FLAGS;
+            op1->SetUnusedValue();
+
+            BlockRange().Remove(op2);
+
+            return next;
         }
     }
 
@@ -2599,6 +2633,7 @@ void Lowering::LowerCompare(GenTree* cmp)
     }
 #endif // _TARGET_XARCH_
     ContainCheckCompare(cmp->AsOp());
+    return cmp->gtNext;
 }
 
 // Lower "jmp <method>" tail call to insert PInvoke method epilog if required.

src/jit/lower.h

@@ -137,7 +137,7 @@ class Lowering : public Phase
     // Call Lowering
     // ------------------------------
     void LowerCall(GenTree* call);
-    void LowerCompare(GenTree* tree);
+    GenTree* LowerCompare(GenTree* tree);
     void LowerJmpMethod(GenTree* jmp);
     void LowerRet(GenTree* ret);
     GenTree* LowerDelegateInvoke(GenTreeCall* call);

src/jit/lowerxarch.cpp

@@ -1838,41 +1838,6 @@ void Lowering::ContainCheckCompare(GenTreeOp* cmp)
             {
                 MakeSrcContained(cmp, op1);
             }
-            // If op1 codegen sets ZF and SF flags and ==/!= against
-            // zero, we don't need to generate test 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.
-            //
-            // TODO-CQ: We can optimize compare against zero in the
-            // following cases by generating the branch as indicated
-            // against each case.
-            //  1) unsigned compare
-            //        < 0  - always FALSE
-            //       <= 0  - ZF=1 and jne
-            //        > 0  - ZF=0 and je
-            //       >= 0  - always TRUE
-            //
-            // 2) signed compare
-            //        < 0  - SF=1 and js
-            //       >= 0  - SF=0 and jns
-            else if (cmp->OperIs(GT_EQ, GT_NE) && op1->gtSetZSFlags() && op2->IsIntegralConst(0) &&
-                     (op1->gtNext == op2) && (op2->gtNext == cmp))
-            {
-                // Require codegen of op1 to set the flags.
-                assert(!op1->gtSetFlags());
-                op1->gtFlags |= GTF_SET_FLAGS;
-                cmp->gtFlags |= GTF_USE_FLAGS;
-            }
             else
             {
                 SetRegOptional(op1);