Extend the "skip-test" optimization to all instructions that modify the (#120876)

Lotendan · Lotendan · web-flow · commit be9df76fe27a · 2025-12-19T09:49:09.000-08:00
Extend the "skip-test" optimization to all instructions that modify the same flags as "TEST" x86 instruction Originally wanted to only cover the "POPCNT" case, but actually we can go further than that and cover more instructions. Fixes #118811 Using the following code: ```csharp static void M1(int value) { if (int.PopCount(value) > 0) throw null!; } ``` This fix reduces this assembly: ```asm popcnt edi, edi test edi, edi jg SHORT G_M9581_IG04 ``` to this assembly: ```asm popcnt edi, edi jg SHORT G_M000_IG04 ``` Note: this is not exactly as was intended in the issue, where a `jne` was expected. But that should be good enough. Note2: this does not affect `tnzcnt` or `lzcnt` instructions as these instructions do not modify the flags. Co-authored-by: Lotendan <letupertinois@hotmail.fr>
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
@@ -9196,28 +9196,37 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp)
             oper = GT_GE;
         }
     }
-    else if (cmp->IsUnsigned())
+    // Check for cases where the comparison is unsigned
+    // We can apply the same transformations if the comparison is signed but we know
+    // the left expression is never negative
+    else if (cmp->IsUnsigned() || op1->IsNeverNegative(this))
     {
         if ((oper == GT_LE) || (oper == GT_GT))
         {
             if (op2Value == 0)
             {
+                // Unsigned case:
                 // IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT
                 // recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails
                 // if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0)
                 // and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes
                 // occurs as a result of branch inversion.
+                // Non-neg case:
+                // ("expr > 0") equivalent to ("expr != 0")
+                // ("expr <= 0") equivalent to ("expr == 0")
                 oper = (oper == GT_LE) ? GT_EQ : GT_NE;
                 cmp->gtFlags &= ~GTF_UNSIGNED;
             }
             // LE_UN/GT_UN(expr, int/long.MaxValue) => GE/LT(expr, 0).
+            // LE/GT(non-negative expr, int/long.MaxValue) => GE/LT(expr, 0)
             else if (((op1->TypeIs(TYP_LONG) && (op2Value == INT64_MAX))) ||
                      ((genActualType(op1) == TYP_INT) && (op2Value == INT32_MAX)))
             {
                 oper = (oper == GT_LE) ? GT_GE : GT_LT;
                 cmp->gtFlags &= ~GTF_UNSIGNED;
             }
-            // LE_UN/GT_UN(expr, int.MaxValue) => EQ/NE(RSZ(expr, 32), 0).
+            // LE_UN/GT_UN(expr, uint.MaxValue) => EQ/NE(RSZ(expr, 32), 0).
+            // LE/GT(non-negative expr, uint.MaxValue) => EQ/NE(RSZ(expr, 32), 0).
             else if (opts.OptimizationEnabled() && (op1->TypeIs(TYP_LONG) && (op2Value == UINT_MAX)))
             {
                 oper            = (oper == GT_GT) ? GT_NE : GT_EQ;

Original file line number	Diff line number	Diff line change
`@@ -9196,28 +9196,37 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp)`
`9196`	`9196`	`oper = GT_GE;`
`9197`	`9197`	`}`
`9198`	`9198`	`}`
`9199`		`- else if (cmp->IsUnsigned())`
	`9199`	`+ // Check for cases where the comparison is unsigned`
	`9200`	`+ // We can apply the same transformations if the comparison is signed but we know`
	`9201`	`+ // the left expression is never negative`
	`9202`	`+ else if (cmp->IsUnsigned() \|\| op1->IsNeverNegative(this))`
`9200`	`9203`	`{`
`9201`	`9204`	`if ((oper == GT_LE) \|\| (oper == GT_GT))`
`9202`	`9205`	`{`
`9203`	`9206`	`if (op2Value == 0)`
`9204`	`9207`	`{`
	`9208`	`+ // Unsigned case:`
`9205`	`9209`	`// IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT`
`9206`	`9210`	`// recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails`
`9207`	`9211`	`// if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0)`
`9208`	`9212`	`// and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes`
`9209`	`9213`	`// occurs as a result of branch inversion.`
	`9214`	`+ // Non-neg case:`
	`9215`	`+ // ("expr > 0") equivalent to ("expr != 0")`
	`9216`	`+ // ("expr <= 0") equivalent to ("expr == 0")`
`9210`	`9217`	`oper = (oper == GT_LE) ? GT_EQ : GT_NE;`
`9211`	`9218`	`cmp->gtFlags &= ~GTF_UNSIGNED;`
`9212`	`9219`	`}`
`9213`	`9220`	`// LE_UN/GT_UN(expr, int/long.MaxValue) => GE/LT(expr, 0).`
	`9221`	`+ // LE/GT(non-negative expr, int/long.MaxValue) => GE/LT(expr, 0)`
`9214`	`9222`	`else if (((op1->TypeIs(TYP_LONG) && (op2Value == INT64_MAX))) \|\|`
`9215`	`9223`	`((genActualType(op1) == TYP_INT) && (op2Value == INT32_MAX)))`
`9216`	`9224`	`{`
`9217`	`9225`	`oper = (oper == GT_LE) ? GT_GE : GT_LT;`
`9218`	`9226`	`cmp->gtFlags &= ~GTF_UNSIGNED;`
`9219`	`9227`	`}`
`9220`		`- // LE_UN/GT_UN(expr, int.MaxValue) => EQ/NE(RSZ(expr, 32), 0).`
	`9228`	`+ // LE_UN/GT_UN(expr, uint.MaxValue) => EQ/NE(RSZ(expr, 32), 0).`
	`9229`	`+ // LE/GT(non-negative expr, uint.MaxValue) => EQ/NE(RSZ(expr, 32), 0).`
`9221`	`9230`	`else if (opts.OptimizationEnabled() && (op1->TypeIs(TYP_LONG) && (op2Value == UINT_MAX)))`
`9222`	`9231`	`{`
`9223`	`9232`	`oper = (oper == GT_GT) ? GT_NE : GT_EQ;`