[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

Zebediah Figura (she/her) zfigura at codeweavers.com
Tue Jan 25 12:02:04 CST 2022


On 1/21/22 05:04, Giovanni Mascellani wrote:
> Hi,
> 
> Il 20/01/22 19:20, Matteo Bruni ha scritto:
>>>>>> I wonder if that's what it should do in the first place?
>>>>> Yes, witness the following shader:
>>>>>
>>>>> ---
>>>>>
>>>>> uint func(float x) { return 1; }
>>>>> uint func(half x) { return 2; }
>>>>>
>>>>> float4 main(void) : sv_target
>>>>> {
>>>>>        return float4(func(float(0)), func(half(0)), func(int(0)),
>>>>> func(uint(0)));
>>>>> }
>>>>>
>>>>> ---
>>>>>
>>>>> Native reduces this to (1, 2, 1, 1).
>>>> Fun... But at least it means we can fix add_implicit_conversion() and
>>>> avoid that kind of workarounds, if I understand correctly.
>>> I... guess, but it seems janky. The main point of
>>> add_implicit_conversion() is for stuff like assignment RHS or return
>>> values. If we had add_implicit_conversion() pass through half it'd
>>> generate IR like
>>>
>>> 2:  half | 1.0
>>> 3:       | x = @2
>>> 4: float | x
>>>
>>>
>>> (i.e. "x" is a float variable). Which... works, I guess? Maybe it's not
>>> that bad, since we already do a somewhat similar thing for scalar vs 
>>> vec1.
>> I think that's an improvement to the current state of things, right
>> now we're effectively losing information from the get-go. We can
>> always add a half-to-float lowering pass to simplify the IR.
> 
> I don't really understand what Zeb's proposed fragment of code have to 
> do with implicit conversions (that seems mostly a test about overload 
> resolution), and I don't understand why and how implicit conversions 
> should treat half in a special way.

Overload resolution and implicit conversion are essentially two 
strategies for dealing with what that code fragment demonstrates.

> 
> Also, I don't think we should mix types in the IR without casts. If 
> we're doing the same for vec1 vs scalar, I think we should rather fix 
> that than introduce other mistypings.

I don't think this is a desirable invariant to keep. It makes a lot of 
transformations harder for no reason (e.g. you can't "just" peephole two 
subsequent instructions anymore; you now have to deal with a possible 
identity conversion between them). By the time we get out of the parser 
we really shouldn't be doing type validation anyway.



More information about the wine-devel mailing list