diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index b7c9a68289241..13a76d6e22a23 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -10312,8 +10312,7 @@ void Compiler::gtChangeOperToNullCheck(GenTree* tree, BasicBlock* block) assert(tree->OperIs(GT_FIELD, GT_IND, GT_BLK)); tree->ChangeOper(GT_NULLCHECK); tree->ChangeType(gtTypeForNullCheck(tree)); - assert(fgAddrCouldBeNull(tree->gtGetOp1())); - tree->gtFlags |= GTF_EXCEPT; + tree->SetIndirExceptionFlags(this); block->bbFlags |= BBF_HAS_NULLCHECK; optMethodFlags |= OMF_HAS_NULLCHECK; } diff --git a/src/coreclr/jit/earlyprop.cpp b/src/coreclr/jit/earlyprop.cpp index ab858d96fb870..f1822a9a1cb89 100644 --- a/src/coreclr/jit/earlyprop.cpp +++ b/src/coreclr/jit/earlyprop.cpp @@ -313,7 +313,7 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck return tree; } - return nullptr; + return folded ? tree : nullptr; } //------------------------------------------------------------------------------------------- diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 38b5e3bd82133..0312e63b24a4b 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -899,6 +899,9 @@ bool Compiler::fgAddrCouldBeNull(GenTree* addr) case GT_COMMA: return fgAddrCouldBeNull(addr->AsOp()->gtOp2); + case GT_CALL: + return !addr->IsHelperCall() || !s_helperCallProperties.NonNullReturn(addr->AsCall()->GetHelperNum()); + case GT_ADD: if (addr->AsOp()->gtOp1->gtOper == GT_CNS_INT) { diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 3d2748659ef18..52a96961cd8b6 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2391,6 +2391,18 @@ bool GenTreeCall::IsHelperCall(Compiler* compiler, unsigned helper) const return IsHelperCall(compiler->eeFindHelper(helper)); } +//------------------------------------------------------------------------- +// GetHelperNum: Get the helper identifier for this call. +// +// Return Value: +// CORINFO_HELP_UNDEF if this call is not a helper call, appropriate +// CorInfoHelpFunc otherwise. +// +CorInfoHelpFunc GenTreeCall::GetHelperNum() const +{ + return IsHelperCall() ? Compiler::eeGetHelperNum(gtCallMethHnd) : CORINFO_HELP_UNDEF; +} + //-------------------------------------------------------------------------- // Equals: Check if 2 CALL nodes are equal. // @@ -16556,23 +16568,11 @@ void Compiler::gtExtractSideEffList(GenTree* expr, { if (node->OperIsBlk() && !node->OperIsStoreBlk()) { - // Check for a guaranteed non-faulting IND, and create a NOP node instead of a NULLCHECK in that - // case. - if (m_compiler->fgAddrCouldBeNull(node->AsBlk()->Addr())) - { - Append(node); - JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NULLCHECK\n", dspTreeID(node)); - m_compiler->gtChangeOperToNullCheck(node, m_compiler->compCurBB); - } - else - { - JITDUMP("Dropping non-faulting OBJ/BLK node [%06d]\n", dspTreeID(node)); - } - } - else - { - Append(node); + JITDUMP("Replace an unused BLK node [%06d] with a NULLCHECK\n", dspTreeID(node)); + m_compiler->gtChangeOperToNullCheck(node, m_compiler->compCurBB); } + + Append(node); return Compiler::WALK_SKIP_SUBTREES; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 0a0669b0cec20..8a159f6c9a6d1 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -5561,6 +5561,8 @@ struct GenTreeCall final : public GenTree bool IsHelperCall(Compiler* compiler, unsigned helper) const; + CorInfoHelpFunc GetHelperNum() const; + bool AreArgsComplete() const; CorInfoCallConvExtension GetUnmanagedCallConv() const diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 719736da37872..8a09fbb5a6bf4 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -8336,15 +8336,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) // via an underlying address, just null check the address. if (op1->OperIs(GT_FIELD, GT_IND, GT_BLK)) { - GenTree* addr = op1->gtGetOp1(); - if ((addr != nullptr) && fgAddrCouldBeNull(addr)) - { - gtChangeOperToNullCheck(op1, block); - } - else - { - op1 = gtNewNothingNode(); - } + gtChangeOperToNullCheck(op1, block); } else { diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 44dcc3d89a3cb..b2e80b4ecfc27 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7957,16 +7957,13 @@ void Lowering::LowerIndir(GenTreeIndir* ind) #if defined(TARGET_ARM64) // Verify containment safety before creating an LEA that must be contained. // - const bool isContainable = (ind->Addr() != nullptr) && IsInvariantInRange(ind->Addr(), ind); + const bool isContainable = IsInvariantInRange(ind->Addr(), ind); #else const bool isContainable = true; #endif - if (!ind->OperIs(GT_NOP)) - { - TryCreateAddrMode(ind->Addr(), isContainable, ind); - ContainCheckIndir(ind); - } + TryCreateAddrMode(ind->Addr(), isContainable, ind); + ContainCheckIndir(ind); #ifdef TARGET_XARCH if (ind->OperIs(GT_NULLCHECK) || ind->IsUnusedValue()) @@ -8014,15 +8011,6 @@ void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, Bas // assert(ind->OperIs(GT_NULLCHECK, GT_IND, GT_BLK)); - GenTree* const addr = ind->Addr(); - if (!comp->fgAddrCouldBeNull(addr)) - { - addr->SetUnusedValue(); - ind->gtBashToNOP(); - JITDUMP("bash an unused indir [%06u] to NOP.\n", comp->dspTreeID(ind)); - return; - } - ind->ChangeType(comp->gtTypeForNullCheck(ind)); #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) @@ -8030,7 +8018,7 @@ void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, Bas #elif defined(TARGET_ARM) bool useNullCheck = false; #else // TARGET_XARCH - bool useNullCheck = !addr->isContained(); + bool useNullCheck = !ind->Addr()->isContained(); ind->ClearDontExtend(); #endif // !TARGET_XARCH diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f78b23c963a8b..c9a552ab132ef 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9096,24 +9096,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA break; #endif - case GT_NULLCHECK: - { - op1 = tree->AsUnOp()->gtGetOp1(); - if (op1->IsCall()) - { - GenTreeCall* const call = op1->AsCall(); - if (call->IsHelperCall() && s_helperCallProperties.NonNullReturn(eeGetHelperNum(call->gtCallMethHnd))) - { - JITDUMP("\nNULLCHECK on [%06u] will always succeed\n", dspTreeID(call)); - - // TODO: Can we also remove the call? - // - return fgMorphTree(call); - } - } - } - break; - default: break; } @@ -9923,6 +9905,23 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } break; + case GT_NULLCHECK: + if (opts.OptimizationEnabled() && !optValnumCSE_phase && !tree->OperMayThrow(this)) + { + JITDUMP("\nNULLCHECK on [%06u] will always succeed\n", dspTreeID(op1)); + if ((op1->gtFlags & GTF_SIDE_EFFECT) != 0) + { + tree = gtUnusedValNode(op1); + INDEBUG(tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + } + else + { + tree->gtBashToNOP(); + } + return tree; + } + break; + case GT_COLON: if (fgGlobalMorph) { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_84522/Runtime_84522.il b/src/tests/JIT/Regression/JitBlue/Runtime_84522/Runtime_84522.il new file mode 100644 index 0000000000000..1c94c70f521ab --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_84522/Runtime_84522.il @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime { } +.assembly extern xunit.core { } +.assembly Runtime_84522 { } + +.class sealed StructWithInt extends [System.Runtime]System.ValueType +{ + .field public int32 Value +} + +.class Runtime_84522 extends [System.Runtime]System.Object +{ + .method public static int32 Main() + { + .custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = ( + 01 00 00 00 + ) + .entrypoint + + .try + { + ldc.i4 0 + newarr StructWithInt + call void .this::UnusedLdelem(valuetype StructWithInt[]) + leave FAIL + } + catch [System.Runtime]System.IndexOutOfRangeException + { + pop + leave SUCCESS + } + + FAIL: + ldc.i4 0 + ret + + SUCCESS: + ldc.i4 100 + ret + } + + .method private static void UnusedLdelem(valuetype StructWithInt[] arr) noinlining + { + ldarg arr + ldc.i4 0 + ldelem valuetype StructWithInt + pop + ret + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_84522/Runtime_84522.ilproj b/src/tests/JIT/Regression/JitBlue/Runtime_84522/Runtime_84522.ilproj new file mode 100644 index 0000000000000..4b3a54c1abb8a --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_84522/Runtime_84522.ilproj @@ -0,0 +1,9 @@ + + + None + True + + + + +