vkd3d-shader/ir: Validate SSA and TEMP usages.
Merge request reports
Activity
Subject: [PATCH 3/5] vkd3d-shader/ir: Check that TEMP registers have consistent dimensions.
I'm not sure I completely understand what we're doing here. In principle VKD3DSPR_TEMP registers are vec4's and would get VSIR_DIMENSION_VEC4, at least before DXIL. Is the idea here that we want to allow (presumably mainly for DXIL) VSIR_DIMENSION_SCALAR temps, as long as we consistently access them that way?
I'm not sure I completely understand what we're doing here. In principle VKD3DSPR_TEMP registers are vec4's and would get VSIR_DIMENSION_VEC4, at least before DXIL. Is the idea here that we want to allow (presumably mainly for DXIL) VSIR_DIMENSION_SCALAR temps, as long as we consistently access them that way?
Yeah, that's the idea. Of course it's up for debate (maybe I should have made that explicit; in general, my validator MRs are meant to be proposals for specifying the VSIR syntax).
At some point my idea was to mandate that TEMPs are always vec4 and SSAs are always scalar. But Conor's code already violates this, because some SSAs must be vec4 to allow assigning e.g. from sampling instructions; and as I already mentioned I plan to introduce a pass that converts SSAs to TEMPs, and while in principle I could make the TEMPs vec4 and use only
.x
, this feels a bit wasteful: it might be a better idea to explicitly signal to the backend that only the first component is to be used. I think the SPIR-V backend is already able to handle that gracefully. That's why I resolved in favour of allowing both SSAs and TEMPs to be both scalar and vec4, but it makes sense to require that they're consistent so that the semantic is clear.I realize that can be troublesome: for example there are cases in which Conor's code uses temporaries, I don't know exactly why; but I guess it can be an additional burden checking that temporaries are used consistently in different instances. However, I think that having a clear and sensible syntax for VSIR is more important.
At any rate, as I said, that's a proposal, and discussion is welcome.
I hope the new proposal is better.
Let me point out that I'm not completely convinced myself of what is the right choice here. I can see good reasons for mandating the TEMPs are always vec4. So for the moment I'm sticking with the more permissive choice and we'll see in the future. The option of introducing a pass that converts all TEMPs to vec4 remains on the plate, also (though it is already validated that scalar registers always have the obvious write mask and swizzle, so the backend can probably just ignore the dimension if so they wish).
added 1 commit
- 69023773 - vkd3d-shader/ir: Check that SSA registers are used validly.
mentioned in merge request !559 (merged)
added 26 commits
-
69023773...c5bc28b4 - 21 commits from branch
wine:master
- 26c6a872 - vkd3d-shader/ir: Simplify control flow in vsir_validate_register().
- fbd77486 - vkd3d-shader/ir: Use vkd3d_free() instead of free().
- 75034295 - vkd3d-shader/ir: Check that TEMP registers have consistent dimensions.
- 26bebe8f - vkd3d-shader/ir: Check that SSA registers have consistent dimensions.
- b0c8a47f - vkd3d-shader/ir: Check that SSA registers are used validly.
Toggle commit list-
69023773...c5bc28b4 - 21 commits from branch
mentioned in merge request !589 (merged)