On 6/14/22 22:05, Francisco Casas wrote:
Hello,
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"[1] 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 parsing)
- 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.