Skip to content

Fix potential fgIsBigOffset issue in optAssertionIsNonNull#124289

Draft
EgorBo wants to merge 1 commit intodotnet:mainfrom
EgorBo:fix-optAssertionIsNonNull
Draft

Fix potential fgIsBigOffset issue in optAssertionIsNonNull#124289
EgorBo wants to merge 1 commit intodotnet:mainfrom
EgorBo:fix-optAssertionIsNonNull

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 11, 2026

Currently in optAssertionIsNonNull we start from peeling the offset from GenTree:

bool Compiler::optAssertionIsNonNull(GenTree* op, ASSERT_VALARG_TP assertions)
{
if (op->OperIs(GT_ADD) && op->AsOp()->gtGetOp2()->IsCnsIntOrI() &&
!fgIsBigOffset(op->AsOp()->gtGetOp2()->AsIntCon()->IconValue()))
{
op = op->AsOp()->gtGetOp1();
}

Then, for Global-AP we peel it again on top of VN level:

ValueNum vnBase = vn;
target_ssize_t offset = 0;
vnStore->PeelOffsets(&vnBase, &offset);

but this time we don't check the offset with fgIsBigOffset (in fact, we should check the sum of both offsets).

Instead, this PR only does VN-level peeling for global AP and uses a special gtPeelOffset helper for the local one.
Also, removed unsound use of gtEffectiveVal

Copilot AI review requested due to automatic review settings February 11, 2026 18:49
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 11, 2026
@EgorBo EgorBo added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 11, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates CoreCLR JIT assertion propagation to avoid double-peeling offsets (tree-level + VN-level) in optAssertionIsNonNull, ensuring fgIsBigOffset is applied to the combined peeled offset and removing reliance on gtEffectiveVal for this analysis.

Changes:

  • Switch global assertion-prop non-null checks to VN-only offset peeling with an explicit fgIsBigOffset guard.
  • Switch local assertion-prop non-null checks to use gtPeelOffsets to compute the full summed offset before applying fgIsBigOffset.
  • Remove the prior gtEffectiveVal-based restriction so global AP can reason about non-lclvar trees via VN.

Copilot AI review requested due to automatic review settings February 11, 2026 18:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant