Skip to content

Commit 9cbbc60

Browse files
EgorBoCopilot
andcommitted
Remove MorphAddrContext, use peel-offsets-style helper instead
Replace the threaded MorphAddrContext with fgMarkAddrModeForFieldAddr, a pre-walk over an indirection's address tree that peels constant offsets and chained instance FIELD_ADDRs to decide null-check elision. The new GTF_FLD_TGT_NONFAULTING flag on FIELD_ADDR communicates that the consuming indirection will fault on null, so the explicit null check inserted by morph can be elided. SPMI asmdiffs: no diffs across all 11 collections. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fcfb4ae commit 9cbbc60

4 files changed

Lines changed: 127 additions & 78 deletions

File tree

src/coreclr/jit/compiler.h

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6800,18 +6800,7 @@ class Compiler
68006800
GenTree* fgMorphIntoHelperCall(
68016801
GenTree* tree, int helper, bool morphArgs, GenTree* arg1 = nullptr, GenTree* arg2 = nullptr);
68026802

6803-
// A "MorphAddrContext" carries information from the surrounding context. If we are evaluating a byref address,
6804-
// it is useful to know whether the address will be immediately dereferenced, or whether the address value will
6805-
// be used, perhaps by passing it as an argument to a called method. This affects how null checking is done:
6806-
// for sufficiently small offsets, we can rely on OS page protection to implicitly null-check addresses that we
6807-
// know will be dereferenced. To know that reliance on implicit null checking is sound, we must further know that
6808-
// all offsets between the top-level indirection and the bottom are constant, and that their sum is sufficiently
6809-
// small; hence the other fields of MorphAddrContext.
6810-
struct MorphAddrContext
6811-
{
6812-
GenTreeIndir* m_user = nullptr; // Indirection using this address.
6813-
size_t m_totalOffset = 0; // Sum of offsets between the top-level indirection and here (current context).
6814-
};
6803+
void fgMarkAddrModeForFieldAddr(GenTreeIndir* indir);
68156804

68166805
#ifdef FEATURE_SIMD
68176806
GenTree* getSIMDStructFromField(GenTree* tree,
@@ -6846,8 +6835,8 @@ class Compiler
68466835
void fgAssignSetVarDef(GenTree* tree);
68476836

68486837
private:
6849-
GenTree* fgMorphFieldAddr(GenTree* tree, MorphAddrContext* mac);
6850-
GenTree* fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* mac);
6838+
GenTree* fgMorphFieldAddr(GenTree* tree);
6839+
GenTree* fgMorphExpandInstanceField(GenTree* tree);
68516840
GenTree* fgMorphExpandTlsFieldAddr(GenTree* tree);
68526841
bool fgCanFastTailCall(GenTreeCall* call, const char** failReason);
68536842
#if FEATURE_FASTTAILCALL
@@ -6902,7 +6891,7 @@ class Compiler
69026891
GenTree* fgMorphInitBlock(GenTree* tree);
69036892
GenTree* fgMorphCopyBlock(GenTree* tree);
69046893
private:
6905-
GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optAssertionPropDone = nullptr);
6894+
GenTree* fgMorphSmpOp(GenTree* tree, bool* optAssertionPropDone = nullptr);
69066895
bool fgTryReplaceStructLocalWithFields(GenTree** use);
69076896
GenTree* fgMorphFinalizeIndir(GenTreeIndir* indir);
69086897
GenTree* fgOptimizeCast(GenTreeCast* cast);
@@ -6937,7 +6926,7 @@ class Compiler
69376926
GenTree* fgMorphReduceAddOps(GenTree* tree);
69386927

69396928
public:
6940-
GenTree* fgMorphTree(GenTree* tree, MorphAddrContext* mac = nullptr);
6929+
GenTree* fgMorphTree(GenTree* tree);
69416930

69426931
private:
69436932
void fgAssertionGen(GenTree* tree);

src/coreclr/jit/gentree.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,8 @@ enum GenTreeFlags : unsigned
469469

470470
GTF_FLD_TLS = 0x80000000, // GT_FIELD_ADDR -- field address is a Windows x86 TLS reference
471471
GTF_FLD_DEREFERENCED = 0x40000000, // GT_FIELD_ADDR -- used to preserve previous behavior
472+
GTF_FLD_TGT_NONFAULTING = 0x20000000, // GT_FIELD_ADDR -- the consuming indirection will fault on null,
473+
// so the explicit null check inserted by morph can be elided.
472474

