On Mon Sep 4 14:01:18 2023 +0000, Henri Verbeet wrote:
A general design question though: it seems that you could avoid
introducing a new register type simply by creating "regular" temporary registers instead of SSA ones. An SSA register has the additional property that you cannot write it after the first time, essentially, but that doesn't prevent you from using a temporary and just writing it once. Would there be a fundamental problem with that approach? If not, then why did you decide to introduce a new type? For the record, I have the same concern.
Both DXIL and SPIR-V use SSA, so using it in the IR is by far the
simplest way to handle the values. I don't think the second part of that statement necessarily follows from the first part.
Using temps would introduce the problem of selecting an unused temp,
i.e. one whose value is no longer needed, and it becomes even more complex when dealing with `PHI` instructions. Why is that?
Also, temps are written and read with `OpStore` and `OpLoad`, which
SSA renders unnecessary. I'm not inclined to add a comment on this as I think the question won't arise when everything is upstream. How valuable is that? If it's significant, we may want to consider converting TPF to SSA form as well in the SPIR-V backend. It does become a bit of an issue with upstreaming this though; in principle upstreaming dxil support isn't affected much by the current freeze, but that's predicated on not requiring significant changes to the existing code. Making significant changes to the shared IR or SPIR-V backend would likely need to wait until after the 1.9 release.
On revisiting this I think PHI instructions should disappear if temps are handled correctly, and it would eliminate some complications in the structuriser. On the other hand, optimisation of code generation by Vulkan drivers may be compromised when temps are used instead of SSA. We would need to test it to know for sure, but if it's ok, there's no guarantee we won't encounter another driver later which has issues.
How valuable is that? If it's significant, we may want to consider converting TPF to SSA form as well in the SPIR-V backend.
I have wondered that too. TPF's structured control flow probably makes it not especially complicated to do, even though it will need PHI instructions. If `OpStore` and `OpLoad` are an issue, we should see performance improvements in, e.g. SotTR with this change. I'll try that unless anyone has a better idea.