Skip to content

JIT: Allow forward subbing no-return calls #115976

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.

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

Merged
merged 2 commits into from
May 28, 2025

Conversation

jakobbotsch
Copy link
Member

The comment refers to morph getting tripped up, but that should not be the case anymore after #105942.

The comment refers to morph getting tripped up, but that should not be
the case anymore after 24786d2.
@Copilot Copilot AI review requested due to automatic review settings May 25, 2025 14:41
@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 May 25, 2025
Copy link
Contributor

@Copilot 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 the forward substitution logic to also block GT_ASYNC_CONTINUATION nodes and removes the special-case guard against no-return calls, allowing them to be substituted.

  • Added GT_ASYNC_CONTINUATION to the substitution guard.
  • Removed the no-return call check so forward subbing will include calls marked no-return.
  • Updated the guard comment to reflect the new set of blocked node types.
Comments suppressed due to low confidence (2)

src/coreclr/jit/forwardsub.cpp:505

  • Update the JITDUMP message to include async continuations (e.g., 'tree to sub is catch arg, lcl heap, or async continuation') so the debug output matches the new guard comment.
JITDUMP(" tree to sub is catch arg, or lcl heap\n");

src/coreclr/jit/forwardsub.cpp:508

  • Add a unit or integration test case to verify that forward substitution now correctly handles no-return calls, since the previous guard was removed.
// Do not substitute async calls; if the target node has a temp BYREF node,

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

We should be more aggressive in tree pruning when we see no return calls, but we should do that sometime earlier.

@jakobbotsch
Copy link
Member Author

/ba-g Timeout is #115955

@jakobbotsch jakobbotsch merged commit 49387ef into dotnet:main May 28, 2025
106 of 108 checks passed
@jakobbotsch jakobbotsch deleted the forward-sub-noreturn-calls branch May 28, 2025 10:05
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