Delete impCheckForNullPointer (#61576)

The comment above the method mentions "many problems" with leaving
null pointers around, but it is unclear what kind of problems. I can
only speculate those were the problems in legacy codegen which "could
not handle constant op1".

It also mentions that "we cannot even fold (null+offset)", which is
incorrect: "gtFoldExprConst" does in fact fold such expressions to
zero byrefs. It is also the case that spilling the null into a local
affects little as local assertion propagation happily propagates the
null back into its original place.

There was also a little bug associated with the method that got fixed:
morph was trying to use it, and in the process created uses of a local
that was not initialized, since the statement list used by the method
is the importer's one, invalid in morph.

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index d610fa0..51e07b1 100644 (file)
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -4610,7 +4610,6 @@ private:
 
     unsigned impInitBlockLineInfo();
 
-    GenTree* impCheckForNullPointer(GenTree* obj);
     bool impIsThis(GenTree* obj);
     bool impIsLDFTN_TOKEN(const BYTE* delegateCreateStart, const BYTE* newobjCodeAddr);
     bool impIsDUP_LDVIRTFTN_TOKEN(const BYTE* delegateCreateStart, const BYTE* newobjCodeAddr);

diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp
index b99e145..40a0907 100644 (file)
--- a/src/coreclr/jit/compiler.hpp
+++ b/src/coreclr/jit/compiler.hpp
@@ -3767,59 +3767,6 @@ inline bool Compiler::compIsProfilerHookNeeded()
 
 /*****************************************************************************
  *
- *  Check for the special case where the object is the constant 0.
- *  As we can't even fold the tree (null+fldOffs), we are left with
- *  op1 and op2 both being a constant. This causes lots of problems.
- *  We simply grab a temp and assign 0 to it and use it in place of the NULL.
- */
-
-inline GenTree* Compiler::impCheckForNullPointer(GenTree* obj)
-{
-    /* If it is not a GC type, we will be able to fold it.
-       So don't need to do anything */
-
-    if (!varTypeIsGC(obj->TypeGet()))
-    {
-        return obj;
-    }
-
-    if (obj->gtOper == GT_CNS_INT)
-    {
-        assert(obj->gtType == TYP_REF || obj->gtType == TYP_BYREF);
-
-        // We can see non-zero byrefs for RVA statics or for frozen strings.
-        if (obj->AsIntCon()->gtIconVal != 0)
-        {
-#ifdef DEBUG
-            if (!obj->TypeIs(TYP_BYREF))
-            {
-                assert(obj->TypeIs(TYP_REF));
-                assert(obj->IsIconHandle(GTF_ICON_STR_HDL));
-                if (!doesMethodHaveFrozenString())
-                {
-                    assert(compIsForInlining());
-                    assert(impInlineInfo->InlinerCompiler->doesMethodHaveFrozenString());
-                }
-            }
-#endif // DEBUG
-            return obj;
-        }
-
-        unsigned tmp = lvaGrabTemp(true DEBUGARG("CheckForNullPointer"));
-
-        // We don't need to spill while appending as we are only assigning
-        // NULL to a freshly-grabbed temp.
-
-        impAssignTempGen(tmp, obj, (unsigned)CHECK_SPILL_NONE);
-
-        obj = gtNewLclvNode(tmp, obj->gtType);
-    }
-
-    return obj;
-}
-
-/*****************************************************************************
- *
  *  Check for the special case where the object is the methods original 'this' pointer.
  *  Note that, the original 'this' pointer is always local var 0 for non-static method,
  *  even if we might have created the copy of 'this' pointer in lvaArg0Var.

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 9ab9838..d05d3d3 100644 (file)
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -12794,8 +12794,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     }
                 }
 
-                op1 = impCheckForNullPointer(op1);
-
                 /* Mark the block as containing an index expression */
 
                 if (op1->gtOper == GT_LCL_VAR)
@@ -13009,8 +13007,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     op2->gtType = TYP_I_IMPL;
                 }
 
-                op3 = impCheckForNullPointer(op3);
-
                 // Mark the block as containing an index expression
 
                 if (op3->gtOper == GT_LCL_VAR)
@@ -15220,8 +15216,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     case CORINFO_FIELD_INSTANCE_WITH_BASE:
 #endif
                     {
-                        obj = impCheckForNullPointer(obj);
-
                         // If the object is a struct, what we really want is
                         // for the field to operate on the address of the struct.
                         if (!varTypeGCtype(obj->TypeGet()) && impIsValueType(tiObj))
@@ -15550,8 +15544,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     case CORINFO_FIELD_INSTANCE_WITH_BASE:
 #endif
                     {
-                        obj = impCheckForNullPointer(obj);
-
                         /* Create the data member node */
                         op1             = gtNewFieldRef(lclTyp, resolvedToken.hField, obj, fieldInfo.offset);
                         DWORD typeFlags = info.compCompHnd->getClassAttribs(resolvedToken.hClass);

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index fb2fc2b..34205fd 100644 (file)
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -9278,10 +9278,8 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
             fgWalkTreePost(&value, resetMorphedFlag);
 #endif // DEBUG
 
-            GenTree* const nullCheckedArr = impCheckForNullPointer(arr);
-            GenTree* const arrIndexNode   = gtNewIndexRef(TYP_REF, nullCheckedArr, index);
-            GenTree* const arrStore       = gtNewAssignNode(arrIndexNode, value);
-            arrStore->gtFlags |= GTF_ASG;
+            GenTree* const arrIndexNode = gtNewIndexRef(TYP_REF, arr, index);
+            GenTree* const arrStore     = gtNewAssignNode(arrIndexNode, value);
 
             GenTree* result = fgMorphTree(arrStore);
             if (argSetup != nullptr)