From 76c556a55fae956ae8537fd347caf8595329abe4 Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Mon, 16 Jul 2018 11:24:54 -0700
Subject: [PATCH] Improve narrowing of GT_AND nodes. (dotnet/coreclr#18916)

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 | 63 ++++++++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 11 deletions(-)

diff --git a/src/coreclr/src/jit/optimizer.cpp b/src/coreclr/src/jit/optimizer.cpp
index c99703c..cc5d358 100644
--- a/src/coreclr/src/jit/optimizer.cpp
+++ b/src/coreclr/src/jit/optimizer.cpp
@@ -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))
                 {
-- 
2.7.4