Improve narrowing of GT_AND nodes. (dotnet/coreclr#18995)
authorEugene Rozenfeld <erozen@microsoft.com>
Fri, 20 Jul 2018 22:35:47 +0000 (15:35 -0700)
committerGitHub <noreply@github.com>
Fri, 20 Jul 2018 22:35:47 +0000 (15:35 -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/ad09b5820b4aaf40033d930c4b64a89a78ddb556

src/coreclr/src/jit/optimizer.cpp

index c99703c..6158a82 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 foundOperandThatBlocksNarrowing;
+                foundOperandThatBlocksNarrowing = 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
+                    {
+                        foundOperandThatBlocksNarrowing = 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
+                    {
+                        foundOperandThatBlocksNarrowing = 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, *otherOpPtr, 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 (foundOperandThatBlocksNarrowing)
+                {
+                    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))
                 {