Improve narrowing of GT_AND nodes. (dotnet/coreclr#18916)
authorEugene Rozenfeld <erozen@microsoft.com>
Mon, 16 Jul 2018 18:24:54 +0000 (11:24 -0700)
committerGitHub <noreply@github.com>
Mon, 16 Jul 2018 18:24:54 +0000 (11:24 -0700)
This is a follow-up to to dotnet/coreclr#18816 which resulted in a 6 byte regression in one of the
desktop SuperPMI methods. This change removes that regression and adds a number
of improved diffs.

If we are narrowing GT_AND to an unsigned type and one of the operands can be narrowed
into that type, the result of the GT_AND will also fit into that type and can be narrowed.
The same is true if one of the operands is an int const and can be narrowed into 'dsst'.

The change also ensures that we don't call optNarrowTree(false) more than once on each of the
GT_AND operands.

Commit migrated from https://github.com/dotnet/coreclr/commit/c0bad3c66218f61dd3e4f151b9d2eae22c6d88d2

src/coreclr/src/jit/optimizer.cpp

index c99703c..cc5d358 100644 (file)
@@ -5711,34 +5711,76 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
         switch (tree->gtOper)
         {
             case GT_AND:
+                noway_assert(genActualType(tree->gtType) == genActualType(op1->gtType));
                 noway_assert(genActualType(tree->gtType) == genActualType(op2->gtType));
 
-                // Is op2 a small constant than can be narrowed into dstt?
-                // if so the result of the GT_AND will also fit into 'dstt' and can be narrowed
-                if ((op2->gtOper == GT_CNS_INT) && optNarrowTree(op2, srct, dstt, NoVNPair, false))
+                GenTree* opToNarrow;
+                opToNarrow = nullptr;
+                GenTree** otherOpPtr;
+                otherOpPtr = nullptr;
+                bool canNotNarrowOperand;
+                canNotNarrowOperand = false;
+
+                // If 'dstt' is unsigned and one of the operands can be narrowed into 'dsst',
+                // the result of the GT_AND will also fit into 'dstt' and can be narrowed.
+                // The same is true if one of the operands is an int const and can be narrowed into 'dsst'.
+                if (!gtIsActiveCSE_Candidate(op2) && ((op2->gtOper == GT_CNS_INT) || varTypeIsUnsigned(dstt)))
                 {
-                    // We will change the type of the tree and narrow op2
+                    if (optNarrowTree(op2, srct, dstt, NoVNPair, false))
+                    {
+                        opToNarrow = op2;
+                        otherOpPtr = &tree->gtOp.gtOp1;
+                    }
+                    else
+                    {
+                        canNotNarrowOperand = true;
+                    }
+                }
+
+                if ((opToNarrow == nullptr) && !gtIsActiveCSE_Candidate(op1) &&
+                    ((op1->gtOper == GT_CNS_INT) || varTypeIsUnsigned(dstt)))
+                {
+                    if (optNarrowTree(op1, srct, dstt, NoVNPair, false))
+                    {
+                        opToNarrow = op1;
+                        otherOpPtr = &tree->gtOp.gtOp2;
+                    }
+                    else
+                    {
+                        canNotNarrowOperand = true;
+                    }
+                }
+
+                if (opToNarrow != nullptr)
+                {
+                    // We will change the type of the tree and narrow opToNarrow
                     //
                     if (doit)
                     {
                         tree->gtType = genActualType(dstt);
                         tree->SetVNs(vnpNarrow);
 
-                        optNarrowTree(op2, srct, dstt, NoVNPair, true);
-                        // We may also need to cast away the upper bits of op1
+                        optNarrowTree(opToNarrow, srct, dstt, NoVNPair, true);
+                        // We may also need to cast away the upper bits of *otherOpPtr
                         if (srcSize == 8)
                         {
                             assert(tree->gtType == TYP_INT);
-                            op1 = gtNewCastNode(TYP_INT, op1, false, TYP_INT);
+                            GenTree* castOp = gtNewCastNode(TYP_INT, op1, false, TYP_INT);
 #ifdef DEBUG
-                            op1->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
+                            castOp->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
 #endif
-                            tree->gtOp.gtOp1 = op1;
+                            *otherOpPtr = castOp;
                         }
                     }
                     return true;
                 }
 
+                if (canNotNarrowOperand)
+                {
+                    noway_assert(doit == false);
+                    return false;
+                }
+
                 goto COMMON_BINOP;
 
             case GT_ADD:
@@ -5753,10 +5795,9 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
 
             case GT_OR:
             case GT_XOR:
-            COMMON_BINOP:
                 noway_assert(genActualType(tree->gtType) == genActualType(op1->gtType));
                 noway_assert(genActualType(tree->gtType) == genActualType(op2->gtType));
-
+            COMMON_BINOP:
                 if (gtIsActiveCSE_Candidate(op1) || gtIsActiveCSE_Candidate(op2) ||
                     !optNarrowTree(op1, srct, dstt, NoVNPair, doit) || !optNarrowTree(op2, srct, dstt, NoVNPair, doit))
                 {