[PATCH vkd3d 1/5] vkd3d-shader/hlsl: Use list_empty() instead of comparing list_count() against zero.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- libs/vkd3d-shader/hlsl.y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 636882c4b..5a17449a3 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -326,7 +326,7 @@ static bool append_conditional_break(struct hlsl_ctx *ctx, struct list *cond_lis struct hlsl_ir_if *iff;
/* E.g. "for (i = 0; ; ++i)". */ - if (!list_count(cond_list)) + if (list_empty(cond_list)) return true;
condition = node_from_list(cond_list);
Ported from 5d01ebab89cee8a3499ee00729c048068d5b719d from Wine.
Despite the commit message there, even GCC 11.1 chokes on this.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- include/private/rbtree.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/private/rbtree.h b/include/private/rbtree.h index b4993da3c..b5d38bca5 100644 --- a/include/private/rbtree.h +++ b/include/private/rbtree.h @@ -133,7 +133,7 @@ static inline struct rb_entry *rb_postorder_next(struct rb_entry *iter) /* iterate through the tree using a tree entry */ #define RB_FOR_EACH_ENTRY(elem, tree, type, field) \ for ((elem) = RB_ENTRY_VALUE(rb_head((tree)->root), type, field); \ - &(elem)->field; \ + (elem) != RB_ENTRY_VALUE(0, type, field); \ (elem) = RB_ENTRY_VALUE(rb_next(&elem->field), type, field))
/* iterate through the tree using using postorder, making it safe to free the entry */ @@ -145,7 +145,7 @@ static inline struct rb_entry *rb_postorder_next(struct rb_entry *iter) /* iterate through the tree using a tree entry and postorder, making it safe to free the entry */ #define RB_FOR_EACH_ENTRY_DESTRUCTOR(elem, elem2, tree, type, field) \ for ((elem) = RB_ENTRY_VALUE(rb_postorder_head((tree)->root), type, field); \ - &(elem)->field \ + (elem) != WINE_RB_ENTRY_VALUE(0, type, field) \ && (((elem2) = RB_ENTRY_VALUE(rb_postorder_next(&(elem)->field), type, field)) || 1); \ (elem) = (elem2))
Hi,
On 17/12/21 18:48, Zebediah Figura wrote:
--- a/include/private/rbtree.h +++ b/include/private/rbtree.h @@ -133,7 +133,7 @@ static inline struct rb_entry *rb_postorder_next(struct rb_entry *iter) /* iterate through the tree using a tree entry */ #define RB_FOR_EACH_ENTRY(elem, tree, type, field) \ for ((elem) = RB_ENTRY_VALUE(rb_head((tree)->root), type, field); \
&(elem)->field; \
(elem) != RB_ENTRY_VALUE(0, type, field); \ (elem) = RB_ENTRY_VALUE(rb_next(&elem->field), type, field))
Couldn't we take the chance to just avoid constructing dangling pointers (which is, as I read the C standard, undefined behavior)? That would amount, for example, to using
(elem) = (rb_next(&(elem)->field) ? RB_ENTRY_VALUE(rb_next(&(elem)->field), type, field) : NULL)
as iteration expression, similarly for the initialization expression and just checking that elem is not NULL for the condition expression.
Thanks, Giovanni.
On Fri, 17 Dec 2021 at 19:05, Zebediah Figura zfigura@codeweavers.com wrote:
Ported from 5d01ebab89cee8a3499ee00729c048068d5b719d from Wine.
Despite the commit message there, even GCC 11.1 chokes on this.
For what it's worth, there's a single usage of RB_FOR_EACH_ENTRY in vkd3d, and it looks like something that doesn't particularly need it. We have generally preferred using rb_for_each_entry(), so we could also just get rid of these macros.
On 1/6/22 03:33, Henri Verbeet wrote:
On Fri, 17 Dec 2021 at 19:05, Zebediah Figura zfigura@codeweavers.com wrote:
Ported from 5d01ebab89cee8a3499ee00729c048068d5b719d from Wine.
Despite the commit message there, even GCC 11.1 chokes on this.
For what it's worth, there's a single usage of RB_FOR_EACH_ENTRY in vkd3d, and it looks like something that doesn't particularly need it. We have generally preferred using rb_for_each_entry(), so we could also just get rid of these macros.
Is there a reason to prefer rb_for_each_entry()? It has always struck me as clumsier to program with callbacks, although I guess that's a matter of personal taste.
On Tue, 11 Jan 2022 at 00:06, Zebediah Figura (she/her) zfigura@codeweavers.com wrote:
On 1/6/22 03:33, Henri Verbeet wrote:
On Fri, 17 Dec 2021 at 19:05, Zebediah Figura zfigura@codeweavers.com wrote:
Ported from 5d01ebab89cee8a3499ee00729c048068d5b719d from Wine.
Despite the commit message there, even GCC 11.1 chokes on this.
For what it's worth, there's a single usage of RB_FOR_EACH_ENTRY in vkd3d, and it looks like something that doesn't particularly need it. We have generally preferred using rb_for_each_entry(), so we could also just get rid of these macros.
Is there a reason to prefer rb_for_each_entry()? It has always struck me as clumsier to program with callbacks, although I guess that's a matter of personal taste.
Yes, mostly a combination of style preferences and a general dislike of macros. (E.g., note that these macros, like many others, aren't safe from evaluating arguments multiple times.) Even when doing loop style iteration though, note that we could just use rb_head() and rb_next(), without necessarily needing these macros.
Signed-off-by: Henri Verbeet hverbeet@codeweavers.com
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- libs/vkd3d-shader/hlsl_codegen.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 75716bdf2..e43e6378e 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -724,6 +724,31 @@ static bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, voi { switch (expr->op) { + case HLSL_OP1_CAST: + if (instr->data_type->dimx != arg1->node.data_type->dimx + || instr->data_type->dimy != arg1->node.data_type->dimy) + { + FIXME("Cast from %s to %s.\n", debug_hlsl_type(ctx, arg1->node.data_type), + debug_hlsl_type(ctx, instr->data_type)); + vkd3d_free(res); + return false; + } + + switch (arg1->node.data_type->base_type) + { + case HLSL_TYPE_INT: + for (i = 0; i < dimx; ++i) + res->value[i].i = arg1->value[i].u; + break; + + default: + FIXME("Cast from %s to %s.\n", debug_hlsl_type(ctx, arg1->node.data_type), + debug_hlsl_type(ctx, instr->data_type)); + vkd3d_free(res); + return false; + } + break; + case HLSL_OP1_NEG: for (i = 0; i < instr->data_type->dimx; ++i) res->value[i].u = -arg1->value[i].u;
Signed-off-by: Giovanni Mascellani gmascellani@codeweavers.com
On 17/12/21 18:48, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
libs/vkd3d-shader/hlsl_codegen.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 75716bdf2..e43e6378e 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -724,6 +724,31 @@ static bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, voi { switch (expr->op) {
case HLSL_OP1_CAST:if (instr->data_type->dimx != arg1->node.data_type->dimx|| instr->data_type->dimy != arg1->node.data_type->dimy){FIXME("Cast from %s to %s.\n", debug_hlsl_type(ctx, arg1->node.data_type),debug_hlsl_type(ctx, instr->data_type));vkd3d_free(res);return false;}switch (arg1->node.data_type->base_type){case HLSL_TYPE_INT:for (i = 0; i < dimx; ++i)res->value[i].i = arg1->value[i].u;break;default:FIXME("Cast from %s to %s.\n", debug_hlsl_type(ctx, arg1->node.data_type),debug_hlsl_type(ctx, instr->data_type));vkd3d_free(res);return false;}break;case HLSL_OP1_NEG: for (i = 0; i < instr->data_type->dimx; ++i) res->value[i].u = -arg1->value[i].u;
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- libs/vkd3d-shader/hlsl_codegen.c | 34 ++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index e43e6378e..7f1829d2c 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -652,7 +652,36 @@ static bool lower_narrowing_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *ins return false; }
-static bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +static bool fold_constant_swizzles(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +{ + struct hlsl_ir_constant *value, *res; + struct hlsl_ir_swizzle *swizzle; + unsigned int i, swizzle_bits; + + if (instr->type != HLSL_IR_SWIZZLE) + return false; + swizzle = hlsl_ir_swizzle(instr); + if (swizzle->val.node->type != HLSL_IR_CONSTANT) + return false; + value = hlsl_ir_constant(swizzle->val.node); + + if (!(res = hlsl_alloc(ctx, sizeof(*res)))) + return false; + init_node(&res->node, HLSL_IR_CONSTANT, instr->data_type, instr->loc); + + swizzle_bits = swizzle->swizzle; + for (i = 0; i < swizzle->node.data_type->dimx; ++i) + { + res->value[i] = value->value[swizzle_bits & 3]; + swizzle_bits >>= 2; + } + + list_add_before(&swizzle->node.entry, &res->node.entry); + replace_node(&swizzle->node, &res->node); + return true; +} + +static bool fold_constant_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) { struct hlsl_ir_constant *arg1, *arg2 = NULL, *res; struct hlsl_ir_expr *expr; @@ -1733,7 +1762,8 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun transform_ir(ctx, lower_narrowing_casts, body, NULL); do { - progress = transform_ir(ctx, fold_constants, body, NULL); + progress = transform_ir(ctx, fold_constant_exprs, body, NULL); + progress |= transform_ir(ctx, fold_constant_swizzles, body, NULL); progress |= copy_propagation_execute(ctx, body); } while (progress);
Sorry, I just noticed this causes a test crash. Please ignore this patch for now; the rest should apply independently.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- Makefile.am | 2 ++ tests/hlsl-intrinsic-override.shader_test | 31 +++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tests/hlsl-intrinsic-override.shader_test
diff --git a/Makefile.am b/Makefile.am index 20fee06d0..0c2fbf7d7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -68,6 +68,7 @@ vkd3d_shader_tests = \ tests/hlsl-duplicate-modifiers.shader_test \ tests/hlsl-for.shader_test \ tests/hlsl-function-overload.shader_test \ + tests/hlsl-intrinsic-override.shader_test \ tests/hlsl-invalid.shader_test \ tests/hlsl-majority-pragma.shader_test \ tests/hlsl-majority-typedef.shader_test \ @@ -293,6 +294,7 @@ XFAIL_TESTS = \ tests/hlsl-duplicate-modifiers.shader_test \ tests/hlsl-for.shader_test \ tests/hlsl-function-overload.shader_test \ + tests/hlsl-intrinsic-override.shader_test \ tests/hlsl-majority-pragma.shader_test \ tests/hlsl-majority-typedef.shader_test \ tests/hlsl-nested-arrays.shader_test \ diff --git a/tests/hlsl-intrinsic-override.shader_test b/tests/hlsl-intrinsic-override.shader_test new file mode 100644 index 000000000..55a23f21d --- /dev/null +++ b/tests/hlsl-intrinsic-override.shader_test @@ -0,0 +1,31 @@ +[pixel shader] + +float2 max(float2 a, float2 b) +{ + return a + b; +} + +float4 main() : sv_target +{ + return float4(max(0.1, 0.2), max(float2(0.1, 0.2), float2(0.3, 0.4))); +} + +[test] +draw quad +probe all rgba (0.3, 0.3, 0.4, 0.6) + +[pixel shader] + +float2 max(float2 a, float3 b) +{ + return a + b.xy; +} + +float4 main() : sv_target +{ + return float4(max(0.1, 0.2), max(float2(0.1, 0.2), float2(0.3, 0.4))); +} + +[test] +draw quad +probe all rgba (0.3, 0.3, 0.3, 0.4)
Signed-off-by: Giovanni Mascellani gmascellani@codeweavers.com
On 17/12/21 18:48, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
Makefile.am | 2 ++ tests/hlsl-intrinsic-override.shader_test | 31 +++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tests/hlsl-intrinsic-override.shader_test
diff --git a/Makefile.am b/Makefile.am index 20fee06d0..0c2fbf7d7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -68,6 +68,7 @@ vkd3d_shader_tests = \ tests/hlsl-duplicate-modifiers.shader_test \ tests/hlsl-for.shader_test \ tests/hlsl-function-overload.shader_test \
- tests/hlsl-intrinsic-override.shader_test \ tests/hlsl-invalid.shader_test \ tests/hlsl-majority-pragma.shader_test \ tests/hlsl-majority-typedef.shader_test \
@@ -293,6 +294,7 @@ XFAIL_TESTS = \ tests/hlsl-duplicate-modifiers.shader_test \ tests/hlsl-for.shader_test \ tests/hlsl-function-overload.shader_test \
- tests/hlsl-intrinsic-override.shader_test \ tests/hlsl-majority-pragma.shader_test \ tests/hlsl-majority-typedef.shader_test \ tests/hlsl-nested-arrays.shader_test \
diff --git a/tests/hlsl-intrinsic-override.shader_test b/tests/hlsl-intrinsic-override.shader_test new file mode 100644 index 000000000..55a23f21d --- /dev/null +++ b/tests/hlsl-intrinsic-override.shader_test @@ -0,0 +1,31 @@ +[pixel shader]
+float2 max(float2 a, float2 b) +{
- return a + b;
+}
+float4 main() : sv_target +{
- return float4(max(0.1, 0.2), max(float2(0.1, 0.2), float2(0.3, 0.4)));
+}
+[test] +draw quad +probe all rgba (0.3, 0.3, 0.4, 0.6)
+[pixel shader]
+float2 max(float2 a, float3 b) +{
- return a + b.xy;
+}
+float4 main() : sv_target +{
- return float4(max(0.1, 0.2), max(float2(0.1, 0.2), float2(0.3, 0.4)));
+}
+[test] +draw quad +probe all rgba (0.3, 0.3, 0.3, 0.4)
Signed-off-by: Giovanni Mascellani gmascellani@codeweavers.com
On 17/12/21 18:48, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
libs/vkd3d-shader/hlsl.y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 636882c4b..5a17449a3 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -326,7 +326,7 @@ static bool append_conditional_break(struct hlsl_ctx *ctx, struct list *cond_lis struct hlsl_ir_if *iff;
/* E.g. "for (i = 0; ; ++i)". */
- if (!list_count(cond_list))
if (list_empty(cond_list)) return true;
condition = node_from_list(cond_list);
-
Giovanni Mascellani -
Henri Verbeet -
Henri Verbeet -
Zebediah Figura -
Zebediah Figura (she/her)