zfigura at codeweavers.com
Wed Jun 15 18:20:13 CDT 2022
On 6/14/22 22:05, Francisco Casas wrote:
> On 13-06-22 11:59, Francisco Casas wrote:
>> Okay, I stopped working on multiple register offsets and now I am
>> working on this approach.
>> I still think that adding a pass to translate these "route"s to the
>> original register "offset"s, so that we can implement this change
>> gradually, is good. We can move the pass forward as we change more
>> passes to work with component offsets. And we can debug more easily
>> the things that we don't translate correctly.
>> I am aiming to implement the new approach until right after
>> split_matrix_copies, and put the translation afterwards. So far it
>> seems to be going nicely.
> I attach a patch that achieves this, in case there are any opinions.
> The "paths" of component indexes seem like the best way to solve the
> problem after all. I think that keeping them as an arrays of nodes in
> the hlsl_deref allows to write more uniform code than using a more
> complex data structure, as Giovanni mentioned.
> IMO, I think this patch manages matrices nicely too, but there is the
> problem that it is based on top of
> bb49bdba gmascellani vkd3d-shader/hlsl: Allow majority modifiers on
> function declarations.
> which is a little behind of master.
> So I still have to rebase it on top of current master, and in particular
> on top of the recent patches pertaining matrices. So I will probably
> have to make some design decisions while solving the conflicts.
> I guess that the main objective is that we don't want to deal with
> matrices at all after splitting matrix copies.
From skimming the patch, here are some immediate thoughts:
* I have a patch which has been around for a while which replaces the
field linked list with an array. I think we wanted that anyway, but we
especially want it given list_get().
* All current (upstream) uses of hlsl_new_store() and hlsl_new_load()
with a nonzero offset fall into one of two categories:
- retrieving the element at a specific offset (initializer and matrix
- adding another offset to an existing hlsl_deref (splitting passes,
initial parsing of field/element derefs)
I might have missed something, but assuming I've got that right, I
think that the hlsl_new_load() and hlsl_new_store() should take
arguments to reflect that. (I.e. split each into two functions, one
taking an unsigned int and the other taking a const struct hlsl_deref
*). In that case struct path_arg would be limited to hlsl.c, if it even
needs to exist.
Note that this step could also be done independently of anything
else, which would help make the "big" patch more reviewable.
* Similarly I think hlsl_new_resource_load() and friends should just
take a "const struct hlsl_deref *" or two. I'm not immediately sure what
to do with prepend_input_copy() and friends—maybe those should be
converted to iterative passes instead of recursive ones.
* Stuff like free_hlsl_deref() can be made into a separate patch.
More information about the wine-devel