Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 8936d23

Browse files
committed
Jit: fix unnecessary null checks in some field accesses
Morph was sometimes passing the existing MorphAddressContext down to fgMorphField even when the field access was for a field value. If that context contained indefinite offsets, morph would then insert an explicit null check on the object pointer for the field access. Typically the field offset is small enough that this explicit check is not needed. The implicit check done when fetching the field's value is sufficient protection. The fix is to have `fgMorphSmpOp` clear out the context for child `GT_FIELD` nodes, unless the field parent is a `GT_ADDR`. Note if there is an `op2` node the parent cannot be `GT_ADDR` so these field children always get an empty context. No tests added since this kicks in reasonably frequently in corelib and elsewhere in frameworks. Closes #10942.
1 parent 2030aff commit 8936d23

File tree

1 file changed

+25
-1
lines changed

1 file changed

+25
-1
lines changed

src/jit/morph.cpp

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11416,6 +11416,20 @@ GenTreePtr Compiler::fgMorphSmpOp(GenTreePtr tree, MorphAddrContext* mac)
1141611416
}
1141711417
}
1141811418

11419+
// If gtOp1 is a GT_FIELD, we need to pass down the mac if
11420+
// its parent is GT_ADDR, since the address of the field
11421+
// is part of an ongoing address computation. Otherwise
11422+
// op1 represents the value of the field and so any address
11423+
// calculations it does are in a new context.
11424+
if ((op1->gtOper == GT_FIELD) && (tree->gtOper != GT_ADDR))
11425+
{
11426+
subMac1 = nullptr;
11427+
11428+
// The impact of this field's value to any ongoing
11429+
// address computation is handled below when looking
11430+
// at op2.
11431+
}
11432+
1141911433
tree->gtOp.gtOp1 = op1 = fgMorphTree(op1, subMac1);
1142011434

1142111435
#if LOCAL_ASSERTION_PROP
@@ -11496,7 +11510,6 @@ GenTreePtr Compiler::fgMorphSmpOp(GenTreePtr tree, MorphAddrContext* mac)
1149611510
// (These are used to convey parent context about how addresses being calculated
1149711511
// will be used; see the specification comment for MorphAddrContext for full details.)
1149811512
// Assume it's an Ind context to start.
11499-
MorphAddrContext subIndMac2(MACK_Ind);
1150011513
switch (tree->gtOper)
1150111514
{
1150211515
case GT_ADD:
@@ -11517,6 +11530,17 @@ GenTreePtr Compiler::fgMorphSmpOp(GenTreePtr tree, MorphAddrContext* mac)
1151711530
default:
1151811531
break;
1151911532
}
11533+
11534+
// If gtOp2 is a GT_FIELD, we must be taking its value,
11535+
// so it should evaluate its address in a new context.
11536+
if (op2->gtOper == GT_FIELD)
11537+
{
11538+
// The impact of this field's value to any ongoing
11539+
// address computation is handled above when looking
11540+
// at op1.
11541+
mac = nullptr;
11542+
}
11543+
1152011544
tree->gtOp.gtOp2 = op2 = fgMorphTree(op2, mac);
1152111545

1152211546
/* Propagate the side effect flags from op2 */

0 commit comments

Comments
 (0)