On Thu, Aug 4, 2022 at 9:10 PM Francisco Casas [email protected] wrote:
Hello,
On 28-07-22 16:59, Matteo Bruni wrote:
On Wed, Jul 20, 2022 at 3:24 PM Francisco Casas [email protected] wrote:
Signed-off-by: Francisco Casas [email protected]
v3:
- No changes.
The recursive structure of prepend_input_var_copy() and append_output_var_copy() could be preserved creating additional loads to complete the paths. Otherwise we would be requiring passing whole paths as arguments.
These additional loads should be handled by DCE.
Still, matrix vectors are copied iteratively instead of recursively now, to avoid the boilerplate of creating new loads in this last step.
Signed-off-by: Francisco Casas [email protected]
libs/vkd3d-shader/hlsl_codegen.c | 188 +++++++++++++++++++------------ 1 file changed, 116 insertions(+), 72 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 6437006b..7a245007 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -239,59 +239,75 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir return ext_var; }
-static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_var *var,
struct hlsl_type *type, unsigned int field_offset, unsigned int modifiers, const struct hlsl_semantic *semantic)
+static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *lhs,
struct hlsl_type *type, unsigned int modifiers, const struct hlsl_semantic *semantic)
I haven't put a lot of thought into this, but it seems to me we could pass a deref instead of a redundant load and be mostly set. Mostly because we then need to add the instructions to the instruction list in some other way (e.g. by passing an explicit list).
I tried it passing a hlsl_blocks to be initialized (like in the component load/store initializers) but in the end it didn't seem like a good idea to me:
The problem is that we would need to create derefs in the wild (outside hlsl_ir_nodes), which would mean exposing several static functions in hlsl.c, like init_deref_from_component_index(). We would also need to use "hlsl_cleanup_deref()" which for now is only non-static because it is needed in the the pass that translates index paths back to register offsets.
So I don't think exposing the deref functions is better than adding these redundant loads, which are later deleted by the DCE pass, if I am not mistaken.
Alternatively, we could create the nodes just as a container for the derefs and immediately free them, but that also seems a little ugly.
So my vote is to keep this signature. Maybe remove the "type" parameter since it can be obtained from the deref inside the load node.
Fair enough. That was hard to see without actually trying and it sounds like redundant loads are the lesser evil in this case. Maybe a comment somewhere explaining that these loads are redundant and supposed to be eliminated by DCE would be nice.