Skip to content

[release/10.0] Revert "JIT: Remove side effect detection quirk in loop hoisting (#118463)" with some improvements #119346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
agocke merged 5 commits into from
Sep 22, 2025
Merged

[release/10.0] Revert "JIT: Remove side effect detection quirk in loop hoisting (#118463)" with some improvements #119346

agocke merged 5 commits into from
Sep 22, 2025

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Sep 4, 2025 •
edited by jakobbotsch
Loading

Backport of #119138 to release/10.0

/cc @jakobbotsch

Customer Impact

  • Customer reported
  • Found internally

The JIT may hoist out expressions that throw exceptions from loops even if those expressions would not actually have been executed. This can result in exceptions being thrown in programs that should not throw any exceptions.

Regression

  • Yes
  • No

Introduced by #118463

Testing

Found by fuzzing. Regression test included.

Risk

Low.

@jakobbotsch
Revert "JIT: Remove side effect detection quirk in loop hoisting (#11…
249265a 
…8463)"

This reverts commit e0b14e9. That
change is not correct as it removes the safe guard that prevents
hoisting out of conditionally executed blocks.
@jakobbotsch
Add test
7e32891
@jakobbotsch
Rename m_beforeSideEffect -> m_canHoistSideEffects
35c1e3d
@jakobbotsch
Fix comment
426674d
@jakobbotsch
Move some code around
6f175f4
jeffschwMSFT
jeffschwMSFT approved these changes Sep 4, 2025
View reviewed changes
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. please get a code review. we can merge when ready

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Sep 4, 2025
@jeffschwMSFT jeffschwMSFT added this to the 10.0.0 milestone Sep 4, 2025
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Sep 4, 2025

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

@jeffschwMSFT
Copy link
Member

jeffschwMSFT commented Sep 14, 2025

@jakobbotsch please get a code review and we can merge

@agocke agocke merged commit 9749add into release/10.0 Sep 22, 2025
108 of 109 checks passed
@agocke agocke deleted the backport/pr-119138-to-release/10.0 branch September 22, 2025 23:02
@github-actions github-actions bot locked and limited conversation to collaborators Oct 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jeffschwMSFT jeffschwMSFT jeffschwMSFT approved these changes

@AndyAyersMS AndyAyersMS AndyAyersMS approved these changes

Assignees

No one assigned

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Servicing-approved Approved for servicing release

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffschwMSFT @AndyAyersMS @agocke @jakobbotsch