[PATCH vkd3d v2 4/6] vkd3d-shader: Generate the synthetic name for the temp variable instead of the uniform.

[Zebediah Figura (she/her)]

On 4/20/21 3:19 PM, Matteo Bruni wrote:
> On Tue, Apr 20, 2021 at 5:30 PM Zebediah Figura (she/her)
> <zfigura at codeweavers.com> wrote:
>>
>> On 4/20/21 3:15 AM, Matteo Bruni wrote:
>>> Signed-off-by: Matteo Bruni <mbruni at codeweavers.com>
>>> ---
>>> I think a better way to handle this is to use a separate temp variable
>>> for each LOAD of the uniform, thus replacing each uniform LOAD with a
>>> STORE to a temp variable and a LOAD from that. That way we get less
>>> register pressure without having to complicate the register allocation
>>> or liveness computation algorithms. That is assuming that register
>>> pressure on the temporary registers is quite problematic for SM2 or
>>> even SM3 shaders since there are so few of them and there is no
>>> spilling to save the day.
>>>
>>> Of course there is a complication with that, that is preserving
>>> "writes" to uniforms. That could be taken into account pretty simply
>>> with write tracking, but another option is to do the above
>>> optimization only when legacy mode is not set (AFAIU writing to
>>> uniforms / globals is only allowed on older d3dx9 / d3dcompiler
>>> versions).
>>>
>>> Just food for thought. Also, ignore everything if you have something
>>> fancier already queued up.
>>>
>>
>> It's a bit orthogonal to this patch, but yes, I see the point. It's not
>> clear to me that write tracking isn't more complicated, though.
>> Personally I'm inclined to leave it alone until it actually does become
>> a problem.
> 
> That's certainly fine. I probably misspoke about write tracking, for
> some reason I was thinking we had a last_write thing already but we
> obviously don't :#
> 
> The point about "legacy mode" or something like that stands though and
> is more generally relevant. The HLSL compiler was heavily overhauled
> with the October 2006 SDK, among other things dropping ps_1_*  output
> support, and the parser again with the March 2008 SDK, which made it
> quite a bit more strict. Not sure off the top of my head what DLL
> versions those correspond to (the former is probably around _32 / _33
> time given that the option to use the older compiler is called
> D3DXSHADER_USE_LEGACY_D3DX9_31_DLL) but I can try to find out. For
> reference, the latter in particular is what made writing to globals
> invalid.
> Again it's not urgent by any means but it's also something to keep in mind.

You're right, I should have mentioned that. I think I checked 
varyings—which are always valid to both read from and write to—and then 
assumed that the same would be true of uniforms, which isn't the case.

Thus far I've avoided touching vkd3d options because API design is hard, 
but I can at least put it on the long-term list of things to look into...

> 
>> (as a side note—I don't know if it makes sense to try to code register
>> limits into the HLSL compiler (or validator). As I understand
>> wined3d/vkd3d shader translation layers won't care, which removes much
>> of the reason for us to. Not that wine should be considered
>> vkd3d-shader's only customer, but...)
> 
> Yeah, sure, this should only matter on "native" d3d, at least for
> temporary variables. Not so much for uniforms or varyings though (but
> those have a different set of issues and solutions, e.g. you want to
> coalesce identical constants).
> 

Indeed, I'll have to write some validation for those.