From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Mon, 15 Nov 2021 22:42:51 +0000 (+0300) Subject: Delete impCheckForNullPointer (#61576) X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~12249 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2e97ac49fa77643c535b960476e24233e8d4685f;p=platform%2Fupstream%2Fdotnet%2Fruntime.git 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 --- 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 --- 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 --- 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 --- 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)