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

linker/inline: handle OpPhis. #960

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented Dec 5, 2022

This is a prerequisite for:

The problem this PR solves is that the inliner did not support "φ nodes" (OpPhi in SPIR-V), as they did not exist at the point where it was ran (i.e. all passes that would introduce OpPhis ran after the inliner).
This manifested as broken OpPhis, since the sources of control-flow edges (i.e. the side with the branch instruction) changed without the OpPhis in the destinations being updated to reflect the CFG change.

But with SPIR-T, we want the speedups that come from being able run the inliner after passes that create OpPhis (as the SPIR-T CFG structurizer can handle OpPhis just fine, unlike our existing one).
More specifically, in #940, we end up with two mem2reg passes, trying to do as much as possible before, and the rest after, inlining (though ideally they it should be interleaved with inlining, as per EmbarkStudios/spirt#6).

Thankfully most of the changes (in the big inline_block function) were only for readability, and the bulk of the additions just change some basic-block IDs in place, they don't do any complicated analysis or rewriting, so it felt straight-forward enough to have it in the existing inliner, instead of trying to replace it with a SPIR-T one.

@eddyb eddyb merged commit eea4485 into EmbarkStudios:main Dec 9, 2022
@eddyb eddyb deleted the linker-inline-phis branch December 9, 2022 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants