Move and-cmp-test transform from TreeNodeInfoInitCmp to LowerCompare

FX diff shows a 32 bytes improvement without any regressions. Previously the first GT_AND operand wasn't marked reg optional and sometimes it ended up being loaded into a register:

mov      eax, dword ptr [rsp+B0H]
test     eax, 1

Not anymore:

test     dword ptr [rsp+B0H], 1

Also, we now generate TEST instead of AND even in cases where the second AND operand isn't a containable constant.

diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp
index c4d4fb9e360311299c9af81e139f846c467d90a3..9974befec0db76530b42943fc4fa703fc6ff3921 100644 (file)
--- a/src/jit/codegenxarch.cpp
+++ b/src/jit/codegenxarch.cpp
@@ -6146,7 +6146,7 @@ void CodeGen::genCompareInt(GenTreePtr treeNode)
 
     // By default we use an int32 sized cmp instruction
     //
-    ins               = ((treeNode->OperGet() == GT_TEST_EQ) || (treeNode->OperGet() == GT_TEST_NE)) ? INS_test : INS_cmp;
+    ins = ((treeNode->OperGet() == GT_TEST_EQ) || (treeNode->OperGet() == GT_TEST_NE)) ? INS_test : INS_cmp;
     var_types cmpType = TYP_INT;
 
     // In the if/then/else statement below we may change the

diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp
index 3c9fd90491fcbc36f86259e890e2a1dde568ddc0..80d3bad50d3051d5d9952fce13d9b39c21f684b7 100644 (file)
--- a/src/jit/lower.cpp
+++ b/src/jit/lower.cpp
@@ -2057,6 +2057,10 @@ void Lowering::LowerCompare(GenTree* cmp)
         }
         else if (op1->OperGet() == GT_AND && ((cmp->OperGet() == GT_EQ) || (cmp->OperGet() == GT_NE)))
         {
+            //
+            // Transform ((x AND y) EQ|NE 0) into (x TEST_EQ|TEST_NE y) when possible.
+            //
+
             GenTree* andOp1 = op1->gtGetOp1();
             GenTree* andOp2 = op1->gtGetOp2();
 
@@ -2074,6 +2078,36 @@ void Lowering::LowerCompare(GenTree* cmp)
                     cmp->SetOperRaw(GenTree::ReverseRelop(cmp->OperGet()));
                 }
             }
+
+            // TODO-CQ: The second GT_AND operand does not need to be a constant, this is a temporary
+            // restriction to prevent TreeNodeInfoInitCmp creating a contained second memory operand.
+            // Unlike CMP TEST does not have a TEST reg, mem form.
+
+            if (op2Value == 0 && andOp2->IsIntegralConst())
+            {
+                BlockRange().Remove(op1);
+                BlockRange().Remove(op2);
+
+                cmp->SetOperRaw(cmp->OperGet() == GT_EQ ? GT_TEST_EQ : GT_TEST_NE);
+                cmp->gtOp.gtOp1 = andOp1;
+                cmp->gtOp.gtOp2 = andOp2;
+
+                if (andOp1->isMemoryOp() && varTypeIsSmallInt(andOp1))
+                {
+                    ssize_t andOp2Value = andOp2->AsIntCon()->IconValue();
+
+                    // TOOD-CQ: There's more than can be done here. This is just enough to prevent
+                    // code size regressions.
+
+                    if (((andOp1->TypeGet() == TYP_UBYTE) && FitsIn<UINT8>(andOp2Value)) ||
+                        ((andOp1->TypeGet() == TYP_BYTE) && FitsIn<INT8>(andOp2Value)) ||
+                        ((andOp1->TypeGet() == TYP_CHAR) && FitsIn<UINT16>(andOp2Value)) ||
+                        ((andOp1->TypeGet() == TYP_SHORT) && FitsIn<INT16>(andOp2Value)))
+                    {
+                        andOp2->gtType = andOp1->TypeGet();
+                    }
+                }
+            }
         }
     }
 #endif // _TARGET_XARCH_

