Skip to content
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

JIT: Remove bbFallsThrough restriction in fgOptimizeEmptyBlock #99634

Merged

Conversation

amanasifkhalid
Copy link
Member

Now that the JIT's implicit fallthrough assumption for certain block kinds is gone, we should start stripping out any logic dependent on bbFallsThrough. Note that fgReorderBlocks makes several decisions using bbFallsThrough -- I hope we can get rid of all of that in one go when we try a new block layout algorithm.

As for this change to fgOptimizeEmptyBlock, the pattern of always placing a BBJ_ALWAYS after a BBJ_COND to maintain implicit fallthrough is gone, so if we have a BBJ_COND block that "falls through" to an empty BBJ_ALWAYS that jumps somewhere else, this is logically equivalent to the BBJ_COND's false successor simply being the target of the BBJ_ALWAYS block, so we should have no problem removing the latter block. However, this creates churn during later optimization phases...

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 12, 2024
Copy link
Contributor

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

@AndyAyersMS
Copy link
Member

However, this creates churn during later optimization phases...

Not sure I understand. Are you saying you're not going as far as you wanted to with this change?

@amanasifkhalid
Copy link
Member Author

amanasifkhalid commented Mar 14, 2024

Not sure I understand. Are you saying you're not going as far as you wanted to with this change?

Sorry for the confusion, I didn't mean to imply I was holding back. With this change, we will now remove empty BBJ_ALWAYS blocks that succeed BBJ_COND blocks in the blocklist, as we no longer require implicit fallthrough between them. This change to the flowgraph isn't semantically significant, as bbFalseEdge models the new target the same way we previously modelled false jumps with fallthrough into a BBJ_ALWAYS. But like previous fallthrough removal changes, this change exposes fgReorderBlocks's sensitivity to these flowgraph modifications, as shown by the diffs.

@AndyAyersMS
Copy link
Member

At least superficially the diffs look great, including some nice TP wins.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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