Reuse code for Metal and WGSL entry point legalization #6063
Conversation
fairywreath
changed the title
|
I recommend comparing/diffing the (old) individual implementations of the legalization in their separate files with the new unified implementation to make the review process easier. |
| #include "slang-ir-clone.h" | ||
| #include "slang-ir-insts.h" | ||
| #include "slang-ir-util.h" | ||
| #include "slang-parameter-binding.h" | ||
|
|
||
| #include <set> |
There was a problem hiding this comment.
this shouldn't be needed?
There was a problem hiding this comment.
std::set is actually used and was added here #4536. Looking at that PR's commit history, A custom TreeMap/TreeSet was considered but std::set ended up being used.
Yes this is doable. Should I modify this PR or have these changes as a follow-up? I initially intended the unified implementation as close to the original ones as possible so the code changes are easy to see and so things don't break, but now that the checks all passed it should be easy to further improve the code. |
I will update this PR - it should be easy to view the changes commit by commit so readability/reviewing is not an issue |
There was a problem hiding this comment.
Looks good to me. Thanks for the cleanup!
Our CPU/CUDA backend is in great need of a refactor to reuse the same varying legalization logic we have for other targets. We are missing a lot of legalization there. This PR is a good step towards the greater unification.
|
Looks like there are still conflicts. |
|
Sorry for the plethora of dirty commits but the last two commits are the only changes, the rest are reverted(I had incorrect merge commits). I had to move additional logic from #5963. |
Related to #4657, related to #5933.
Reuse common code for Metal and WGSL entry point legalization.
Hoisting + packing is also currently not done for WGSL entry point parameters and they are required for #5933 as only struct entry points are given attributes. I will have a follow up fix for that as that change will break lots of test and I do not want to make this PR any bigger.
#4657 ultimately aims to also unify the legalization logic with C/CPP/CUDA backends - this PR is a good first step to unify the shading language backends.