Move small unsigned type tweak to LowerCompare

This belongs in either LowerCompare or genCompareInt, it has nothing to do with TreeNodeInfoInitCmp.

Since this comparison change doesn't require codegen to actually generate a small compare let's move it to LowerCompare. It already tweaks types in a manner that requires changing the comparison type and having this there removes some redundancy.

No FX diffs.

diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp
index c5578bb..edab7ba 100644 (file)
--- a/src/jit/lower.cpp
+++ b/src/jit/lower.cpp
@@ -2026,11 +2026,6 @@ void Lowering::LowerCompare(GenTree* cmp)
             // (e.g "cmp ubyte, 200") we also get a smaller instruction encoding.
             //
 
-            if (varTypeIsUnsigned(op1Type))
-            {
-                cmp->gtFlags |= GTF_UNSIGNED;
-            }
-
             op2->gtType = op1Type;
         }
         else if (op1->OperIs(GT_CAST) && !op1->gtOverflow())
@@ -2047,11 +2042,9 @@ void Lowering::LowerCompare(GenTree* cmp)
                 {
                     assert(!castOp->gtOverflowEx()); // Must not be an overflow checking operation
 
-                    castOp->gtType = castToType;
-
+                    castOp->gtType  = castToType;
                     cmp->gtOp.gtOp1 = castOp;
-                    cmp->gtFlags |= GTF_UNSIGNED;
-                    op2->gtType = castToType;
+                    op2->gtType     = castToType;
 
                     BlockRange().Remove(cast);
                 }
@@ -2124,6 +2117,22 @@ void Lowering::LowerCompare(GenTree* cmp)
             }
         }
     }
+
+    if (cmp->gtGetOp1()->TypeGet() == cmp->gtGetOp2()->TypeGet())
+    {
+        if (varTypeIsSmall(cmp->gtGetOp1()->TypeGet()) && varTypeIsUnsigned(cmp->gtGetOp1()->TypeGet()))
+        {
+            //
+            // If both operands have the same type then codegen will use the common operand type to
+            // determine the instruction type. For small types this would result in performing a
+            // signed comparison of two small unsigned values without zero extending them to TYP_INT
+            // which is incorrect. Note that making the comparison unsigned doesn't imply that codegen
+            // has to generate a small comparison, it can still correctly generate a TYP_INT comparison.
+            //
+
+            cmp->gtFlags |= GTF_UNSIGNED;
+        }
+    }
 #endif // _TARGET_XARCH_
 
 #ifndef _TARGET_64BIT_

diff --git a/src/jit/lowerxarch.cpp b/src/jit/lowerxarch.cpp
index 14cd9fd..8a67415 100644 (file)
--- a/src/jit/lowerxarch.cpp
+++ b/src/jit/lowerxarch.cpp
@@ -3520,16 +3520,6 @@ void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree)
             // if one of them is on stack.
             SetRegOptional(PreferredRegOptionalOperand(tree));
         }
-
-        if (varTypeIsSmall(op1Type) && varTypeIsUnsigned(op1Type))
-        {
-            // Mark the tree as doing unsigned comparison if
-            // both the operands are small and unsigned types.
-            // Otherwise we will end up performing a signed comparison
-            // of two small unsigned values without zero extending them to
-            // TYP_INT size and which is incorrect.
-            tree->gtFlags |= GTF_UNSIGNED;
-        }
     }
 }