[PATCH vkd3d 03/12] vkd3d-shader/hlsl: Introduce hlsl_free_deref().
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 13 +++++++++---- libs/vkd3d-shader/hlsl.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index b349eb15..23136aeb 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1539,6 +1539,11 @@ void hlsl_free_instr_list(struct list *list) hlsl_free_instr(node); } +void hlsl_free_deref(struct hlsl_deref *deref) +{ + hlsl_src_remove(&deref->offset); +} + static void free_ir_constant(struct hlsl_ir_constant *constant) { vkd3d_free(constant); @@ -1568,7 +1573,7 @@ static void free_ir_jump(struct hlsl_ir_jump *jump) static void free_ir_load(struct hlsl_ir_load *load) { - hlsl_src_remove(&load->src.offset); + hlsl_free_deref(&load->src); vkd3d_free(load); } @@ -1581,8 +1586,8 @@ static void free_ir_loop(struct hlsl_ir_loop *loop) static void free_ir_resource_load(struct hlsl_ir_resource_load *load) { hlsl_src_remove(&load->coords); - hlsl_src_remove(&load->sampler.offset); - hlsl_src_remove(&load->resource.offset); + hlsl_free_deref(&load->sampler); + hlsl_free_deref(&load->resource); hlsl_src_remove(&load->texel_offset); vkd3d_free(load); } @@ -1590,7 +1595,7 @@ static void free_ir_resource_load(struct hlsl_ir_resource_load *load) static void free_ir_store(struct hlsl_ir_store *store) { hlsl_src_remove(&store->rhs); - hlsl_src_remove(&store->lhs.offset); + hlsl_free_deref(&store->lhs); vkd3d_free(store); } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index ce1f69df..5bfbc5a7 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -717,6 +717,8 @@ 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); +void hlsl_free_deref(struct hlsl_deref *deref); + void hlsl_replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new); void hlsl_free_instr(struct hlsl_ir_node *node); -- 2.34.1
On 7/1/22 16:24, Francisco Casas wrote:
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 13 +++++++++---- libs/vkd3d-shader/hlsl.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index b349eb15..23136aeb 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1539,6 +1539,11 @@ void hlsl_free_instr_list(struct list *list) hlsl_free_instr(node); }
+void hlsl_free_deref(struct hlsl_deref *deref) +{ + hlsl_src_remove(&deref->offset); +} +
This can be static, at least here (but I'd be surprised if it needs to be used outside of hlsl.c...)
On 04-07-22 19:02, Zebediah Figura wrote:
On 7/1/22 16:24, Francisco Casas wrote:
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 13 +++++++++---- libs/vkd3d-shader/hlsl.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index b349eb15..23136aeb 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1539,6 +1539,11 @@ void hlsl_free_instr_list(struct list *list) hlsl_free_instr(node); } +void hlsl_free_deref(struct hlsl_deref *deref) +{ + hlsl_src_remove(&deref->offset); +} +
This can be static, at least here (but I'd be surprised if it needs to be used outside of hlsl.c...)
hlsl_free_deref() is needed by the transform_deref_paths_into_offsets pass some patches ahead. But just there. I can make it static here and undo that in the vkd3d-shader/hlsl: Move hlsl_new_offset_from_path_index() to hlsl_codegen.c. patch.
Hi, Il 01/07/22 23:24, Francisco Casas ha scritto:
+void hlsl_free_deref(struct hlsl_deref *deref) +{ + hlsl_src_remove(&deref->offset); +} +
Again, mostly a nitpick, but personally when a function is called "free" and receives a pointer, I assume that it will free the pointer itself (after perhaps doing cleanup). Here there is the cleanup, but the pointer is not freed, so I would rename to "hlsl_unlink_deref", for example. Not sure if others agree with my sentiment. Giovanni.
On Tue, 5 Jul 2022 at 12:11, Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
Il 01/07/22 23:24, Francisco Casas ha scritto:
+void hlsl_free_deref(struct hlsl_deref *deref) +{ + hlsl_src_remove(&deref->offset); +} +
Again, mostly a nitpick, but personally when a function is called "free" and receives a pointer, I assume that it will free the pointer itself (after perhaps doing cleanup). Here there is the cleanup, but the pointer is not freed, so I would rename to "hlsl_unlink_deref", for example. Not sure if others agree with my sentiment.
The convention we had been using in wined3d and vkd3d for a while now is <object>_destroy() for functions that free the underlying storage, and <object>_cleanup() for functions that don't. Similarly, we have <object>_create() for functions that allocate storage, and <object>_init() for functions that don't. I'm not sure the HLSL compiler necessarily follows that convention though.
On 05-07-22 06:49, Henri Verbeet wrote:
On Tue, 5 Jul 2022 at 12:11, Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
Il 01/07/22 23:24, Francisco Casas ha scritto:
+void hlsl_free_deref(struct hlsl_deref *deref) +{ + hlsl_src_remove(&deref->offset); +} +
Again, mostly a nitpick, but personally when a function is called "free" and receives a pointer, I assume that it will free the pointer itself (after perhaps doing cleanup). Here there is the cleanup, but the pointer is not freed, so I would rename to "hlsl_unlink_deref", for example. Not sure if others agree with my sentiment.
The convention we had been using in wined3d and vkd3d for a while now is <object>_destroy() for functions that free the underlying storage, and <object>_cleanup() for functions that don't. Similarly, we have <object>_create() for functions that allocate storage, and <object>_init() for functions that don't. I'm not sure the HLSL compiler necessarily follows that convention though.
Okay, so, I am going for 'cleanup_deref'. I will rename it to 'hlsl_cleanup_deref' once it starts to be used outside hlsl.c and thus, cannot continue being static (Zeb's suggestion) in order to follow the convention. This renaming adds a little noise to vkd3d-shader/hlsl: Replace register offsets with index paths in parse code.
On 7/5/22 05:49, Henri Verbeet wrote:
On Tue, 5 Jul 2022 at 12:11, Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
Il 01/07/22 23:24, Francisco Casas ha scritto:
+void hlsl_free_deref(struct hlsl_deref *deref) +{ + hlsl_src_remove(&deref->offset); +} +
Again, mostly a nitpick, but personally when a function is called "free" and receives a pointer, I assume that it will free the pointer itself (after perhaps doing cleanup). Here there is the cleanup, but the pointer is not freed, so I would rename to "hlsl_unlink_deref", for example. Not sure if others agree with my sentiment.
The convention we had been using in wined3d and vkd3d for a while now is <object>_destroy() for functions that free the underlying storage, and <object>_cleanup() for functions that don't. Similarly, we have <object>_create() for functions that allocate storage, and <object>_init() for functions that don't. I'm not sure the HLSL compiler necessarily follows that convention though.
Broadly, although in this case hlsl_deref isn't exactly an object in the usual sense, so I was less concerned with it.
participants (4)
-
Francisco Casas -
Giovanni Mascellani -
Henri Verbeet -
Zebediah Figura