This patch makes index expressions on resources hlsl_ir_index nodes instead of hlsl_ir_resource_load nodes, because it is not known if they will be used later as the lhs of an hlsl_ir_resource_store.
For now, the only benefit is consistency.
-- v2: vkd3d-shader/hlsl: Don't keep the implicit mipmap level on hlsl_ir_index.
From: Francisco Casas fcasas@codeweavers.com
This patch makes index expressions on resources hlsl_ir_index nodes instead of hlsl_ir_resource_load nodes, because it is not known if they will be used later as the lhs of an hlsl_ir_resource_store.
For now, the only benefit is consistency. --- libs/vkd3d-shader/hlsl.c | 5 ++++ libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl.y | 42 +++++++++++++++++--------------- libs/vkd3d-shader/hlsl_codegen.c | 20 ++++++++++++++- 4 files changed, 48 insertions(+), 20 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 32511590..5c271ed4 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1362,6 +1362,11 @@ bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index) return type->class == HLSL_CLASS_MATRIX && !hlsl_type_is_row_major(type); }
+bool hlsl_index_is_resource_access(struct hlsl_ir_index *index) +{ + return index->val.node->data_type->class == HLSL_CLASS_OBJECT; +} + 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 974252d4..1d02d7a3 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -1091,6 +1091,7 @@ struct hlsl_ir_store *hlsl_new_store_component(struct hlsl_ctx *ctx, struct hlsl const struct hlsl_deref *lhs, unsigned int comp, struct hlsl_ir_node *rhs);
bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index); +bool hlsl_index_is_resource_access(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); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 194d21f4..13d2aab0 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -759,9 +759,7 @@ static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct h && (expr_type->base_type == HLSL_TYPE_TEXTURE || expr_type->base_type == HLSL_TYPE_UAV) && expr_type->sampler_dim != HLSL_SAMPLER_DIM_GENERIC) { - struct hlsl_resource_load_params load_params = {.type = HLSL_RESOURCE_LOAD}; unsigned int dim_count = hlsl_sampler_dim_count(expr_type->sampler_dim); - struct hlsl_ir_resource_load *resource_load;
if (index_type->class > HLSL_CLASS_VECTOR || index_type->dimx != dim_count) { @@ -781,13 +779,10 @@ static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct h if (!(index = add_zero_mipmap_level(ctx, instrs, index, dim_count, loc))) return false;
- load_params.format = expr_type->e.resource_format; - load_params.resource = array; - load_params.coords = index; - - if (!(resource_load = hlsl_new_resource_load(ctx, &load_params, loc))) + if (!(return_index = hlsl_new_index(ctx, array, index, loc))) return false; - list_add_tail(instrs, &resource_load->node.entry); + list_add_tail(instrs, &return_index->entry); + return true; }
@@ -1742,7 +1737,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 && lhs->type != HLSL_IR_INDEX) + while (lhs->type != HLSL_IR_LOAD && lhs->type != HLSL_IR_INDEX) { if (lhs->type == HLSL_IR_EXPR && hlsl_ir_expr(lhs)->op == HLSL_OP1_CAST) { @@ -1779,17 +1774,19 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in } }
- if (lhs->type == HLSL_IR_RESOURCE_LOAD) + if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_resource_access(hlsl_ir_index(lhs))) { - struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(lhs); + struct hlsl_ir_node *load_coords = hlsl_ir_index(lhs)->idx.node; struct hlsl_ir_resource_store *store; + struct hlsl_deref resource_deref; struct hlsl_type *resource_type; struct hlsl_ir_swizzle *coords; unsigned int dim_count;
- /* Such an lvalue was produced by an index expression. */ - assert(load->load_type == HLSL_RESOURCE_LOAD); - resource_type = hlsl_deref_get_type(ctx, &load->resource); + if (!hlsl_init_deref_from_index_chain(ctx, &resource_deref, hlsl_ir_index(lhs)->val.node)) + return NULL; + + resource_type = hlsl_deref_get_type(ctx, &resource_deref); assert(resource_type->class == HLSL_CLASS_OBJECT); assert(resource_type->base_type == HLSL_TYPE_TEXTURE || resource_type->base_type == HLSL_TYPE_UAV);
@@ -1804,16 +1801,23 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in "Resource store expressions must write to all components.");
/* Remove the (implicit) mipmap level from the load expression. */ - assert(load->coords.node->data_type->class == HLSL_CLASS_VECTOR); - assert(load->coords.node->data_type->base_type == HLSL_TYPE_UINT); - assert(load->coords.node->data_type->dimx == dim_count + 1); - if (!(coords = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, Y, Z, W), dim_count, load->coords.node, &lhs->loc))) + assert(load_coords->data_type->class == HLSL_CLASS_VECTOR); + assert(load_coords->data_type->base_type == HLSL_TYPE_UINT); + assert(load_coords->data_type->dimx == dim_count + 1); + if (!(coords = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, Y, Z, W), dim_count, load_coords, &lhs->loc))) + { + hlsl_cleanup_deref(&resource_deref); return NULL; + } list_add_tail(instrs, &coords->node.entry);
- if (!(store = hlsl_new_resource_store(ctx, &load->resource, &coords->node, rhs, &lhs->loc))) + if (!(store = hlsl_new_resource_store(ctx, &resource_deref, &coords->node, rhs, &lhs->loc))) + { + hlsl_cleanup_deref(&resource_deref); return NULL; + } list_add_tail(instrs, &store->node.entry); + hlsl_cleanup_deref(&resource_deref); } else if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_noncontiguous(hlsl_ir_index(lhs))) { diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 42f8ab3b..3ee173b3 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -736,7 +736,8 @@ static bool lower_calls(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void * * 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 into individual - * hlsl_ir_loads. */ + * hlsl_ir_loads, or individual hlsl_ir_resource_loads, in case the indexing is a + * resource access. */ static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) { struct hlsl_deref var_deref; @@ -751,6 +752,23 @@ static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, index = hlsl_ir_index(instr); val = index->val.node;
+ if (hlsl_index_is_resource_access(index)) + { + struct hlsl_resource_load_params params = {0}; + struct hlsl_ir_resource_load *load; + + params.type = HLSL_RESOURCE_LOAD; + params.resource = val; + params.coords = index->idx.node; + params.format = val->data_type->e.resource_format; + + if (!(load = hlsl_new_resource_load(ctx, ¶ms, &instr->loc))) + return false; + list_add_before(&instr->entry, &load->node.entry); + hlsl_replace_node(instr, &load->node); + return true; + } + if (!(var = hlsl_new_synthetic_var(ctx, "index-val", val->data_type, &instr->loc))) return false; hlsl_init_simple_deref_from_var(&var_deref, var);
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl.y | 54 +++----------------------------- libs/vkd3d-shader/hlsl_codegen.c | 47 ++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 50 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 13d2aab0..f58a8cda 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -716,39 +716,6 @@ static struct hlsl_ir_node *add_binary_arithmetic_expr(struct hlsl_ctx *ctx, str enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg1, struct hlsl_ir_node *arg2, const struct vkd3d_shader_location *loc);
-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) -{ - struct hlsl_ir_load *coords_load; - struct hlsl_deref coords_deref; - struct hlsl_ir_constant *zero; - struct hlsl_ir_store *store; - struct hlsl_ir_var *coords; - - if (!(coords = hlsl_new_synthetic_var(ctx, "coords", - hlsl_get_vector_type(ctx, HLSL_TYPE_UINT, dim_count + 1), loc))) - return NULL; - - hlsl_init_simple_deref_from_var(&coords_deref, coords); - if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, index, (1u << dim_count) - 1, loc))) - return NULL; - list_add_tail(instrs, &store->node.entry); - - if (!(zero = hlsl_new_uint_constant(ctx, 0, loc))) - return NULL; - list_add_tail(instrs, &zero->node.entry); - - if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, &zero->node, 1u << dim_count, loc))) - return NULL; - list_add_tail(instrs, &store->node.entry); - - if (!(coords_load = hlsl_new_var_load(ctx, coords, loc))) - return NULL; - list_add_tail(instrs, &coords_load->node.entry); - - return &coords_load->node; -} - 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) { @@ -776,9 +743,6 @@ static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct h hlsl_get_vector_type(ctx, HLSL_TYPE_UINT, dim_count), &index->loc))) return false;
- if (!(index = add_zero_mipmap_level(ctx, instrs, index, dim_count, loc))) - return false; - if (!(return_index = hlsl_new_index(ctx, array, index, loc))) return false; list_add_tail(instrs, &return_index->entry); @@ -1776,11 +1740,10 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in
if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_resource_access(hlsl_ir_index(lhs))) { - struct hlsl_ir_node *load_coords = hlsl_ir_index(lhs)->idx.node; + struct hlsl_ir_node *coords = hlsl_ir_index(lhs)->idx.node; struct hlsl_ir_resource_store *store; struct hlsl_deref resource_deref; struct hlsl_type *resource_type; - struct hlsl_ir_swizzle *coords; unsigned int dim_count;
if (!hlsl_init_deref_from_index_chain(ctx, &resource_deref, hlsl_ir_index(lhs)->val.node)) @@ -1800,18 +1763,11 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in hlsl_error(ctx, &lhs->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_WRITEMASK, "Resource store expressions must write to all components.");
- /* Remove the (implicit) mipmap level from the load expression. */ - assert(load_coords->data_type->class == HLSL_CLASS_VECTOR); - assert(load_coords->data_type->base_type == HLSL_TYPE_UINT); - assert(load_coords->data_type->dimx == dim_count + 1); - if (!(coords = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, Y, Z, W), dim_count, load_coords, &lhs->loc))) - { - hlsl_cleanup_deref(&resource_deref); - return NULL; - } - list_add_tail(instrs, &coords->node.entry); + assert(coords->data_type->class == HLSL_CLASS_VECTOR); + assert(coords->data_type->base_type == HLSL_TYPE_UINT); + assert(coords->data_type->dimx == dim_count);
- if (!(store = hlsl_new_resource_store(ctx, &resource_deref, &coords->node, rhs, &lhs->loc))) + if (!(store = hlsl_new_resource_store(ctx, &resource_deref, coords, rhs, &lhs->loc))) { hlsl_cleanup_deref(&resource_deref); return NULL; diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 3ee173b3..3536e8cc 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -732,6 +732,42 @@ static bool lower_calls(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void * return true; }
+static struct hlsl_ir_node *add_zero_mipmap_level(struct hlsl_ctx *ctx, struct hlsl_ir_node *index, + const struct vkd3d_shader_location *loc) +{ + unsigned int dim_count = index->data_type->dimx; + struct hlsl_ir_load *coords_load; + struct hlsl_deref coords_deref; + struct hlsl_ir_constant *zero; + struct hlsl_ir_store *store; + struct hlsl_ir_var *coords; + + assert(dim_count < 4); + + if (!(coords = hlsl_new_synthetic_var(ctx, "coords", + hlsl_get_vector_type(ctx, HLSL_TYPE_UINT, dim_count + 1), loc))) + return NULL; + + hlsl_init_simple_deref_from_var(&coords_deref, coords); + if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, index, (1u << dim_count) - 1, loc))) + return NULL; + list_add_after(&index->entry, &store->node.entry); + + if (!(zero = hlsl_new_uint_constant(ctx, 0, loc))) + return NULL; + list_add_after(&store->node.entry, &zero->node.entry); + + if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, &zero->node, 1u << dim_count, loc))) + return NULL; + list_add_after(&zero->node.entry, &store->node.entry); + + if (!(coords_load = hlsl_new_var_load(ctx, coords, loc))) + return NULL; + list_add_after(&store->node.entry, &coords_load->node.entry); + + return &coords_load->node; +} + /* 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. @@ -754,12 +790,21 @@ static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
if (hlsl_index_is_resource_access(index)) { + unsigned int dim_count = hlsl_sampler_dim_count(val->data_type->sampler_dim); + struct hlsl_ir_node *coords = index->idx.node; struct hlsl_resource_load_params params = {0}; struct hlsl_ir_resource_load *load;
+ assert(coords->data_type->class == HLSL_CLASS_VECTOR); + assert(coords->data_type->base_type == HLSL_TYPE_UINT); + assert(coords->data_type->dimx == dim_count); + + if (!(coords = add_zero_mipmap_level(ctx, coords, &instr->loc))) + return false; + params.type = HLSL_RESOURCE_LOAD; params.resource = val; - params.coords = index->idx.node; + params.coords = coords; params.format = val->data_type->e.resource_format;
if (!(load = hlsl_new_resource_load(ctx, ¶ms, &instr->loc)))
On Fri Apr 28 17:13:29 2023 +0000, Zebediah Figura wrote:
I do like this for consistency, yes. Should we generate index instructions without the implicit zero mipmap level?
I think it is a good idea. It saves us from having to remove it on resource stores.
I added a second patch for that purpose.
This merge request was approved by Zebediah Figura.
This merge request was approved by Giovanni Mascellani.
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Giovanni Mascellani (＠giomasce)
-
Zebediah Figura (＠zfigura)