HLSL offsetting
Francisco Casas
fcasas at codeweavers.com
Tue Jun 28 17:15:28 CDT 2022
Hello,
JIC, I attach the most recent version of the patch. I still have to
split it, but I addressed all the other suggestions.
Best regards,
Francisco.
On 27-06-22 18:05, Zebediah Figura wrote:
> On 6/27/22 11:54, Francisco Casas wrote:
>> Hello,
>>
>> On 25-06-22 13:29, Zebediah Figura wrote:
>>> On 6/24/22 23:46, Francisco Casas wrote:
>>>> Hello,
>>>>
>>>> I attach the third version of the previous patch, applying the
>>>> suggestions made by Zeb, and some other changes that arose from them.
>>>>
>>>> The patch is based on the master branch, but I guess I will have to
>>>> update it if the recent patches from Giovanni are accepted, before
>>>> sending it for review.
>>>>
>>>
>>> Is it not possible to split up this patch, as I had asked earlier?
>>>
>>
>> I think I can separate some small parts, like hlsl_free_deref() as you
>> suggested, but we would still have a large patch that's hard to split.
>
> Sure. Every little bit helps, though.
>
> I think if we introduce hlsl_new_component_load() and
> hlsl_new_load_from_deref() [or whatever they end up being called] in a
> separate patch (or patches) that'll do a lot to help review.
>
>>
>>> There's also a couple things I notice from skimming:
>>>
>>> * I think add_load() should also be split into deref / component
>>> versions. Most of its users only need to count components.
>>>
>>
>> add_record_load() and add_array_load() will require indexes, but yes,
>> add_matrix_scalar_load(), intrinsic_mul(), add_expr(), and add_cast(),
>> in particular the parts where they deal with matrices, could be
>> simplified if we use component offsets.
>>
>> I will split the function then.
>>
>>> * Actually, should we be passing a deref instead of just a var to
>>> hlsl_new_component_load()? See e.g. add_cast(). That would make it a
>>> simple wrapper around hlsl_new_load_from_deref() [and the names would
>>> also need to be changed...] I'm not sure why I thought it made sense
>>> like this, frankly.
>>>
>>
>> Allowing to pass a deref instead of just a var to
>> hlsl_new_component_load() seems like a good idea. It may save us from
>> creating synthetic variables e.g. in initialize_var_components().
>>
>> I don't think it can be just made a simple wrapper around
>> hlsl_new_load_from_deref() though, since a single component within a
>> struct may require to be accessed with path of larger length than 2.
>> It requires a little more complex implementation, but it can be done.
>
> Ah, right, of course.
>
>>> * Do we need to always split up matrices into scalars? Should we make
>>> non-contiguous loads a problem for parse time? As long as we're
>>> trying to keep backend-specific details out of the HLSL IR, that
>>> seems potentially reasonable; it'd just require emitting multiple
>>> copy instructions for some cases. That would allow us to simplify
>>> split_copy() and add_matrix_scalar_load(), and I believe would thus
>>> also get rid of the need to pass multiple offsets to
>>> hlsl_new_load_from_deref().
>>
>> If we go back to make non-contiguous loads a problem for parse time,
>> then I think it would make sense for row_major matrices to retrieve a
>> row, and column_matrices to retrieve a column when indexing with a path.
>>
>> I will see if I can do it while keeping the component offsets
>> consistent with the order in the initializers.
>
> Right. To be clear I'm not *sure* if this is the best approach, but it
> seems possible.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-vkd3d-shader-hlsl-Replace-register-offsets-with-inde.patch
Type: text/x-patch
Size: 79384 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220628/df6dec80/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-vkd3d-shader-hlsl-Store-the-struct-fields-as-an-arra.patch
Type: text/x-patch
Size: 25575 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220628/df6dec80/attachment-0001.bin>
More information about the wine-devel
mailing list