Skip to content

JIT: fix slow preheader EH bookkeeping in loop cloning#128963

Open
AndyAyersMS wants to merge 1 commit into
dotnet:mainfrom
AndyAyersMS:JIT-fix-loop-clone-EH-bookkeeping
Open

JIT: fix slow preheader EH bookkeeping in loop cloning#128963
AndyAyersMS wants to merge 1 commit into
dotnet:mainfrom
AndyAyersMS:JIT-fix-loop-clone-EH-bookkeeping

Conversation

@AndyAyersMS
Copy link
Copy Markdown
Member

Fix a bug where loop cloning might improperly extend EH regions. This doesn't seem to be reachable with current cloning but I was running into it with some prototypes that did more general loop unswitching.

When the loop's lex-bottom is in a different EH region than the preheader,
`fgFindInsertPoint` returns a different `beforeSlowPreheader` block. The
subsequent loop comparing `ebdTryLast`/`ebdHndLast` against `bottom`
never matches, so no enclosing region gets extended to cover the slow
preheader. `DuplicateWithEH` then resets (rather than extends) the
enclosing `tryLast`/`hndLast` and the cloned inner try ends up beyond the
outer try's `tryLast`, tripping `fgVerifyHandlerTab`.

Compare against `beforeSlowPreheader` instead. `EHClauses(this, ebd)`
starting from the preheader's most-nested enclosing region only visits
enclosing or disjoint regions; disjoint regions can't have a `Last` equal
to `beforeSlowPreheader`, so the simple `==` check is safe.

This is mostly latent in production thanks to `optIsLoopClonable`
strictness, but I hit it from a loop unswitching prototype.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 22:17
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 3, 2026
@AndyAyersMS
Copy link
Copy Markdown
Member Author

@jakobbotsch PTAL
fyi @dotnet/jit-contrib

no diffs expected

Copy link
Copy Markdown
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 adjusts loop cloning’s EH-region extent bookkeeping when inserting the slow-path preheader so that any EH clauses ending at the actual insertion point (beforeSlowPreheader) are extended, preventing inadvertent EH region mis-sizing when the insertion point differs from the loop’s lexical bottom.

Changes:

  • Update EH extent updates to key off beforeSlowPreheader (the actual insertion point) rather than bottom.
  • Add clarifying comments explaining why enclosing EH regions may need to be extended before DuplicateWithEH.

@AndyAyersMS
Copy link
Copy Markdown
Member Author

Here's what my local AI has to say:

The mismatched operand types come from the JIT's implicit-byref struct-argument rewrite. Import starts with the IL for if (&p == null):

ldarga.s 1
conv.u
ldc.i4.0
conv.u
bne.un.s

which initially imports as:

NE int
  LCL_ADDR long V01 arg1
  CAST long <- ulong <- uint
    CNS_INT int 0

Later, Morph - ByRefs changes the small struct argument p to an implicit byref:

Changing the lvType for struct parameter V01 to TYP_BYREF.

and global morph rewrites the address node:

LCL_ADDR long V01 arg1

into:

LCL_VAR byref V01 arg1

The zero side remains a CAST long <- ulong <- uint, so the compare reaches fgOptimizeRelationalComparisonWithCasts as a byref-vs-long/null comparison. That path was not exercised for EQ/NE until #128091 enabled this optimization for equality comparisons.


There is some logic in the stack allocation pass (which also rewrites pointer types) to try and make compensating changes for compares. We could do something like that here, or just bail on this case entirely.

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.

2 participants