HLSL offsetting
Francisco Casas
fcasas at codeweavers.com
Mon Jun 27 11:54:17 CDT 2022
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.
> 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.
> * 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.
Best regards,
Francisco.
More information about the wine-devel
mailing list