473475
GTF_INX_RNGCHK = 0x80000000, // GT_INDEX_ADDR -- this array address should be range-checked
474476
GTF_INX_ADDR_NONNULL = 0x40000000, // GT_INDEX_ADDR -- this array address is not null

src/coreclr/jit/morph.cpp

Lines changed: 95 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3527,19 +3527,90 @@ unsigned Compiler::fgGetFieldMorphingTemp(GenTreeFieldAddr* fieldNode)
35273527
return lclNum;
35283528
}
35293529

3530+
//------------------------------------------------------------------------
3531+
// fgMarkAddrModeForFieldAddr: Walk an indirection's unmorphed address tree to
3532+
// identify FIELD_ADDR nodes whose explicit null check can be elided because
3533+
// this indirection will fault on null instead.
3534+
//
3535+
// Replaces the legacy "MorphAddrContext" thread-through mechanism: peels
3536+
// constant offsets off ADD nodes (via gtPeelOffsets) and recurses through
3537+
// chained instance FIELD_ADDR nodes, accumulating the cumulative constant
3538+
// offset that would be added on top of the FIELD_ADDR's base object.
3539+
//
3540+
// For each instance FIELD_ADDR whose base object could be null:
3541+
// * If the cumulative offset (including this field's offset) is small enough
3542+
// that the OS page guard catches the access, mark the FIELD_ADDR with
3543+
// GTF_FLD_TGT_NONFAULTING so morphing can elide the explicit null check,
3544+
// and clear GTF_IND_NONFAULTING on the consuming indirection so it faults
3545+
// on null instead.
3546+
// * Otherwise, the FIELD_ADDR will introduce an explicit NULLCHECK and stop
3547+
// propagation: mark the indirection as having an ordering side effect (so
3548+
// downstream optimizations treat the inserted NULLCHECK conservatively),
3549+
// and stop walking past this FIELD_ADDR.
3550+
//
3551+
// Arguments:
3552+
// indir - The indirection whose address operand will be morphed.
3553+
//
3554+
void Compiler::fgMarkAddrModeForFieldAddr(GenTreeIndir* indir)
3555+
{
3556+
GenTree* addr = indir->Addr();
3557+
ClrSafeInt<target_ssize_t> total((target_ssize_t)0);
3558+
3559+
while (true)
3560+
{
3561+
// Peel constant offsets off ADD/LEA nodes. gtPeelOffsets stops cleanly on
3562+
// its own overflow; ClrSafeInt is sticky, so we only need a single check
3563+
// before consuming the accumulated offset below.
3564+
target_ssize_t peeled = 0;
3565+
gtPeelOffsets(&addr, &peeled);
3566+
total += peeled;
3567+
3568+
if (!addr->OperIs(GT_FIELD_ADDR) || !addr->AsFieldAddr()->IsInstance())
3569+
{
3570+
return;
3571+
}
3572+
3573+
GenTreeFieldAddr* fld = addr->AsFieldAddr();
3574+
total += ClrSafeInt<target_ssize_t>(fld->gtFldOffset);
3575+
if (total.IsOverflow())
3576+
{
3577+
return;
3578+
}
3579+
3580+
GenTree* objRef = fld->GetFldObj();
3581+
if (fgAddrCouldBeNull(objRef))
3582+
{
3583+
target_ssize_t accessOffset = total.Value();
3584+
if ((accessOffset < 0) || fgIsBigOffset((size_t)accessOffset))
3585+
{
3586+
// The FIELD_ADDR will introduce an explicit NULLCHECK, which acts as a
3587+
// barrier in the consuming indirection's address chain.
3588+
indir->SetHasOrderingSideEffect();
3589+
return;
3590+
}
3591+
3592+
fld->gtFlags |= GTF_FLD_TGT_NONFAULTING;
3593+
// We can elide the null check only by letting it happen as part of the
3594+
// consuming indirection, so it is no longer non-faulting.
3595+
indir->gtFlags &= ~GTF_IND_NONFAULTING;
3596+
}
3597+
3598+
addr = objRef;
3599+
}
3600+
}
3601+
35303602
//------------------------------------------------------------------------
35313603
// fgMorphFieldAddr: Fully morph a FIELD_ADDR tree.
35323604
//
35333605
// Expands the field node into explicit additions.
35343606
//
35353607
// Arguments:
35363608
// tree - The FIELD_ADDR tree
3537-
// mac - The morphing context, used to elide adding null checks
35383609
//
35393610
// Return Value:
35403611
// The fully morphed "tree".
35413612
//
3542-
GenTree* Compiler::fgMorphFieldAddr(GenTree* tree, MorphAddrContext* mac)
3613+
GenTree* Compiler::fgMorphFieldAddr(GenTree* tree)
35433614
{
35443615
assert(tree->OperIs(GT_FIELD_ADDR));
35453616

@@ -3549,7 +3620,7 @@ GenTree* Compiler::fgMorphFieldAddr(GenTree* tree, MorphAddrContext* mac)
35493620

35503621
if (fieldNode->IsInstance())
35513622
{
3552-
tree = fgMorphExpandInstanceField(tree, mac);
3623+
tree = fgMorphExpandInstanceField(tree);
35533624
}
35543625
else if (fieldNode->IsTlsStatic())
35553626
{
@@ -3560,11 +3631,10 @@ GenTree* Compiler::fgMorphFieldAddr(GenTree* tree, MorphAddrContext* mac)
35603631
assert(!"Normal statics are expected to be handled in the importer");
35613632
}
35623633

3563-
// Pass down the current mac; if non null we are computing an address
35643634
GenTree* result;
35653635
if (tree->OperIsSimple())
35663636
{
3567-
result = fgMorphSmpOp(tree, mac);
3637+
result = fgMorphSmpOp(tree);
35683638
result->SetMorphed(this);
35693639

35703640
// Quirk: preserve previous behavior with this NO_CSE.
@@ -3575,7 +3645,7 @@ GenTree* Compiler::fgMorphFieldAddr(GenTree* tree, MorphAddrContext* mac)
35753645
}
35763646
else
35773647
{
3578-
result = fgMorphTree(tree, mac);
3648+
result = fgMorphTree(tree);
35793649
}
35803650

35813651
JITDUMP("\nFinal value of Compiler::fgMorphFieldAddr after morphing:\n");
@@ -3591,12 +3661,11 @@ GenTree* Compiler::fgMorphFieldAddr(GenTree* tree, MorphAddrContext* mac)
35913661
//
35923662
// Arguments:
35933663
// tree - The FIELD_ADDR tree
3594-
// mac - The morphing context, used to elide adding null checks
35953664
//
35963665
// Return Value:
35973666
// The expanded "tree" of an arbitrary shape.
35983667
//
3599-
GenTree* Compiler::fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* mac)
3668+
GenTree* Compiler::fgMorphExpandInstanceField(GenTree* tree)
36003669
{
36013670
assert(tree->OperIs(GT_FIELD_ADDR) && tree->AsFieldAddr()->IsInstance());
36023671

@@ -3673,29 +3742,18 @@ GenTree* Compiler::fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* m
36733742

36743743
if (fgAddrCouldBeNull(objRef))
36753744
{
3676-
// A non-null context here implies our [+ some offset] parent is an indirection, one that
3677-
// will implicitly null-check the produced address.
3678-
addExplicitNullCheck = (mac == nullptr) || fgIsBigOffset(mac->m_totalOffset + fieldOffset);
3745+
// The pre-walk performed by fgMarkAddrModeForFieldAddr sets GTF_FLD_TGT_NONFAULTING when
3746+
// the consuming indirection will implicitly null-check the produced address (the cumulative
3747+
// constant offset down to this FIELD_ADDR is small enough that the OS page guard catches it).
3748+
addExplicitNullCheck = (tree->gtFlags & GTF_FLD_TGT_NONFAULTING) == 0;
36793749

36803750
// The transformation here turns a value dependency (FIELD_ADDR being a
36813751
// known non-null operand) into a control-flow dependency (introducing
36823752
// explicit COMMA(NULLCHECK, ...)). This effectively "disconnects" the
3683-
// null check from the parent of the FIELD_ADDR node. For the cases
3684-
// where we made use of non-nullness we need to make the dependency
3685-
// explicit now.
3686-
if (addExplicitNullCheck)
3687-
{
3688-
if (mac != nullptr)
3689-
{
3690-
mac->m_user->SetHasOrderingSideEffect();
3691-
}
3692-
}
3693-
else
3694-
{
3695-
// We can elide the null check only by letting it happen as part of
3696-
// the consuming indirection, so it is no longer non-faulting.
3697-
mac->m_user->gtFlags &= ~GTF_IND_NONFAULTING;
3698-
}
3753+
// null check from the parent of the FIELD_ADDR node.
3754+
//
3755+
// For the elided case, the consuming indirection has already been marked
3756+
// (~GTF_IND_NONFAULTING) by fgMarkAddrModeForFieldAddr; nothing else to do here.
36993757
}
37003758

37013759
if (addExplicitNullCheck)
@@ -6920,14 +6978,13 @@ GenTreeOp* Compiler::fgMorphCommutative(GenTreeOp* tree)
69206978
//
69216979
// Arguments:
69226980
// tree - tree to morph
6923-
// mac - address context for morphing
69246981
// optAssertionPropDone - [out, optional] set true if local assertions
69256982
// were killed/genned while morphing this tree
69266983
//
69276984
// Returns:
69286985
// Tree, possibly updated
69296986
//
6930-
GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optAssertionPropDone)
6987+
GenTree* Compiler::fgMorphSmpOp(GenTree* tree, bool* optAssertionPropDone)
69316988
{
69326989
assert(tree->OperKind() & GTK_SMPOP);
69336990

@@ -7011,7 +7068,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
70117068
break;
70127069

70137070
case GT_FIELD_ADDR:
7014-
return fgMorphFieldAddr(tree, mac);
7071+
return fgMorphFieldAddr(tree);
70157072

70167073
case GT_INDEX_ADDR:
70177074
return fgMorphIndexAddr(tree->AsIndexAddr());
@@ -7140,7 +7197,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
71407197
{
71417198
assert(tree->OperIs(GT_DIV));
71427199
tree->ChangeOper(GT_UDIV, GenTree::PRESERVE_VN);
7143-
return fgMorphSmpOp(tree, mac);
7200+
return fgMorphSmpOp(tree);
71447201
}
71457202

71467203
#if !defined(TARGET_64BIT) && !defined(TARGET_WASM)
@@ -7207,7 +7264,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
72077264
{
72087265
assert(tree->OperIs(GT_MOD));
72097266
tree->ChangeOper(GT_UMOD, GenTree::PRESERVE_VN);
7210-
return fgMorphSmpOp(tree, mac);
7267+
return fgMorphSmpOp(tree);
72117268
}
72127269

72137270
// Do not use optimizations (unlike UMOD's idiv optimizing during codegen) for signed mod.
@@ -7533,34 +7590,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
75337590
// the other operands in case this is a store. However, doing so unconditionally preserves previous
75347591
// behavior and "fixes up" field store importation that places the null check in the wrong location
75357592
// (before the 'value' operand is evaluated).
7536-
MorphAddrContext indMac;
75377593
if (tree->OperIsIndir() && !tree->OperIsAtomicOp())
75387594
{
7539-
// Communicate to FIELD_ADDR morphing that the parent is an indirection.
7540-
indMac.m_user = tree->AsIndir();
7541-
mac = &indMac;
7542-
}
7543-
// For additions, if we already have a context, keep track of whether all offsets added
7544-
// to the address are constant, and their sum does not overflow.
7545-
else if ((mac != nullptr) && tree->OperIs(GT_ADD) && op2->IsCnsIntOrI())
7546-
{
7547-
ClrSafeInt<size_t> offset(mac->m_totalOffset);
7548-
offset += op2->AsIntCon()->IconValue();
7549-
if (!offset.IsOverflow())
7550-
{
7551-
mac->m_totalOffset = offset.Value();
7552-
}
7553-
else
7554-
{
7555-
mac = nullptr;
7556-
}
7557-
}
7558-
else // Reset the context.
7559-
{
7560-
mac = nullptr;
7595+
// Examine the unmorphed address tree to identify any FIELD_ADDR whose null check
7596+
// can be elided because this indirection will fault on null instead.
7597+
fgMarkAddrModeForFieldAddr(tree->AsIndir());
75617598
}
75627599

7563-
tree->AsOp()->gtOp1 = op1 = fgMorphTree(op1, mac);
7600+
tree->AsOp()->gtOp1 = op1 = fgMorphTree(op1);
75647601

75657602
// If we are exiting the "then" part of a Qmark-Colon we must
75667603
// save the state of the current assertions table so that we
@@ -12448,7 +12485,7 @@ GenTreeOp* Compiler::fgMorphLongMul(GenTreeOp* mul)
1244812485
* Transform the given tree for code generation and return an equivalent tree.
1244912486
*/
1245012487

12451-
GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac)
12488+
GenTree* Compiler::fgMorphTree(GenTree* tree)
1245212489
{
1245312490
assert(tree);
1245412491
tree->ClearMorphed();
@@ -12558,7 +12595,7 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac)
1255812595

1255912596
if (kind & GTK_SMPOP)
1256012597
{
12561-
tree = fgMorphSmpOp(tree, mac, &optAssertionPropDone);
12598+
tree = fgMorphSmpOp(tree, &optAssertionPropDone);
1256212599
goto DONE;
1256312600
}
1256412601

0 commit comments

Comments
 (0)