diff --git a/src/jit/lowerxarch.cpp b/src/jit/lowerxarch.cpp
index 844063091d651d41c13d8e771875246434c66985..2408b761c152a79014ed1c5c7a0489bb30da4f85 100644 (file)
--- a/src/jit/lowerxarch.cpp
+++ b/src/jit/lowerxarch.cpp
@@ -3459,123 +3459,7 @@ void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree)
             else
             {
                 bool op1IsMadeContained = false;
-
-                // When op1 is a GT_AND we can often generate a single "test" instruction
-                // instead of two instructions (an "and" instruction followed by a "cmp"/"test").
-                //
-                // This instruction can only be used for equality or inequality comparisons.
-                // and we must have a compare against zero.
-                //
-                // If we have a postive test for a single bit we can reverse the condition and
-                // make the compare be against zero.
-                //
-                // Example:
-                //                  GT_EQ                              GT_NE
-                //                  /   \                              /   \
-                //             GT_AND   GT_CNS (0x100)  ==>>      GT_AND   GT_CNS (0)
-                //             /    \                             /    \
-                //          andOp1  GT_CNS (0x100)             andOp1  GT_CNS (0x100)
-                //
-                // We will mark the GT_AND node as contained if the tree is an equality compare with zero.
-                // Additionally, when we do this we also allow for a contained memory operand for "andOp1".
-                //
-                bool isEqualityCompare = (tree->gtOper == GT_EQ || tree->gtOper == GT_NE);
-
-                if (isEqualityCompare && (op1->OperGet() == GT_AND))
-                {
-                    GenTreePtr andOp2 = op1->gtOp.gtOp2;
-                    if (IsContainableImmed(op1, andOp2))
-                    {
-                        ssize_t andOp2CnsVal = andOp2->AsIntConCommon()->IconValue();
-                        ssize_t relOp2CnsVal = op2->AsIntConCommon()->IconValue();
-
-                        // Now do we have a equality compare with zero?
-                        //
-                        if (relOp2CnsVal == 0)
-                        {
-                            // Note that child nodes must be made contained before parent nodes
-
-                            // Check for a memory operand for op1 with the test instruction
-                            //
-                            GenTreePtr andOp1 = op1->gtOp.gtOp1;
-                            if (andOp1->isMemoryOp())
-                            {
-                                // If the type of value memoryOp (andOp1) is not the same as the type of constant
-                                // (andOp2) check to see whether it is safe to mark AndOp1 as contained.  For e.g. in
-                                // the following case it is not safe to mark andOp1 as contained
-                                //    AndOp1 = signed byte and andOp2 is an int constant of value 512.
-                                //
-                                // If it is safe, we update the type and value of andOp2 to match with andOp1.
-                                bool containable = (andOp1->TypeGet() == op1->TypeGet());
-                                if (!containable)
-                                {
-                                    ssize_t newIconVal = 0;
-
-                                    switch (andOp1->TypeGet())
-                                    {
-                                        default:
-                                            break;
-                                        case TYP_BYTE:
-                                            newIconVal  = (signed char)andOp2CnsVal;
-                                            containable = FitsIn<signed char>(andOp2CnsVal);
-                                            break;
-                                        case TYP_BOOL:
-                                        case TYP_UBYTE:
-                                            newIconVal  = andOp2CnsVal & 0xFF;
-                                            containable = true;
-                                            break;
-                                        case TYP_SHORT:
-                                            newIconVal  = (signed short)andOp2CnsVal;
-                                            containable = FitsIn<signed short>(andOp2CnsVal);
-                                            break;
-                                        case TYP_CHAR:
-                                            newIconVal  = andOp2CnsVal & 0xFFFF;
-                                            containable = true;
-                                            break;
-                                        case TYP_INT:
-                                            newIconVal  = (INT32)andOp2CnsVal;
-                                            containable = FitsIn<INT32>(andOp2CnsVal);
-                                            break;
-                                        case TYP_UINT:
-                                            newIconVal  = andOp2CnsVal & 0xFFFFFFFF;
-                                            containable = true;
-                                            break;
-
-#ifdef _TARGET_64BIT_
-                                        case TYP_LONG:
-                                            newIconVal  = (INT64)andOp2CnsVal;
-                                            containable = true;
-                                            break;
-                                        case TYP_ULONG:
-                                            newIconVal  = (UINT64)andOp2CnsVal;
-                                            containable = true;
-                                            break;
-#endif //_TARGET_64BIT_
-                                    }
-
-                                    if (containable)
-                                    {
-                                        andOp2->gtType = andOp1->TypeGet();
-                                        andOp2->AsIntConCommon()->SetIconValue(newIconVal);
-                                    }
-                                }
-
-                                // Mark the 'andOp1' memory operand as contained
-                                // Note that for equality comparisons we don't need
-                                // to deal with any signed or unsigned issues.
-                                if (containable)
-                                {
-                                    MakeSrcContained(op1, andOp1);
-                                }
-                            }
-                            // Mark the 'op1' (the GT_AND) operand as contained
-                            MakeSrcContained(tree, op1);
-                            op1IsMadeContained = true;
-
-                            // During Codegen we will now generate "test andOp1, andOp2CnsVal"
-                        }
-                    }
-                }
+                bool isEqualityCompare  = (tree->gtOper == GT_EQ || tree->gtOper == GT_NE);
 
                 // If not made contained, op1 can be marked as reg-optional.
                 if (!op1IsMadeContained)