Fold null checks against known non-null values (#109164)

dotnet · Feb 19, 2025 · 0f812b6 · 0f812b6
1 parent a127b3f
commit 0f812b6
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 35 deletions.
diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
@@ -863,6 +863,9 @@ bool Compiler::fgAddrCouldBeNull(GenTree* addr)
         case GT_ARR_ADDR:
             return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0;
 
+        case GT_BOX:
+            return !addr->IsBoxedValue();
+
         case GT_LCL_VAR:
             return !lvaIsImplicitByRefLocal(addr->AsLclVar()->GetLclNum());
 

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
@@ -14518,9 +14518,10 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
             // Optimize boxed value classes; these are always false.  This IL is
             // generated when a generic value is tested against null:
             //     <T> ... foo(T x) { ... if ((object)x == null) ...
-            if ((val == 0) && op->IsBoxedValue())
+            // Also fold checks against known non-null data like static readonlys
+            if ((val == 0) && !fgAddrCouldBeNull(op))
             {
-                JITDUMP("\nAttempting to optimize BOX(valueType) %s null [%06u]\n", GenTree::OpName(oper),
+                JITDUMP("\nAttempting to optimize BOX(valueType)/non-null %s null [%06u]\n", GenTree::OpName(oper),
                         dspTreeID(tree));
 
                 // We don't expect GT_GT with signed compares, and we
@@ -14532,44 +14533,46 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
                 }
                 else
                 {
-                    // The tree under the box must be side effect free
-                    // since we will drop it if we optimize.
-                    assert(!gtTreeHasSideEffects(op->AsBox()->BoxOp(), GTF_SIDE_EFFECT));
+                    bool wrapEffects = true;
+                    if (op->IsBoxedValue())
+                    {
+                        // The tree under the box must be side effect free
+                        // since we will drop it if we optimize.
+                        assert(!gtTreeHasSideEffects(op->AsBox()->BoxOp(), GTF_SIDE_EFFECT));
 
-                    // See if we can optimize away the box and related statements.
-                    GenTree* boxSourceTree = gtTryRemoveBoxUpstreamEffects(op);
-                    bool     didOptimize   = (boxSourceTree != nullptr);
+                        // See if we can optimize away the box and related statements.
+                        wrapEffects = (gtTryRemoveBoxUpstreamEffects(op) == nullptr);
+                    }
 
-                    // If optimization succeeded, remove the box.
-                    if (didOptimize)
+                    // Set up the result of the compare.
+                    int compareResult;
+                    if (oper == GT_GT)
                     {
-                        // Set up the result of the compare.
-                        int compareResult = 0;
-                        if (oper == GT_GT)
-                        {
-                            // GT_GT(null, box) == false
-                            // GT_GT(box, null) == true
-                            compareResult = (op1 == op);
-                        }
-                        else if (oper == GT_EQ)
-                        {
-                            // GT_EQ(box, null) == false
-                            // GT_EQ(null, box) == false
-                            compareResult = 0;
-                        }
-                        else
-                        {
-                            assert(oper == GT_NE);
-                            // GT_NE(box, null) == true
-                            // GT_NE(null, box) == true
-                            compareResult = 1;
-                        }
-
-                        JITDUMP("\nSuccess: replacing BOX(valueType) %s null with %d\n", GenTree::OpName(oper),
-                                compareResult);
+                        // GT_GT(null, op) == false
+                        // GT_GT(op, null) == true
+                        compareResult = (op1 == op);
+                    }
+                    else if (oper == GT_EQ)
+                    {
+                        // GT_EQ(op, null) == false
+                        // GT_EQ(null, op) == false
+                        compareResult = 0;
+                    }
+                    else
+                    {
+                        assert(oper == GT_NE);
+                        // GT_NE(op, null) == true
+                        // GT_NE(null, op) == true
+                        compareResult = 1;
+                    }
 
-                        return NewMorphedIntConNode(compareResult);
+                    GenTree* newTree = gtNewIconNode(compareResult);
+                    if (wrapEffects)
+                    {
+                        newTree = gtWrapWithSideEffects(newTree, op, GTF_ALL_EFFECT);
                     }
+                    op = newTree;
+                    goto DONE_FOLD;
                 }
             }
             else