[PATCH v3 0/10] MR124: vkd3d-shader/hlsl: Introduce hlsl_ir_index, part 1/2.
This node type is intended for use during parse-time. While we parse an indexing expression such as `a[3]`, we don't know if it will end up as part of an expression (in which case it must be folded into a load) or it is for the lhs of a store (in which case it must be folded into the store's deref). This node type is used to represent these accesses and no longer rely on building an `hlsl_ir_load` for each array index or struct record access. `hlsl_ir_index` chains are lowered into derefs when (and if) they are used to specify the lhs of an assignment. All `hlsl_ir_index`es are lowered into `hlsl_ir_load`s with a compilation pass. The changes introduced in these series allow to solve the problem with the return variable of function calls presented in !93, and to properly support assignment to matrix indexes, which is something we are not doing correctly. Further patches (in my [index node](https://gitlab.winehq.org/fcasas/vkd3d/-/commits/index_node) branch) add support for indexing non-load expressions, such as `(a + b)[1]` and allowing to represent resource loads through `hlsl_ir_index`, so that `hlsl_ir_resource_store`s don't have to rely on `hlsl_ir_resource_load`s. -- v3: vkd3d-shader/hlsl: Copy index chain values into synthetic vars. tests: Test indexing of non-loads. vkd3d-shader/hlsl: Support column-major matrix indexing in the lhs. vkd3d-shader/hlsl: Always load from a synthetic copy in add_load_component(). vkd3d-shader/hlsl: Remove add_load_index(). vkd3d-shader/hlsl: Use hlsl_ir_index for array and record access. vkd3d-shader/hlsl: Introduce hlsl_ir_index. tests: Test side effects on indexes. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/hlsl-function.shader_test | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/hlsl-function.shader_test b/tests/hlsl-function.shader_test index c836808d..a7cad6d7 100644 --- a/tests/hlsl-function.shader_test +++ b/tests/hlsl-function.shader_test @@ -261,3 +261,37 @@ float4 main() : sv_target func(true); return 0; } + + +[pixel shader] +float func(float a) +{ + return a + 1; +} + +float4 main() : sv_target +{ + return float4(func(1.0), func(2.0), func(5.0), func(6.0)); +} + +[test] +draw quad +todo probe all rgba (2.0, 3.0, 6.0, 7.0) + + +[pixel shader] +float func(float a) +{ + return a + 1; +} + +float4 main() : sv_target +{ + float4 a = {func(1.0), func(2.0), func(5.0), func(6.0)}; + + return a; +} + +[test] +draw quad +todo probe all rgba (2.0, 3.0, 6.0, 7.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/hlsl-matrix-indexing.shader_test | 62 ++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/hlsl-matrix-indexing.shader_test b/tests/hlsl-matrix-indexing.shader_test index 716b43d4..7ae1a91d 100644 --- a/tests/hlsl-matrix-indexing.shader_test +++ b/tests/hlsl-matrix-indexing.shader_test @@ -76,3 +76,65 @@ float4 main() : SV_TARGET [test] draw quad probe all rgba (1.0, 5.0, 7.0, 12.0) + + +[pixel shader] +float4 main() : SV_TARGET +{ + float3x2 m = {1, 2, 3, 4, 5, 6}; + + m[1] = float2(30, 40); + + return float4(m[1], m[2]); +} + +[test] +draw quad +todo probe all rgba (30.0, 40.0, 5.0, 6.0) + + +[pixel shader] +float4 main() : SV_TARGET +{ + row_major float3x2 m = {1, 2, 3, 4, 5, 6}; + + m[2] = float2(50, 60); + + return float4(m[1], m[2]); +} + +[test] +draw quad +probe all rgba (3.0, 4.0, 50.0, 60.0) + + +[pixel shader todo] +uniform int i; + +float4 main() : sv_target +{ + float4x4 mat = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; + + return mat[i]; +} + +[test] +uniform 0 int 2 +todo draw quad +todo probe all rgba (8, 9, 10, 11) + + +[pixel shader todo] +uniform int i; + +float4 main() : sv_target +{ + row_major float4x4 mat = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; + + return mat[i]; +} + +[test] +uniform 0 int 3 +todo draw quad +todo probe all rgba (12, 13, 14, 15) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> Co-authored-by: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + tests/side-effects.shader_test | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/side-effects.shader_test diff --git a/Makefile.am b/Makefile.am index 29692432..4232e5d7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -143,6 +143,7 @@ vkd3d_shader_tests = \ tests/sampler-offset.shader_test \ tests/saturate.shader_test \ tests/shader-interstage-interface.shader_test \ + tests/side-effects.shader_test \ tests/sqrt.shader_test \ tests/step.shader_test \ tests/swizzle-constant-prop.shader_test \ diff --git a/tests/side-effects.shader_test b/tests/side-effects.shader_test new file mode 100644 index 00000000..224b4c21 --- /dev/null +++ b/tests/side-effects.shader_test @@ -0,0 +1,27 @@ +[pixel shader] +float4 main() : sv_target +{ + float x[3] = {0, 2, 3}; + + return x[x[1] = 1]; +} + +[test] +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) + + +[pixel shader] +uint4 func(uint t) +{ + return uint4(t + 0, t + 1, t + 2, t + 3); +} + +float4 main() : sv_target +{ + return func(10)[func(0).y]; +} + +[test] +draw quad +todo probe all rgba (11.0, 11.0, 11.0, 11.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> This node type is intended for use during parse-time. While we parse an indexing expression such as "a[3]", we don't know if it will end up as part of an expression (in which case it must be folded into a load) or it is for the lhs of a store (in which case it must be folded into the store's deref). --- This patch adds what may be considered dead-code, but this is promptly used in the following patch. I organized it this way to somewhat ease the review. --- libs/vkd3d-shader/hlsl.c | 60 ++++++++++++++++++++++++++++++++ libs/vkd3d-shader/hlsl.h | 15 ++++++++ libs/vkd3d-shader/hlsl.y | 1 + libs/vkd3d-shader/hlsl_codegen.c | 9 +++++ 4 files changed, 85 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 24822e97..4dcc0685 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1274,6 +1274,28 @@ struct hlsl_ir_swizzle *hlsl_new_swizzle(struct hlsl_ctx *ctx, DWORD s, unsigned return swizzle; } +struct hlsl_ir_node *hlsl_new_index(struct hlsl_ctx *ctx, struct hlsl_ir_node *val, + struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc) +{ + struct hlsl_type *type = val->data_type; + struct hlsl_ir_index *index; + + if (!(index = hlsl_alloc(ctx, sizeof(*index)))) + return NULL; + + if (type->type == HLSL_CLASS_OBJECT) + type = type->e.resource_format; + else if (type->type == HLSL_CLASS_MATRIX) + type = hlsl_get_vector_type(ctx, type->base_type, type->dimx); + else + type = hlsl_get_element_type_from_path_index(ctx, type, idx); + + init_node(&index->node, HLSL_IR_INDEX, type, loc); + hlsl_src_from_node(&index->val, val); + hlsl_src_from_node(&index->idx, idx); + return &index->node; +} + struct hlsl_ir_jump *hlsl_new_jump(struct hlsl_ctx *ctx, enum hlsl_ir_jump_type type, struct vkd3d_shader_location loc) { struct hlsl_ir_jump *jump; @@ -1533,6 +1555,17 @@ static struct hlsl_ir_node *clone_swizzle(struct hlsl_ctx *ctx, return &dst->node; } +static struct hlsl_ir_node *clone_index(struct hlsl_ctx *ctx, struct clone_instr_map *map, + struct hlsl_ir_index *src) +{ + struct hlsl_ir_node *dst; + + if (!(dst = hlsl_new_index(ctx, map_instr(map, src->val.node), map_instr(map, src->idx.node), + &src->node.loc))) + return NULL; + return dst; +} + static struct hlsl_ir_node *clone_instr(struct hlsl_ctx *ctx, struct clone_instr_map *map, const struct hlsl_ir_node *instr) { @@ -1550,6 +1583,9 @@ static struct hlsl_ir_node *clone_instr(struct hlsl_ctx *ctx, case HLSL_IR_IF: return clone_if(ctx, map, hlsl_ir_if(instr)); + case HLSL_IR_INDEX: + return clone_index(ctx, map, hlsl_ir_index(instr)); + case HLSL_IR_JUMP: return clone_jump(ctx, hlsl_ir_jump(instr)); @@ -1939,6 +1975,7 @@ const char *hlsl_node_type_to_string(enum hlsl_ir_node_type type) "HLSL_IR_CONSTANT", "HLSL_IR_EXPR", "HLSL_IR_IF", + "HLSL_IR_INDEX", "HLSL_IR_LOAD", "HLSL_IR_LOOP", "HLSL_IR_JUMP", @@ -2317,6 +2354,14 @@ static void dump_ir_swizzle(struct vkd3d_string_buffer *buffer, const struct hls } } +static void dump_ir_index(struct vkd3d_string_buffer *buffer, const struct hlsl_ir_index *index) +{ + dump_src(buffer, &index->val); + vkd3d_string_buffer_printf(buffer, "[idx:"); + dump_src(buffer, &index->idx); + vkd3d_string_buffer_printf(buffer, "]"); +} + static void dump_instr(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, const struct hlsl_ir_node *instr) { if (instr->index) @@ -2344,6 +2389,10 @@ static void dump_instr(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, dump_ir_if(ctx, buffer, hlsl_ir_if(instr)); break; + case HLSL_IR_INDEX: + dump_ir_index(buffer, hlsl_ir_index(instr)); + break; + case HLSL_IR_JUMP: dump_ir_jump(buffer, hlsl_ir_jump(instr)); break; @@ -2518,6 +2567,13 @@ static void free_ir_swizzle(struct hlsl_ir_swizzle *swizzle) vkd3d_free(swizzle); } +static void free_ir_index(struct hlsl_ir_index *index) +{ + hlsl_src_remove(&index->val); + hlsl_src_remove(&index->idx); + vkd3d_free(index); +} + void hlsl_free_instr(struct hlsl_ir_node *node) { assert(list_empty(&node->uses)); @@ -2540,6 +2596,10 @@ void hlsl_free_instr(struct hlsl_ir_node *node) free_ir_if(hlsl_ir_if(node)); break; + case HLSL_IR_INDEX: + free_ir_index(hlsl_ir_index(node)); + break; + case HLSL_IR_JUMP: free_ir_jump(hlsl_ir_jump(node)); break; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 24d0f517..f801349d 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -252,6 +252,7 @@ enum hlsl_ir_node_type HLSL_IR_CONSTANT, HLSL_IR_EXPR, HLSL_IR_IF, + HLSL_IR_INDEX, HLSL_IR_LOAD, HLSL_IR_LOOP, HLSL_IR_JUMP, @@ -538,6 +539,12 @@ struct hlsl_ir_swizzle DWORD swizzle; }; +struct hlsl_ir_index +{ + struct hlsl_ir_node node; + struct hlsl_src val, idx; +}; + /* Reference to a variable, or a part of it (e.g. a vector within a matrix within a struct). */ struct hlsl_deref { @@ -848,6 +855,12 @@ static inline struct hlsl_ir_swizzle *hlsl_ir_swizzle(const struct hlsl_ir_node return CONTAINING_RECORD(node, struct hlsl_ir_swizzle, node); } +static inline struct hlsl_ir_index *hlsl_ir_index(const struct hlsl_ir_node *node) +{ + assert(node->type == HLSL_IR_INDEX); + return CONTAINING_RECORD(node, struct hlsl_ir_index, node); +} + static inline void hlsl_src_from_node(struct hlsl_src *src, struct hlsl_ir_node *node) { src->node = node; @@ -1048,6 +1061,8 @@ struct hlsl_ir_store *hlsl_new_store_index(struct hlsl_ctx *ctx, const struct hl struct hlsl_ir_store *hlsl_new_store_component(struct hlsl_ctx *ctx, struct hlsl_block *block, const struct hlsl_deref *lhs, unsigned int comp, struct hlsl_ir_node *rhs); +struct hlsl_ir_node *hlsl_new_index(struct hlsl_ctx *ctx, struct hlsl_ir_node *val, + struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc); struct hlsl_ir_loop *hlsl_new_loop(struct hlsl_ctx *ctx, struct vkd3d_shader_location loc); struct hlsl_ir_resource_load *hlsl_new_resource_load(struct hlsl_ctx *ctx, const struct hlsl_resource_load_params *params, const struct vkd3d_shader_location *loc); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index e011ea24..bc0de136 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1153,6 +1153,7 @@ static unsigned int evaluate_array_dimension(struct hlsl_ir_node *node) } case HLSL_IR_EXPR: + case HLSL_IR_INDEX: case HLSL_IR_LOAD: case HLSL_IR_RESOURCE_LOAD: case HLSL_IR_SWIZZLE: diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 65bd9d4c..4c0b2dc2 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2155,6 +2155,7 @@ static bool dce(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) { case HLSL_IR_CONSTANT: case HLSL_IR_EXPR: + case HLSL_IR_INDEX: case HLSL_IR_LOAD: case HLSL_IR_RESOURCE_LOAD: case HLSL_IR_SWIZZLE: @@ -2394,6 +2395,14 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop swizzle->val.node->last_read = instr->index; break; } + case HLSL_IR_INDEX: + { + struct hlsl_ir_index *index = hlsl_ir_index(instr); + + index->val.node->last_read = instr->index; + index->idx.node->last_read = instr->index; + break; + } case HLSL_IR_CONSTANT: case HLSL_IR_JUMP: break; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com>
From this point on, it is no longer true that only hlsl_ir_loads can return objects, because an object can also come from chain of hlsl_ir_indexes that ends in an hlsl_ir_load.
For this reason, hlsl_resource_load_params now expects both the resource as the sampler to be just an hlsl_ir_node pointer instead of a pointer to a more specific hlsl_ir_load. --- libs/vkd3d-shader/hlsl.c | 90 ++++++++++++++++- libs/vkd3d-shader/hlsl.h | 5 +- libs/vkd3d-shader/hlsl.y | 128 ++++++++----------------- libs/vkd3d-shader/hlsl_codegen.c | 98 +++++++++++++++++++ tests/hlsl-matrix-indexing.shader_test | 4 +- 5 files changed, 227 insertions(+), 98 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 4dcc0685..56dea24f 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -435,6 +435,73 @@ static bool init_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hl return true; } +bool hlsl_init_deref_from_index_chain(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hlsl_ir_node *chain) +{ + struct hlsl_ir_index *index; + struct hlsl_ir_load *load; + unsigned int chain_len, i; + struct hlsl_ir_node *ptr; + + deref->path = NULL; + deref->path_len = 0; + deref->offset.node = NULL; + + assert(chain); + if (chain->type == HLSL_IR_INDEX) + assert(!hlsl_index_is_noncontiguous(hlsl_ir_index(chain))); + + /* Find the length of the index chain */ + chain_len = 0; + ptr = chain; + while (ptr->type == HLSL_IR_INDEX) + { + index = hlsl_ir_index(ptr); + + chain_len++; + ptr = index->val.node; + } + + if (ptr->type != HLSL_IR_LOAD) + { + /* This should be an hlsl_error() for invalid l-values, but indexing non-load expressions is + * also not supported yet so we use a hlsl_fixme() for now. */ + hlsl_fixme(ctx, &chain->loc, "Indexing %s.", hlsl_node_type_to_string(ptr->type)); + return false; + } + load = hlsl_ir_load(ptr); + + if (!init_deref(ctx, deref, load->src.var, load->src.path_len + chain_len)) + return false; + + for (i = 0; i < load->src.path_len; ++i) + hlsl_src_from_node(&deref->path[i], load->src.path[i].node); + + chain_len = 0; + ptr = chain; + while (ptr->type == HLSL_IR_INDEX) + { + unsigned int p = deref->path_len - 1 - chain_len; + + index = hlsl_ir_index(ptr); + if (hlsl_index_is_noncontiguous(index)) + { + hlsl_src_from_node(&deref->path[p], deref->path[p + 1].node); + hlsl_src_remove(&deref->path[p + 1]); + hlsl_src_from_node(&deref->path[p + 1], index->idx.node); + } + else + { + hlsl_src_from_node(&deref->path[p], index->idx.node); + } + + chain_len++; + ptr = index->val.node; + } + assert(deref->path_len == load->src.path_len + chain_len); + + return true; +} + struct hlsl_type *hlsl_deref_get_type(struct hlsl_ctx *ctx, const struct hlsl_deref *deref) { struct hlsl_type *type; @@ -1225,17 +1292,23 @@ struct hlsl_ir_resource_load *hlsl_new_resource_load(struct hlsl_ctx *ctx, return NULL; init_node(&load->node, HLSL_IR_RESOURCE_LOAD, params->format, loc); load->load_type = params->type; - if (!hlsl_copy_deref(ctx, &load->resource, ¶ms->resource)) + + if (!hlsl_init_deref_from_index_chain(ctx, &load->resource, params->resource)) { vkd3d_free(load); return NULL; } - if (!hlsl_copy_deref(ctx, &load->sampler, ¶ms->sampler)) + + if (params->sampler) { - hlsl_cleanup_deref(&load->resource); - vkd3d_free(load); - return NULL; + if (!hlsl_init_deref_from_index_chain(ctx, &load->sampler, params->sampler)) + { + hlsl_cleanup_deref(&load->resource); + vkd3d_free(load); + return NULL; + } } + hlsl_src_from_node(&load->coords, params->coords); hlsl_src_from_node(&load->texel_offset, params->texel_offset); hlsl_src_from_node(&load->lod, params->lod); @@ -1274,6 +1347,13 @@ struct hlsl_ir_swizzle *hlsl_new_swizzle(struct hlsl_ctx *ctx, DWORD s, unsigned return swizzle; } +bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index) +{ + struct hlsl_type *type = index->val.node->data_type; + + return type->type == HLSL_CLASS_MATRIX && !hlsl_type_is_row_major(type); +} + struct hlsl_ir_node *hlsl_new_index(struct hlsl_ctx *ctx, struct hlsl_ir_node *val, struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc) { diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index f801349d..2d66ed9e 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -785,7 +785,7 @@ struct hlsl_resource_load_params { struct hlsl_type *format; enum hlsl_resource_load_type type; - struct hlsl_deref resource, sampler; + struct hlsl_ir_node *resource, *sampler; struct hlsl_ir_node *coords, *lod, *texel_offset; }; @@ -997,6 +997,7 @@ void hlsl_dump_function(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func, enum vkd3d_shader_target_type target_type, struct vkd3d_shader_code *out); +bool hlsl_init_deref_from_index_chain(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hlsl_ir_node *chain); bool hlsl_copy_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, const struct hlsl_deref *other); void hlsl_cleanup_deref(struct hlsl_deref *deref); @@ -1061,6 +1062,8 @@ struct hlsl_ir_store *hlsl_new_store_index(struct hlsl_ctx *ctx, const struct hl struct hlsl_ir_store *hlsl_new_store_component(struct hlsl_ctx *ctx, struct hlsl_block *block, const struct hlsl_deref *lhs, unsigned int comp, struct hlsl_ir_node *rhs); +bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index); + struct hlsl_ir_node *hlsl_new_index(struct hlsl_ctx *ctx, struct hlsl_ir_node *val, struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc); struct hlsl_ir_loop *hlsl_new_loop(struct hlsl_ctx *ctx, struct vkd3d_shader_location loc); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index bc0de136..0ad3bc1d 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -685,9 +685,10 @@ static struct hlsl_ir_load *add_load_component(struct hlsl_ctx *ctx, struct list return load; } -static bool add_record_load(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *record, +static bool add_record_access(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *record, unsigned int idx, const struct vkd3d_shader_location loc) { + struct hlsl_ir_node *index; struct hlsl_ir_constant *c; assert(idx < record->data_type->e.record.field_count); @@ -696,60 +697,17 @@ static bool add_record_load(struct hlsl_ctx *ctx, struct list *instrs, struct hl return false; list_add_tail(instrs, &c->node.entry); - return !!add_load_index(ctx, instrs, record, &c->node, &loc); + if (!(index = hlsl_new_index(ctx, record, &c->node, &loc))) + return false; + list_add_tail(instrs, &index->entry); + + return true; } static struct hlsl_ir_node *add_binary_arithmetic_expr(struct hlsl_ctx *ctx, struct list *instrs, enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg1, struct hlsl_ir_node *arg2, const struct vkd3d_shader_location *loc); -static bool add_matrix_index(struct hlsl_ctx *ctx, struct list *instrs, - struct hlsl_ir_node *matrix, struct hlsl_ir_node *index, const struct vkd3d_shader_location *loc) -{ - struct hlsl_type *mat_type = matrix->data_type, *ret_type; - struct hlsl_deref var_deref; - struct hlsl_ir_load *load; - struct hlsl_ir_var *var; - unsigned int i; - - if (hlsl_type_is_row_major(mat_type)) - return add_load_index(ctx, instrs, matrix, index, loc); - - ret_type = hlsl_get_vector_type(ctx, mat_type->base_type, mat_type->dimx); - - if (!(var = hlsl_new_synthetic_var(ctx, "index", ret_type, loc))) - return false; - hlsl_init_simple_deref_from_var(&var_deref, var); - - for (i = 0; i < mat_type->dimx; ++i) - { - struct hlsl_ir_load *column, *value; - struct hlsl_ir_store *store; - struct hlsl_ir_constant *c; - struct hlsl_block block; - - if (!(c = hlsl_new_uint_constant(ctx, i, loc))) - return false; - list_add_tail(instrs, &c->node.entry); - - if (!(column = add_load_index(ctx, instrs, matrix, &c->node, loc))) - return false; - - if (!(value = add_load_index(ctx, instrs, &column->node, index, loc))) - return false; - - if (!(store = hlsl_new_store_component(ctx, &block, &var_deref, i, &value->node))) - return false; - list_move_tail(instrs, &block.instrs); - } - - if (!(load = hlsl_new_var_load(ctx, var, *loc))) - return false; - list_add_tail(instrs, &load->node.entry); - - return true; -} - static struct hlsl_ir_node *add_zero_mipmap_level(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *index, unsigned int dim_count, const struct vkd3d_shader_location *loc) { @@ -783,10 +741,11 @@ static struct hlsl_ir_node *add_zero_mipmap_level(struct hlsl_ctx *ctx, struct l return &coords_load->node; } -static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *array, +static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *array, struct hlsl_ir_node *index, const struct vkd3d_shader_location *loc) { const struct hlsl_type *expr_type = array->data_type, *index_type = index->data_type; + struct hlsl_ir_node *return_index; struct hlsl_ir_expr *cast; if (expr_type->type == HLSL_CLASS_OBJECT @@ -795,8 +754,6 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls { struct hlsl_resource_load_params load_params = {.type = HLSL_RESOURCE_LOAD}; unsigned int dim_count = hlsl_sampler_dim_count(expr_type->sampler_dim); - /* Only HLSL_IR_LOAD can return an object. */ - struct hlsl_ir_load *object_load = hlsl_ir_load(array); struct hlsl_ir_resource_load *resource_load; if (index_type->type > HLSL_CLASS_VECTOR || index_type->dimx != dim_count) @@ -818,7 +775,7 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls return false; load_params.format = expr_type->e.resource_format; - load_params.resource = object_load->src; + load_params.resource = array; load_params.coords = index; if (!(resource_load = hlsl_new_resource_load(ctx, &load_params, loc))) @@ -838,10 +795,7 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls list_add_tail(instrs, &cast->node.entry); index = &cast->node; - if (expr_type->type == HLSL_CLASS_MATRIX) - return add_matrix_index(ctx, instrs, array, index, loc); - - if (expr_type->type != HLSL_CLASS_ARRAY && expr_type->type != HLSL_CLASS_VECTOR) + if (expr_type->type != HLSL_CLASS_ARRAY && expr_type->type != HLSL_CLASS_VECTOR && expr_type->type != HLSL_CLASS_MATRIX) { if (expr_type->type == HLSL_CLASS_SCALAR) hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_INDEX, "Scalar expressions cannot be array-indexed."); @@ -850,8 +804,9 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls return false; } - if (!add_load_index(ctx, instrs, array, index, loc)) + if (!(return_index = hlsl_new_index(ctx, array, index, loc))) return false; + list_add_tail(instrs, &return_index->entry); return true; } @@ -1727,7 +1682,7 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in if (!(rhs = add_implicit_conversion(ctx, instrs, rhs, lhs_type, &rhs->loc))) return NULL; - while (lhs->type != HLSL_IR_LOAD && lhs->type != HLSL_IR_RESOURCE_LOAD) + while (lhs->type != HLSL_IR_LOAD && lhs->type != HLSL_IR_RESOURCE_LOAD && lhs->type != HLSL_IR_INDEX) { if (lhs->type == HLSL_IR_EXPR && hlsl_ir_expr(lhs)->op == HLSL_OP1_CAST) { @@ -1800,13 +1755,26 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in return NULL; list_add_tail(instrs, &store->node.entry); } + else if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_noncontiguous(hlsl_ir_index(lhs))) + { + hlsl_fixme(ctx, &lhs->loc, "Column-major matrix index store."); + return NULL; + } else { struct hlsl_ir_store *store; + struct hlsl_deref deref; - if (!(store = hlsl_new_store_index(ctx, &hlsl_ir_load(lhs)->src, NULL, rhs, writemask, &rhs->loc))) + if (!hlsl_init_deref_from_index_chain(ctx, &deref, lhs)) + return NULL; + + if (!(store = hlsl_new_store_index(ctx, &deref, NULL, rhs, writemask, &rhs->loc))) + { + hlsl_cleanup_deref(&deref); return NULL; + } list_add_tail(instrs, &store->node.entry); + hlsl_cleanup_deref(&deref); } /* Don't use the instruction itself as a source, as this makes structure @@ -3058,7 +3026,6 @@ static bool intrinsic_tex(struct hlsl_ctx *ctx, const struct parse_initializer * struct hlsl_resource_load_params load_params = {.type = HLSL_RESOURCE_SAMPLE}; const struct hlsl_type *sampler_type; struct hlsl_ir_resource_load *load; - struct hlsl_ir_load *sampler_load; struct hlsl_ir_node *coords; if (params->args_count != 2 && params->args_count != 4) @@ -3087,10 +3054,7 @@ static bool intrinsic_tex(struct hlsl_ctx *ctx, const struct parse_initializer * } else { - /* Only HLSL_IR_LOAD can return an object. */ - sampler_load = hlsl_ir_load(params->args[0]); - - load_params.resource = sampler_load->src; + load_params.resource = params->args[0]; } if (!(coords = add_implicit_conversion(ctx, params->instrs, params->args[1], @@ -3424,7 +3388,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl const char *name, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { const struct hlsl_type *object_type = object->data_type; - struct hlsl_ir_load *object_load; if (object_type->type != HLSL_CLASS_OBJECT || object_type->base_type != HLSL_TYPE_TEXTURE || object_type->sampler_dim == HLSL_SAMPLER_DIM_GENERIC) @@ -3438,9 +3401,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl return false; } - /* Only HLSL_IR_LOAD can return an object. */ - object_load = hlsl_ir_load(object); - if (!strcmp(name, "Load") && object_type->sampler_dim != HLSL_SAMPLER_DIM_CUBE && object_type->sampler_dim != HLSL_SAMPLER_DIM_CUBEARRAY) @@ -3484,7 +3444,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl return false; load_params.format = object_type->e.resource_format; - load_params.resource = object_load->src; + load_params.resource = object; if (!(load = hlsl_new_resource_load(ctx, &load_params, loc))) return false; @@ -3500,7 +3460,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl struct hlsl_resource_load_params load_params = {.type = HLSL_RESOURCE_SAMPLE}; const struct hlsl_type *sampler_type; struct hlsl_ir_resource_load *load; - struct hlsl_ir_load *sampler_load; if (params->args_count < 2 || params->args_count > 4 + !!offset_dim) { @@ -3523,9 +3482,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl return false; } - /* Only HLSL_IR_LOAD can return an object. */ - sampler_load = hlsl_ir_load(params->args[0]); - if (!(load_params.coords = add_implicit_conversion(ctx, instrs, params->args[1], hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) return false; @@ -3543,8 +3499,8 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl hlsl_fixme(ctx, loc, "Tiled resource status argument."); load_params.format = object_type->e.resource_format; - load_params.resource = object_load->src; - load_params.sampler = sampler_load->src; + load_params.resource = object; + load_params.sampler = params->args[0]; if (!(load = hlsl_new_resource_load(ctx, &load_params, loc))) return false; @@ -3564,7 +3520,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl struct hlsl_resource_load_params load_params = {0}; const struct hlsl_type *sampler_type; struct hlsl_ir_resource_load *load; - struct hlsl_ir_load *sampler_load; unsigned int read_channel; if (!strcmp(name, "GatherGreen")) @@ -3640,16 +3595,13 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl return false; } - /* Only HLSL_IR_LOAD can return an object. */ - sampler_load = hlsl_ir_load(params->args[0]); - if (!(load_params.coords = add_implicit_conversion(ctx, instrs, params->args[1], hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) return false; load_params.format = hlsl_get_vector_type(ctx, object_type->e.resource_format->base_type, 4); - load_params.resource = object_load->src; - load_params.sampler = sampler_load->src; + load_params.resource = object; + load_params.sampler = params->args[0]; if (!(load = hlsl_new_resource_load(ctx, &load_params, loc))) return false; @@ -3665,7 +3617,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl const unsigned int offset_dim = hlsl_offset_dim_count(object_type->sampler_dim); const struct hlsl_type *sampler_type; struct hlsl_ir_resource_load *load; - struct hlsl_ir_load *sampler_load; if (params->args_count < 3 || params->args_count > 4 + !!offset_dim) { @@ -3688,9 +3639,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl return false; } - /* Only HLSL_IR_LOAD can return an object. */ - sampler_load = hlsl_ir_load(params->args[0]); - if (!(load_params.coords = add_implicit_conversion(ctx, instrs, params->args[1], hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) load_params.coords = params->args[1]; @@ -3710,8 +3658,8 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl hlsl_fixme(ctx, loc, "Tiled resource status argument."); load_params.format = object_type->e.resource_format; - load_params.resource = object_load->src; - load_params.sampler = sampler_load->src; + load_params.resource = object; + load_params.sampler = params->args[0]; if (!(load = hlsl_new_resource_load(ctx, &load_params, loc))) return false; @@ -5296,7 +5244,7 @@ postfix_expr: } field_idx = field - type->e.record.fields; - if (!add_record_load(ctx, $1, node, field_idx, @2)) + if (!add_record_access(ctx, $1, node, field_idx, @2)) YYABORT; $$ = $1; } @@ -5325,7 +5273,7 @@ postfix_expr: list_move_tail($1, $3); vkd3d_free($3); - if (!add_array_load(ctx, $1, array, index, &@2)) + if (!add_array_access(ctx, $1, array, index, &@2)) { destroy_instr_list($1); YYABORT; diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 4c0b2dc2..b743be69 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -731,6 +731,101 @@ static bool lower_calls(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void * return true; } +/* Single hlsl_ir_index instructions on column-major matrices cannot be turned into derefs, however, + * index chains that refer to a single component can be. This pass turns the former in the latter. */ +static bool lower_noncontiguous_matrix_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +{ + struct hlsl_ir_node *mat, *copy; + struct hlsl_ir_index *index; + struct hlsl_deref var_deref; + struct hlsl_ir_load *load; + struct hlsl_ir_var *var; + unsigned int i; + + if (instr->type != HLSL_IR_INDEX) + return false; + index = hlsl_ir_index(instr); + + if (!hlsl_index_is_noncontiguous(index)) + return false; + + mat = index->val.node; + assert(!hlsl_type_is_row_major(mat->data_type)); + + /* We will remove this instruction but we need to generate nodes that refer to it, + * so we create a copy. */ + if (!(copy = hlsl_new_index(ctx, index->val.node, index->idx.node, &instr->loc))) + return false; + list_add_before(&instr->entry, ©->entry); + + if (!(var = hlsl_new_synthetic_var(ctx, "index", instr->data_type, &instr->loc))) + return false; + hlsl_init_simple_deref_from_var(&var_deref, var); + + for (i = 0; i < mat->data_type->dimx; ++i) + { + struct hlsl_ir_store *store; + struct hlsl_ir_constant *c; + struct hlsl_ir_node *value; + struct hlsl_block block; + + if (!(c = hlsl_new_uint_constant(ctx, i, &instr->loc))) + return false; + list_add_before(&instr->entry, &c->node.entry); + + if (!(value = hlsl_new_index(ctx, copy, &c->node, &instr->loc))) + return false; + list_add_before(&instr->entry, &value->entry); + + if (!(store = hlsl_new_store_component(ctx, &block, &var_deref, i, value))) + return false; + list_move_before(&instr->entry, &block.instrs); + } + + if (!(load = hlsl_new_var_load(ctx, var, instr->loc))) + return false; + list_add_before(&instr->entry, &load->node.entry); + hlsl_replace_node(instr, &load->node); + + return true; +} + +/* hlsl_ir_index nodes are a parse-time construct used to represent array indexing and struct + * record access before knowing if they will be used in the lhs of an assignment --in which case + * they are lowered into a deref-- or as the load of an element within a larger value. + * For the latter case, this pass takes care of lowering hlsl_ir_indexes (and index chains) into + * individual hlsl_ir_loads. + * It is worth noting that the generated hlsl_ir_loads don't load from a copy of the variable loaded + * at the beginning of the index chain, but from the same variable instead, because it is assumed + * that the value of the variable won't change between the the hlsl_ir_load instruction and the + * hlsl_ir_index. */ +static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +{ + struct hlsl_ir_index *index; + struct hlsl_ir_load *load; + struct hlsl_deref deref; + + if (instr->type != HLSL_IR_INDEX) + return false; + index = hlsl_ir_index(instr); + + if (hlsl_index_is_noncontiguous(index)) + return false; + + if (!hlsl_init_deref_from_index_chain(ctx, &deref, instr)) + return false; + + if (!(load = hlsl_new_load_index(ctx, &deref, NULL, &instr->loc))) + { + hlsl_cleanup_deref(&deref); + return false; + } + list_add_before(&instr->entry, &load->node.entry); + hlsl_replace_node(instr, &load->node); + hlsl_cleanup_deref(&deref); + return true; +} + /* Lower casts from vec1 to vecN to swizzles. */ static bool lower_broadcasts(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) { @@ -3233,6 +3328,9 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry while (transform_ir(ctx, lower_calls, body, NULL)); + transform_ir(ctx, lower_noncontiguous_matrix_index_loads, body, NULL); + transform_ir(ctx, lower_index_loads, body, NULL); + LIST_FOR_EACH_ENTRY(var, &ctx->globals->vars, struct hlsl_ir_var, scope_entry) { if (var->storage_modifiers & HLSL_STORAGE_UNIFORM) diff --git a/tests/hlsl-matrix-indexing.shader_test b/tests/hlsl-matrix-indexing.shader_test index 7ae1a91d..4ca0e871 100644 --- a/tests/hlsl-matrix-indexing.shader_test +++ b/tests/hlsl-matrix-indexing.shader_test @@ -78,7 +78,7 @@ draw quad probe all rgba (1.0, 5.0, 7.0, 12.0) -[pixel shader] +[pixel shader todo] float4 main() : SV_TARGET { float3x2 m = {1, 2, 3, 4, 5, 6}; @@ -89,7 +89,7 @@ float4 main() : SV_TARGET } [test] -draw quad +todo draw quad todo probe all rgba (30.0, 40.0, 5.0, 6.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> --- In v1 this call was replaced with a single hlsl_ir_index. But it can't be done this way now becase column-wise hlsl_ir_index were discarded. --- libs/vkd3d-shader/hlsl.y | 59 ++++++++-------------------------------- 1 file changed, 11 insertions(+), 48 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 0ad3bc1d..58051e46 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -620,38 +620,6 @@ static struct hlsl_ir_jump *add_return(struct hlsl_ctx *ctx, struct list *instrs return jump; } -static struct hlsl_ir_load *add_load_index(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *var_instr, - struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc) -{ - const struct hlsl_deref *src; - struct hlsl_ir_load *load; - - if (var_instr->type == HLSL_IR_LOAD) - { - src = &hlsl_ir_load(var_instr)->src; - } - else - { - struct hlsl_ir_store *store; - struct hlsl_ir_var *var; - - if (!(var = hlsl_new_synthetic_var(ctx, "deref", var_instr->data_type, &var_instr->loc))) - return NULL; - - if (!(store = hlsl_new_simple_store(ctx, var, var_instr))) - return NULL; - list_add_tail(instrs, &store->node.entry); - - src = &store->lhs; - } - - if (!(load = hlsl_new_load_index(ctx, src, idx, loc))) - return NULL; - list_add_tail(instrs, &load->node.entry); - - return load; -} - static struct hlsl_ir_load *add_load_component(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *var_instr, unsigned int comp, const struct vkd3d_shader_location *loc) { @@ -1264,46 +1232,41 @@ static struct hlsl_ir_node *add_expr(struct hlsl_ctx *ctx, struct list *instrs, if (type->type == HLSL_CLASS_MATRIX) { - struct hlsl_type *vector_type; + struct hlsl_type *scalar_type; struct hlsl_deref var_deref; struct hlsl_ir_load *load; struct hlsl_ir_var *var; - vector_type = hlsl_get_vector_type(ctx, type->base_type, hlsl_type_minor_size(type)); + scalar_type = hlsl_get_scalar_type(ctx, type->base_type); if (!(var = hlsl_new_synthetic_var(ctx, "split_op", type, loc))) return NULL; hlsl_init_simple_deref_from_var(&var_deref, var); - for (i = 0; i < hlsl_type_major_size(type); ++i) + for (i = 0; i < type->dimy * type->dimx; ++i) { - struct hlsl_ir_node *value, *vector_operands[HLSL_MAX_OPERANDS] = { NULL }; + struct hlsl_ir_node *value, *cell_operands[HLSL_MAX_OPERANDS] = { NULL }; struct hlsl_ir_store *store; - struct hlsl_ir_constant *c; + struct hlsl_block block; unsigned int j; - if (!(c = hlsl_new_uint_constant(ctx, i, loc))) - return NULL; - list_add_tail(instrs, &c->node.entry); - for (j = 0; j < HLSL_MAX_OPERANDS; j++) { if (operands[j]) { - struct hlsl_ir_load *load; - - if (!(load = add_load_index(ctx, instrs, operands[j], &c->node, loc))) + if (!(load = add_load_component(ctx, instrs, operands[j], i, loc))) return NULL; - vector_operands[j] = &load->node; + + cell_operands[j] = &load->node; } } - if (!(value = add_expr(ctx, instrs, op, vector_operands, vector_type, loc))) + if (!(value = add_expr(ctx, instrs, op, cell_operands, scalar_type, loc))) return NULL; - if (!(store = hlsl_new_store_index(ctx, &var_deref, &c->node, value, 0, loc))) + if (!(store = hlsl_new_store_component(ctx, &block, &var_deref, i, value))) return NULL; - list_add_tail(instrs, &store->node.entry); + list_move_tail(instrs, &block.instrs); } if (!(load = hlsl_new_var_load(ctx, var, *loc))) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 24 ++++++++--------------- tests/hlsl-function.shader_test | 4 ++-- tests/hlsl-intrinsic-override.shader_test | 2 +- 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 58051e46..0f8a39c2 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -624,27 +624,19 @@ static struct hlsl_ir_load *add_load_component(struct hlsl_ctx *ctx, struct list unsigned int comp, const struct vkd3d_shader_location *loc) { const struct hlsl_deref *src; + struct hlsl_ir_store *store; struct hlsl_ir_load *load; struct hlsl_block block; + struct hlsl_ir_var *var; - if (var_instr->type == HLSL_IR_LOAD) - { - src = &hlsl_ir_load(var_instr)->src; - } - else - { - struct hlsl_ir_store *store; - struct hlsl_ir_var *var; - - if (!(var = hlsl_new_synthetic_var(ctx, "deref", var_instr->data_type, &var_instr->loc))) - return NULL; + if (!(var = hlsl_new_synthetic_var(ctx, "deref", var_instr->data_type, &var_instr->loc))) + return NULL; - if (!(store = hlsl_new_simple_store(ctx, var, var_instr))) - return NULL; - list_add_tail(instrs, &store->node.entry); + if (!(store = hlsl_new_simple_store(ctx, var, var_instr))) + return NULL; + list_add_tail(instrs, &store->node.entry); - src = &store->lhs; - } + src = &store->lhs; if (!(load = hlsl_new_load_component(ctx, &block, src, comp, loc))) return NULL; diff --git a/tests/hlsl-function.shader_test b/tests/hlsl-function.shader_test index a7cad6d7..4d4c2e2b 100644 --- a/tests/hlsl-function.shader_test +++ b/tests/hlsl-function.shader_test @@ -276,7 +276,7 @@ float4 main() : sv_target [test] draw quad -todo probe all rgba (2.0, 3.0, 6.0, 7.0) +probe all rgba (2.0, 3.0, 6.0, 7.0) [pixel shader] @@ -294,4 +294,4 @@ float4 main() : sv_target [test] draw quad -todo probe all rgba (2.0, 3.0, 6.0, 7.0) +probe all rgba (2.0, 3.0, 6.0, 7.0) diff --git a/tests/hlsl-intrinsic-override.shader_test b/tests/hlsl-intrinsic-override.shader_test index 13e1752d..55a23f21 100644 --- a/tests/hlsl-intrinsic-override.shader_test +++ b/tests/hlsl-intrinsic-override.shader_test @@ -12,7 +12,7 @@ float4 main() : sv_target [test] draw quad -todo probe all rgba (0.3, 0.3, 0.4, 0.6) +probe all rgba (0.3, 0.3, 0.4, 0.6) [pixel shader] -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 39 ++++++++++++++++++++++++-- tests/hlsl-matrix-indexing.shader_test | 6 ++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 0f8a39c2..44d63ecd 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1712,8 +1712,43 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in } else if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_noncontiguous(hlsl_ir_index(lhs))) { - hlsl_fixme(ctx, &lhs->loc, "Column-major matrix index store."); - return NULL; + struct hlsl_ir_index *row = hlsl_ir_index(lhs); + struct hlsl_ir_node *mat = row->val.node; + unsigned int i, k = 0; + + for (i = 0; i < mat->data_type->dimx; ++i) + { + struct hlsl_ir_store *store; + struct hlsl_ir_constant *c; + struct hlsl_ir_node *cell; + struct hlsl_ir_load *load; + struct hlsl_deref deref; + + if (!(writemask & (1 << i))) + continue; + + if (!(c = hlsl_new_uint_constant(ctx, i, &lhs->loc))) + return NULL; + list_add_tail(instrs, &c->node.entry); + + if (!(cell = hlsl_new_index(ctx, &row->node, &c->node, &lhs->loc))) + return NULL; + list_add_tail(instrs, &cell->entry); + + if (!(load = add_load_component(ctx, instrs, rhs, k++, &rhs->loc))) + return NULL; + + if (!hlsl_init_deref_from_index_chain(ctx, &deref, cell)) + return NULL; + + if (!(store = hlsl_new_store_index(ctx, &deref, NULL, &load->node, 0, &rhs->loc))) + { + hlsl_cleanup_deref(&deref); + return NULL; + } + list_add_tail(instrs, &store->node.entry); + hlsl_cleanup_deref(&deref); + } } else { diff --git a/tests/hlsl-matrix-indexing.shader_test b/tests/hlsl-matrix-indexing.shader_test index 4ca0e871..1dc1ede0 100644 --- a/tests/hlsl-matrix-indexing.shader_test +++ b/tests/hlsl-matrix-indexing.shader_test @@ -78,7 +78,7 @@ draw quad probe all rgba (1.0, 5.0, 7.0, 12.0) -[pixel shader todo] +[pixel shader] float4 main() : SV_TARGET { float3x2 m = {1, 2, 3, 4, 5, 6}; @@ -89,8 +89,8 @@ float4 main() : SV_TARGET } [test] -todo draw quad -todo probe all rgba (30.0, 40.0, 5.0, 6.0) +draw quad +probe all rgba (30.0, 40.0, 5.0, 6.0) [pixel shader] -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> --- Makefile.am | 1 + tests/expr-indexing.shader_test | 106 ++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 tests/expr-indexing.shader_test diff --git a/Makefile.am b/Makefile.am index 4232e5d7..0a21e2dd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -63,6 +63,7 @@ vkd3d_shader_tests = \ tests/conditional.shader_test \ tests/entry-point-semantics.shader_test \ tests/exp.shader_test \ + tests/expr-indexing.shader_test \ tests/floor.shader_test \ tests/frac.shader_test \ tests/function-return.shader_test \ diff --git a/tests/expr-indexing.shader_test b/tests/expr-indexing.shader_test new file mode 100644 index 00000000..5ced28c5 --- /dev/null +++ b/tests/expr-indexing.shader_test @@ -0,0 +1,106 @@ +[pixel shader todo] +float4 a, b; + +float4 main() : sv_target +{ + return (a + b)[1]; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 float4 5.0 6.0 7.0 8.0 +todo draw quad +todo probe all rgba (8.0, 8.0, 8.0, 8.0) + + +[pixel shader todo] +float4 a, b; +int i; + +float4 main() : sv_target +{ + return (a + b)[i]; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 float4 5.0 6.0 7.0 8.0 +uniform 8 int 2 +todo draw quad +todo probe all rgba (10.0, 10.0, 10.0, 10.0) + + +[pixel shader todo] +float4 a; +int i; + +float4 main() : sv_target +{ + return a.wyxz[3]; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +todo draw quad +todo probe all rgba (3.0, 3.0, 3.0, 3.0) + + +[pixel shader todo] +float4 a; +int i; + +float4 main() : sv_target +{ + return a.wyxz[i]; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 int 0 +todo draw quad +todo probe all rgba (4.0, 4.0, 4.0, 4.0) +uniform 4 int 2 +todo draw quad +todo probe all rgba (1.0, 1.0, 1.0, 1.0) + + +[pixel shader] +float4 a; +int i; + +float4 foo() +{ + return a; +} + +float4 main() : sv_target +{ + return foo()[3]; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 int 1 +draw quad +probe all rgba (4.0, 4.0, 4.0, 4.0) + + +[pixel shader todo] +float4 a; +int i; + +float4 foo() +{ + return a; +} + +float4 main() : sv_target +{ + return foo()[i]; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 int 1 +todo draw quad +todo probe all rgba (2.0, 2.0, 2.0, 2.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
From: Francisco Casas <fcasas(a)codeweavers.com> By replacing the value pointer of the last index in the chain with a load to a synthetic variable that holds the value. This also allows to index nodes that are not hlsl_ir_loads such as: (a + b)[1] --- libs/vkd3d-shader/hlsl.c | 4 +-- libs/vkd3d-shader/hlsl_codegen.c | 46 +++++++++++++++++++++++++++++--- tests/expr-indexing.shader_test | 12 ++++----- tests/side-effects.shader_test | 2 +- 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 56dea24f..16cd35ee 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -463,9 +463,7 @@ bool hlsl_init_deref_from_index_chain(struct hlsl_ctx *ctx, struct hlsl_deref *d if (ptr->type != HLSL_IR_LOAD) { - /* This should be an hlsl_error() for invalid l-values, but indexing non-load expressions is - * also not supported yet so we use a hlsl_fixme() for now. */ - hlsl_fixme(ctx, &chain->loc, "Indexing %s.", hlsl_node_type_to_string(ptr->type)); + hlsl_error(ctx, &chain->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_LVALUE, "Invalid l-value."); return false; } load = hlsl_ir_load(ptr); diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index b743be69..4b13a77c 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -731,6 +731,45 @@ static bool lower_calls(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void * return true; } +/* This pass replaces the values that each index chain points to with a load to a copy in a + * synthetic variable. So that later, hlsl_derefs can be safely created from these loads. */ +static bool copy_index_values(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +{ + struct hlsl_ir_index *index; + struct hlsl_ir_node *val; + + if (instr->type != HLSL_IR_INDEX) + return false; + + index = hlsl_ir_index(instr); + val = index->val.node; + + if (val->type != HLSL_IR_INDEX) + { + struct hlsl_ir_store *store; + struct hlsl_ir_load *load; + struct hlsl_ir_var *var; + + if (!(var = hlsl_new_synthetic_var(ctx, "index-val", val->data_type, &val->loc))) + return false; + + if (!(store = hlsl_new_simple_store(ctx, var, val))) + return false; + list_add_before(&instr->entry, &store->node.entry); + + if (!(load = hlsl_new_var_load(ctx, var, instr->loc))) + return false; + list_add_before(&instr->entry, &load->node.entry); + + hlsl_src_remove(&index->val); + hlsl_src_from_node(&index->val, &load->node); + + return true; + } + + return false; +} + /* Single hlsl_ir_index instructions on column-major matrices cannot be turned into derefs, however, * index chains that refer to a single component can be. This pass turns the former in the latter. */ static bool lower_noncontiguous_matrix_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) @@ -796,9 +835,9 @@ static bool lower_noncontiguous_matrix_index_loads(struct hlsl_ctx *ctx, struct * For the latter case, this pass takes care of lowering hlsl_ir_indexes (and index chains) into * individual hlsl_ir_loads. * It is worth noting that the generated hlsl_ir_loads don't load from a copy of the variable loaded - * at the beginning of the index chain, but from the same variable instead, because it is assumed - * that the value of the variable won't change between the the hlsl_ir_load instruction and the - * hlsl_ir_index. */ + * at the beginning of the index chain, but from the same variable instead. This can be done because + * the copy_index_values pass takes care of creating the copies and pointing the index chains to + * them. */ static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) { struct hlsl_ir_index *index; @@ -3328,6 +3367,7 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry while (transform_ir(ctx, lower_calls, body, NULL)); + transform_ir(ctx, copy_index_values, body, NULL); transform_ir(ctx, lower_noncontiguous_matrix_index_loads, body, NULL); transform_ir(ctx, lower_index_loads, body, NULL); diff --git a/tests/expr-indexing.shader_test b/tests/expr-indexing.shader_test index 5ced28c5..546deac7 100644 --- a/tests/expr-indexing.shader_test +++ b/tests/expr-indexing.shader_test @@ -1,4 +1,4 @@ -[pixel shader todo] +[pixel shader] float4 a, b; float4 main() : sv_target @@ -9,8 +9,8 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 5.0 6.0 7.0 8.0 -todo draw quad -todo probe all rgba (8.0, 8.0, 8.0, 8.0) +draw quad +probe all rgba (8.0, 8.0, 8.0, 8.0) [pixel shader todo] @@ -30,7 +30,7 @@ todo draw quad todo probe all rgba (10.0, 10.0, 10.0, 10.0) -[pixel shader todo] +[pixel shader] float4 a; int i; @@ -41,8 +41,8 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 -todo draw quad -todo probe all rgba (3.0, 3.0, 3.0, 3.0) +draw quad +probe all rgba (3.0, 3.0, 3.0, 3.0) [pixel shader todo] diff --git a/tests/side-effects.shader_test b/tests/side-effects.shader_test index 224b4c21..c961ed12 100644 --- a/tests/side-effects.shader_test +++ b/tests/side-effects.shader_test @@ -24,4 +24,4 @@ float4 main() : sv_target [test] draw quad -todo probe all rgba (11.0, 11.0, 11.0, 11.0) +probe all rgba (11.0, 11.0, 11.0, 11.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124
I wanted to avoid copying the load into a synthetic variable and then loading from it, since the generated IR before dce is getting increasingly bloated, which may make debugging difficult.
I'd rather have clear and consistent IR behaviour at the cost of conciseness. But I guess either way our hand is kind of forced by native behaviour... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124#note_27696
Maybe it'd be clearer to handle the noncontiguous matrix case by checking if this iteration is a scalar whose parent is a noncontiguous matrix dereference?
Since the loop goes from the component to the variable, rather than the other way around, that would require paying attention to the next iteration, rather than the previous one, and advance `ptr` two steps in that case. I don't think it improves simplicity. I don't think it's simpler, but it is a bit clearer. I don't think it matters a lot though, so I won't argue too much for it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124#note_27697
In this case it is still better to create the copy though, because otherwise we would need a way to specify which uses must be transferred to the replacement and which ones must be kept pointing to the original instruction.
Oh, of course, I didn't consider that. So yeah, this is pretty much the only way to do it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124#note_27700
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
* For the latter case, this pass takes care of lowering hlsl_ir_indexes (and index chains) into * individual hlsl_ir_loads. * It is worth noting that the generated hlsl_ir_loads don't load from a copy of the variable loaded - * at the beginning of the index chain, but from the same variable instead, because it is assumed - * that the value of the variable won't change between the the hlsl_ir_load instruction and the - * hlsl_ir_index. */ + * at the beginning of the index chain, but from the same variable instead. This can be done because + * the copy_index_values pass takes care of creating the copies and pointing the index chains to + * them. */ I'm not quite happy about the explanation before or after this commit.
From my perspective, the result of "x[x[1] = 1]" in patch 3/10 is counterintuitive. If the array is evaluated before its index, then I would expect it to have an intermediate value, just like func(10) does. Like, there *shouldn't* be any difference between two different node types conceptually.
...and then I realized, in the middle of writing that, that "the array is evaluated before its index" is an implicit assumption, and lo and behold, testing shows that's simply not the case. Here's a simple shader that proves it: ``` float4 func(void) { static uint i; i += 1; return float4(0.1, 0.2, 0.3, 0.4) + i; } float4 main() : sv_target { return func()[func().x]; } ``` This returns 2.2; if the array were evaluated first it'd be 1.3 (as vkd3d with this patch series). This means, I think that if we reverse the list_move_tail() in the array indexing rule, there's no reason that we can't just always use a synthetic load when lowering indices. Yes, this results in an unnecessary proliferation of temporary variables and LOAD instructions. But I'd really rather have IR that behaves according to consistent rules as a top priority, so that (a) that kind of function-call behaviour that we're trying to fix never happens again, and (b) we don't have to do mental gymnastics to prove that it can't happen. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124#note_27704
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
return true; }
+/* This pass replaces the values that each index chain points to with a load to a copy in a + * synthetic variable. So that later, hlsl_derefs can be safely created from these loads. */
This isn't grammatical English I'm afraid; "so that" begins an indirect clause and hence can't be a standalone sentence. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124#note_27705
This means, I think that if we reverse the list_move_tail() in the array indexing rule, there's no reason that we can't just always use a synthetic load when lowering indices.
Okay, I will reverse the `list_move_tail()` ordering, but I am not sure I understand the rest of the sentence. Patch 10/10 indeed stores the hlsl_ir_index "vals" to a synthetic var, so that then the `lower_index_loads` pass turns them into hlsl_ir_loads to that synthetic var. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124#note_27706
This means, I think that if we reverse the list_move_tail() in the array indexing rule, there's no reason that we can't just always use a synthetic load when lowering indices.
Okay, I will reverse the `list_move_tail()` ordering, but I am not sure I understand the rest of the sentence. Patch 10/10 indeed stores the hlsl_ir_index "vals" to a synthetic var, so that then the `lower_index_loads` pass turns them into hlsl_ir_loads to that synthetic var.
The point is patch 10/10 guards that on "val->type != HLSL_IR_INDEX", but I don't think we want that. And yes, I'm pretty sure we can't just get rid of that conditional. I haven't thought it through fully, but I think everything here needs to be one pass, and it presumably needs to convert one index at a time rather than transforming multiple dereferences at once into a single load. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/124#note_27708
participants (3)
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Zebediah Figura (＠zfigura)