[PATCH vkd3d v2 01/10] vkd3d-shader/hlsl: Introduce a helper for getting numeric types.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 4 ++-- libs/vkd3d-shader/hlsl.h | 28 ++++++++++++++++++++++++++++ libs/vkd3d-shader/hlsl.y | 28 ++++++++++++---------------- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 9bce6beb..8a56025d 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -560,7 +560,7 @@ struct hlsl_ir_constant *hlsl_new_uint_constant(struct hlsl_ctx *ctx, unsigned i if (!(c = hlsl_alloc(ctx, sizeof(*c)))) return NULL; - init_node(&c->node, HLSL_IR_CONSTANT, ctx->builtin_types.scalar[HLSL_TYPE_UINT], loc); + init_node(&c->node, HLSL_IR_CONSTANT, hlsl_get_scalar_type(ctx, HLSL_TYPE_UINT), loc); c->value[0].u = n; return c; } @@ -650,7 +650,7 @@ struct hlsl_ir_swizzle *hlsl_new_swizzle(struct hlsl_ctx *ctx, DWORD s, unsigned if (!(swizzle = hlsl_alloc(ctx, sizeof(*swizzle)))) return NULL; init_node(&swizzle->node, HLSL_IR_SWIZZLE, - ctx->builtin_types.vector[val->data_type->base_type][components - 1], *loc); + hlsl_get_vector_type(ctx, val->data_type->base_type, components), *loc); hlsl_src_from_node(&swizzle->val, val); swizzle->swizzle = s; return swizzle; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index e401971e..31b7ddbb 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -629,6 +629,34 @@ static inline void hlsl_release_string_buffer(struct hlsl_ctx *ctx, struct vkd3d vkd3d_string_buffer_release(&ctx->string_buffers, buffer); } +static inline struct hlsl_type *hlsl_get_scalar_type(const struct hlsl_ctx *ctx, enum hlsl_base_type base_type) +{ + return ctx->builtin_types.scalar[base_type]; +} + +static inline struct hlsl_type *hlsl_get_vector_type(const struct hlsl_ctx *ctx, enum hlsl_base_type base_type, + unsigned int dimx) +{ + return ctx->builtin_types.vector[base_type][dimx - 1]; +} + +static inline struct hlsl_type *hlsl_get_matrix_type(const struct hlsl_ctx *ctx, enum hlsl_base_type base_type, + unsigned int dimx, unsigned int dimy) +{ + return ctx->builtin_types.matrix[base_type][dimx - 1][dimy - 1]; +} + +static inline struct hlsl_type *hlsl_get_numeric_type(const struct hlsl_ctx *ctx, enum hlsl_type_class type, + enum hlsl_base_type base_type, unsigned int dimx, unsigned int dimy) +{ + if (type == HLSL_CLASS_SCALAR) + return hlsl_get_scalar_type(ctx, base_type); + else if (type == HLSL_CLASS_VECTOR) + return hlsl_get_vector_type(ctx, base_type, dimx); + else + return hlsl_get_matrix_type(ctx, base_type, dimx, dimy); +} + const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op); const char *debug_hlsl_type(struct hlsl_ctx *ctx, const struct hlsl_type *type); const char *debug_hlsl_writemask(unsigned int writemask); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ae19cd1a..61789def 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1021,11 +1021,7 @@ static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type } } - if (type == HLSL_CLASS_SCALAR) - return ctx->builtin_types.scalar[base]; - if (type == HLSL_CLASS_VECTOR) - return ctx->builtin_types.vector[base][dimx - 1]; - return ctx->builtin_types.matrix[base][dimx - 1][dimy - 1]; + return hlsl_get_numeric_type(ctx, type, base, dimx, dimy); } static struct hlsl_ir_expr *add_expr(struct hlsl_ctx *ctx, struct list *instrs, enum hlsl_ir_expr_op op, @@ -1761,7 +1757,7 @@ static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type *type } if (!(arg = add_implicit_conversion(ctx, params->instrs, arg, - ctx->builtin_types.vector[type->base_type][width - 1], &arg->loc))) + hlsl_get_vector_type(ctx, type->base_type, width), &arg->loc))) continue; if (!(store = hlsl_new_store(ctx, var, NULL, arg, @@ -1816,9 +1812,9 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl if (params->args_count >= 2) FIXME("Ignoring index and/or offset parameter(s).\n"); - /* -1 for zero-indexing; +1 for the mipmap level */ + /* +1 for the mipmap level */ if (!(coords = add_implicit_conversion(ctx, instrs, params->args[0], - ctx->builtin_types.vector[HLSL_TYPE_INT][sampler_dim - 1 + 1], loc))) + hlsl_get_vector_type(ctx, HLSL_TYPE_INT, sampler_dim + 1), loc))) return false; if (!(load = hlsl_new_resource_load(ctx, object_type->e.resource_format, HLSL_RESOURCE_LOAD, @@ -2500,7 +2496,7 @@ type: YYABORT; } - $$ = ctx->builtin_types.vector[$3->base_type][$5 - 1]; + $$ = hlsl_get_vector_type(ctx, $3->base_type, $5); } | KW_MATRIX '<' type ',' C_INTEGER ',' C_INTEGER '>' { @@ -2528,7 +2524,7 @@ type: YYABORT; } - $$ = ctx->builtin_types.matrix[$3->base_type][$7 - 1][$5 - 1]; + $$ = hlsl_get_matrix_type(ctx, $3->base_type, $7, $5); } | KW_VOID { @@ -2560,7 +2556,7 @@ type: } | texture_type { - $$ = hlsl_new_texture_type(ctx, $1, ctx->builtin_types.vector[HLSL_TYPE_FLOAT][4 - 1]); + $$ = hlsl_new_texture_type(ctx, $1, hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, 4)); } | texture_type '<' type '>' { @@ -2971,7 +2967,7 @@ primary_expr: if (!(c = hlsl_alloc(ctx, sizeof(*c)))) YYABORT; - init_node(&c->node, HLSL_IR_CONSTANT, ctx->builtin_types.scalar[HLSL_TYPE_FLOAT], @1); + init_node(&c->node, HLSL_IR_CONSTANT, hlsl_get_scalar_type(ctx, HLSL_TYPE_FLOAT), @1); c->value[0].f = $1; if (!($$ = make_list(ctx, &c->node))) YYABORT; @@ -2982,7 +2978,7 @@ primary_expr: if (!(c = hlsl_alloc(ctx, sizeof(*c)))) YYABORT; - init_node(&c->node, HLSL_IR_CONSTANT, ctx->builtin_types.scalar[HLSL_TYPE_INT], @1); + init_node(&c->node, HLSL_IR_CONSTANT, hlsl_get_scalar_type(ctx, HLSL_TYPE_INT), @1); c->value[0].i = $1; if (!($$ = make_list(ctx, &c->node))) YYABORT; @@ -2993,7 +2989,7 @@ primary_expr: if (!(c = hlsl_alloc(ctx, sizeof(*c)))) YYABORT; - init_node(&c->node, HLSL_IR_CONSTANT, ctx->builtin_types.scalar[HLSL_TYPE_BOOL], @1); + init_node(&c->node, HLSL_IR_CONSTANT, hlsl_get_scalar_type(ctx, HLSL_TYPE_BOOL), @1); c->value[0].b = $1; if (!($$ = make_list(ctx, &c->node))) YYABORT; @@ -3030,7 +3026,7 @@ primary_expr: struct hlsl_ir_var *var; if (!(var = hlsl_new_synthetic_var(ctx, "<state-block-expr>", - ctx->builtin_types.scalar[HLSL_TYPE_INT], @1))) + hlsl_get_scalar_type(ctx, HLSL_TYPE_INT), @1))) YYABORT; if (!(load = hlsl_new_var_load(ctx, var, @1))) YYABORT; @@ -3116,7 +3112,7 @@ postfix_expr: YYABORT; } - if (!(cast = hlsl_new_cast(ctx, index, ctx->builtin_types.scalar[HLSL_TYPE_UINT], &index->loc))) + if (!(cast = hlsl_new_cast(ctx, index, hlsl_get_scalar_type(ctx, HLSL_TYPE_UINT), &index->loc))) { destroy_instr_list($1); YYABORT; -- 2.33.0
Function expr_common_shape can be used for boolean operators, for which a common shape must be determined even if the base type of the result is always bool. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 82 +++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 38 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 61789def..5f320323 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -912,13 +912,9 @@ static enum hlsl_base_type expr_common_base_type(enum hlsl_base_type t1, enum hl return HLSL_TYPE_INT; } -static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct hlsl_type *t2, - struct vkd3d_shader_location *loc) +static bool expr_common_shape(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct hlsl_type *t2, + struct vkd3d_shader_location *loc, enum hlsl_type_class *type, unsigned int *dimx, unsigned int *dimy) { - enum hlsl_type_class type; - enum hlsl_base_type base; - unsigned int dimx, dimy; - if (t1->type > HLSL_CLASS_LAST_NUMERIC) { struct vkd3d_string_buffer *string; @@ -927,7 +923,7 @@ static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Expression of type \"%s\" cannot be used in a numeric expression.", string->buffer); hlsl_release_string_buffer(ctx, string); - return NULL; + return false; } if (t2->type > HLSL_CLASS_LAST_NUMERIC) @@ -938,12 +934,9 @@ static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Expression of type \"%s\" cannot be used in a numeric expression.", string->buffer); hlsl_release_string_buffer(ctx, string); - return NULL; + return false; } - if (hlsl_types_are_equal(t1, t2)) - return t1; - if (!expr_compatible_data_types(t1, t2)) { struct vkd3d_string_buffer *t1_string = hlsl_type_to_string(ctx, t1); @@ -955,28 +948,26 @@ static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type t1_string->buffer, t2_string->buffer); hlsl_release_string_buffer(ctx, t1_string); hlsl_release_string_buffer(ctx, t2_string); - return NULL; + return false; } - base = expr_common_base_type(t1->base_type, t2->base_type); - if (t1->dimx == 1 && t1->dimy == 1) { - type = t2->type; - dimx = t2->dimx; - dimy = t2->dimy; + *type = t2->type; + *dimx = t2->dimx; + *dimy = t2->dimy; } else if (t2->dimx == 1 && t2->dimy == 1) { - type = t1->type; - dimx = t1->dimx; - dimy = t1->dimy; + *type = t1->type; + *dimx = t1->dimx; + *dimy = t1->dimy; } else if (t1->type == HLSL_CLASS_MATRIX && t2->type == HLSL_CLASS_MATRIX) { - type = HLSL_CLASS_MATRIX; - dimx = min(t1->dimx, t2->dimx); - dimy = min(t1->dimy, t2->dimy); + *type = HLSL_CLASS_MATRIX; + *dimx = min(t1->dimx, t2->dimx); + *dimy = min(t1->dimy, t2->dimy); } else { @@ -987,40 +978,55 @@ static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type max_dim_2 = max(t2->dimx, t2->dimy); if (t1->dimx * t1->dimy == t2->dimx * t2->dimy) { - type = HLSL_CLASS_VECTOR; - dimx = max(t1->dimx, t2->dimx); - dimy = 1; + *type = HLSL_CLASS_VECTOR; + *dimx = max(t1->dimx, t2->dimx); + *dimy = 1; } else if (max_dim_1 <= max_dim_2) { - type = t1->type; - if (type == HLSL_CLASS_VECTOR) + *type = t1->type; + if (*type == HLSL_CLASS_VECTOR) { - dimx = max_dim_1; - dimy = 1; + *dimx = max_dim_1; + *dimy = 1; } else { - dimx = t1->dimx; - dimy = t1->dimy; + *dimx = t1->dimx; + *dimy = t1->dimy; } } else { - type = t2->type; - if (type == HLSL_CLASS_VECTOR) + *type = t2->type; + if (*type == HLSL_CLASS_VECTOR) { - dimx = max_dim_2; - dimy = 1; + *dimx = max_dim_2; + *dimy = 1; } else { - dimx = t2->dimx; - dimy = t2->dimy; + *dimx = t2->dimx; + *dimy = t2->dimy; } } } + return true; +} + +static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct hlsl_type *t2, + struct vkd3d_shader_location *loc) +{ + enum hlsl_type_class type; + enum hlsl_base_type base; + unsigned int dimx, dimy; + + if (!expr_common_shape(ctx, t1, t2, loc, &type, &dimx, &dimy)) + return NULL; + + base = expr_common_base_type(t1->base_type, t2->base_type); + return hlsl_get_numeric_type(ctx, type, base, dimx, dimy); } -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> --- FWIW I don't necessarily see introducing a struct for the 3 out parameters all that bad, but this also works for me.
When t1 is a vector type, it's already supposed to have dimx == max_dim_1 and dimy == 1, and the same for t2. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 5f320323..d2cc1878 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -985,30 +985,14 @@ static bool expr_common_shape(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct else if (max_dim_1 <= max_dim_2) { *type = t1->type; - if (*type == HLSL_CLASS_VECTOR) - { - *dimx = max_dim_1; - *dimy = 1; - } - else - { - *dimx = t1->dimx; - *dimy = t1->dimy; - } + *dimx = t1->dimx; + *dimy = t1->dimy; } else { *type = t2->type; - if (*type == HLSL_CLASS_VECTOR) - { - *dimx = max_dim_2; - *dimy = 1; - } - else - { - *dimx = t2->dimx; - *dimy = t2->dimy; - } + *dimx = t2->dimx; + *dimy = t2->dimy; } } -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
For example, the common shape between a matrix 1x4 and a matrix 2x4 is a vector of length 4, not a matrix 1x4. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index d2cc1878..bfe2bbb9 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -996,6 +996,9 @@ static bool expr_common_shape(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct } } + if (*type == HLSL_CLASS_MATRIX && *dimy == 1) + *type = HLSL_CLASS_VECTOR; + return true; } -- 2.33.0
On 10/15/21 03:36, Giovanni Mascellani wrote:
For example, the common shape between a matrix 1x4 and a matrix 2x4 is a vector of length 4, not a matrix 1x4.
What leads you to this conclusion? Even the most obvious case I checked suggests this isn't true: --- uniform float2x4 x; uniform float1x4 y; float4 main() : sv_target { return (x + y).xyzw; } --- Native d3dcompiler_47 fails here with: Z:\home\meg\test.hlsl(6,13-17): warning X3206: implicit truncation of vector type Z:\home\meg\test.hlsl(6,12-23): error X3018: invalid subscript 'xyzw' Z:\home\meg\test.hlsl(6,5-24): error X3080: 'main': function must return a value Same if I swap the order of addition, or the x and y dimensions. Returning (x + y) works fine, but as I understand that's because an implicit cast from float1x4 to float4 is valid.
Hi, Il 15/10/21 19:47, Zebediah Figura (she/her) ha scritto:
What leads you to this conclusion? Even the most obvious case I checked suggests this isn't true:
---
uniform float2x4 x; uniform float1x4 y;
float4 main() : sv_target { return (x + y).xyzw; }
---
Ah, interesting. My source was this: http://shader-playground.timjones.io/cfaa21ebc1961e76a302e48e87a7a033 The error message says: "error X3017: cannot convert from 'const const float4' to 'texture'". But clearly I was too optimistic about the self consistency of the native compiler. I think for the moment it is safe to drop this patch. Unfortunately it is referenced in the commit message of the following one (05/10), so I'll resend the whole series again. The content of 05/10 still seems to be valid even when tested with swizzling: the type of x+y is the type of x when x and y are a vector and a matrix (in some order) and have the same number of entries. Thanks, Giovanni.
On 10/18/21 02:24, Giovanni Mascellani wrote:
Hi,
Il 15/10/21 19:47, Zebediah Figura (she/her) ha scritto:
What leads you to this conclusion? Even the most obvious case I checked suggests this isn't true:
---
uniform float2x4 x; uniform float1x4 y;
float4 main() : sv_target { return (x + y).xyzw; }
---
Ah, interesting. My source was this:
http://shader-playground.timjones.io/cfaa21ebc1961e76a302e48e87a7a033
The error message says: "error X3017: cannot convert from 'const const float4' to 'texture'". But clearly I was too optimistic about the self consistency of the native compiler.
I get a similar message with "(texture)y". I also get a similar message with the following shader: --- void func(float1x4 arg) { } uniform Texture2D t; float4 main() : sv_target { func(t); return 0; } --- I suspect what happens is that native just reports float1xN as floatN in error messages, but fundamentally it's still a matrix type.
I think for the moment it is safe to drop this patch. Unfortunately it is referenced in the commit message of the following one (05/10), so I'll resend the whole series again. The content of 05/10 still seems to be valid even when tested with swizzling: the type of x+y is the type of x when x and y are a vector and a matrix (in some order) and have the same number of entries.
I don't think anything else depends on 5/10, right? It should be fine just to resend that patch alone.
The assumption about the size of matrices is not correct: it is legitimate to compose a matrix 2x2 with a vector of length 4, in which case it appears that the result has the shape of the first (leftmost) operand. Even for matrices 1xN or Nx1, the result is not always a vector: in general it has the shape of the first operand again, which gets possibly decayed to a vector. This algorithm is unfortunately not complete yet: sometimes, but not always, vectors of length 1 are decayed to scalars. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index bfe2bbb9..c42e372e 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -971,18 +971,7 @@ static bool expr_common_shape(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct } else { - /* Two vectors or a vector and a matrix (matrix must be 1xn or nx1) */ - unsigned int max_dim_1, max_dim_2; - - max_dim_1 = max(t1->dimx, t1->dimy); - max_dim_2 = max(t2->dimx, t2->dimy); - if (t1->dimx * t1->dimy == t2->dimx * t2->dimy) - { - *type = HLSL_CLASS_VECTOR; - *dimx = max(t1->dimx, t2->dimx); - *dimy = 1; - } - else if (max_dim_1 <= max_dim_2) + if (t1->dimx * t1->dimy <= t2->dimx * t2->dimy) { *type = t1->type; *dimx = t1->dimx; -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
HLSL seems to treat matrices 1xN or Nx1 as vectors when looking for implicit conversions. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index c42e372e..0ed09732 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -240,14 +240,22 @@ static bool implicit_compatible_data_types(struct hlsl_type *t1, struct hlsl_typ if (t1->type == HLSL_CLASS_MATRIX || t2->type == HLSL_CLASS_MATRIX) { - if (t1->type == HLSL_CLASS_MATRIX && t2->type == HLSL_CLASS_MATRIX - && t1->dimx >= t2->dimx && t1->dimy >= t2->dimy) - return true; + if (t1->type == HLSL_CLASS_MATRIX && t2->type == HLSL_CLASS_MATRIX) + return t1->dimx >= t2->dimx && t1->dimy >= t2->dimy; + + /* Matrix-vector conversion is apparently allowed if they have + * the same components count, or if the matrix is 1xN or Nx1 + * and we are reducing the component count */ + if (t1->type == HLSL_CLASS_VECTOR || t2->type == HLSL_CLASS_VECTOR) + { + if (hlsl_type_component_count(t1) == hlsl_type_component_count(t2)) + return true; + + if ((t1->type == HLSL_CLASS_VECTOR || t1->dimx == 1 || t1->dimy == 1) && + (t2->type == HLSL_CLASS_VECTOR || t2->dimx == 1 || t2->dimy == 1)) + return hlsl_type_component_count(t1) >= hlsl_type_component_count(t2); + } - /* Matrix-vector conversion is apparently allowed if they have the same components count */ - if ((t1->type == HLSL_CLASS_VECTOR || t2->type == HLSL_CLASS_VECTOR) - && hlsl_type_component_count(t1) == hlsl_type_component_count(t2)) - return true; return false; } -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- Although with the comment that it'd be nice to see some specific tests for these. I manually verified that this logic holds for three of the four places where we currently use add_implicit_conversion (viz. return values, assignments, and constructors); it doesn't hold for the fourth (the argument to Load) but that's probably not worth fixing.
On Fri, Oct 15, 2021 at 10:47 PM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
Although with the comment that it'd be nice to see some specific tests for these. I manually verified that this logic holds for three of the four places where we currently use add_implicit_conversion (viz. return values, assignments, and constructors); it doesn't hold for the fourth (the argument to Load) but that's probably not worth fixing.
Right, that seems to be a quirk that we probably don't want to replicate unless really necessary. Also, yes, tests are always welcome.
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> --- Tiny note: the comment mentions "reducing the component count" but technically it's also okay if there is no reduction (thus the >=). I'm wondering if there is some way to say that in a non-awkward manner...
Hi, Il 21/10/21 16:17, Matteo Bruni ha scritto:
Tiny note: the comment mentions "reducing the component count" but technically it's also okay if there is no reduction (thus the >=). I'm wondering if there is some way to say that in a non-awkward manner...
Well, in the comment there is a disjunction: same component count || (another condition && reducing component count). That seems to me a relatively straightforward way to say that. Giovanni.
On Thu, Oct 21, 2021 at 4:55 PM Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
Hi,
Il 21/10/21 16:17, Matteo Bruni ha scritto:
Tiny note: the comment mentions "reducing the component count" but technically it's also okay if there is no reduction (thus the >=). I'm wondering if there is some way to say that in a non-awkward manner...
Well, in the comment there is a disjunction: same component count || (another condition && reducing component count). That seems to me a relatively straightforward way to say that.
Giovanni.
I guess I misread it.
This commit moves the logic for casting operands to a common type out of add_expr, so that different helpers can use different logics (corresponding to the different typing rules used by e.g. arithmetic, comparison or bitwise operations). Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 71 ++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 0ed09732..7bbf0318 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1072,14 +1072,38 @@ static struct list *add_unary_expr(struct hlsl_ctx *ctx, struct list *instrs, return instrs; } -static struct list *add_binary_expr(struct hlsl_ctx *ctx, struct list *list1, struct list *list2, +static struct hlsl_ir_expr *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, + struct vkd3d_shader_location *loc) +{ + struct hlsl_type *common_type; + enum hlsl_base_type base = expr_common_base_type(arg1->data_type->base_type, arg2->data_type->base_type); + enum hlsl_type_class type; + unsigned int dimx, dimy; + struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {0}; + + if (!expr_common_shape(ctx, arg1->data_type, arg2->data_type, loc, &type, &dimx, &dimy)) + return NULL; + + common_type = hlsl_get_numeric_type(ctx, type, base, dimx, dimy); + + if (!(args[0] = add_implicit_conversion(ctx, instrs, arg1, common_type, loc))) + return NULL; + + if (!(args[1] = add_implicit_conversion(ctx, instrs, arg2, common_type, loc))) + return NULL; + + return add_expr(ctx, instrs, op, args, loc); +} + +static struct list *add_binary_arithmetic_expr_merge(struct hlsl_ctx *ctx, struct list *list1, struct list *list2, enum hlsl_ir_expr_op op, struct vkd3d_shader_location loc) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {node_from_list(list1), node_from_list(list2)}; + struct hlsl_ir_node *arg1 = node_from_list(list1), *arg2 = node_from_list(list2); list_move_tail(list1, list2); vkd3d_free(list2); - add_expr(ctx, list1, op, args, &loc); + add_binary_arithmetic_expr(ctx, list1, op, arg1, arg2, &loc); return list1; } @@ -1159,12 +1183,11 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in } if (assign_op != ASSIGN_OP_ASSIGN) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {lhs, rhs}; enum hlsl_ir_expr_op op = op_from_assignment(assign_op); struct hlsl_ir_expr *expr; assert(op); - if (!(expr = add_expr(ctx, instrs, op, args, &rhs->loc))) + if (!(expr = add_binary_arithmetic_expr(ctx, instrs, op, lhs, rhs, &rhs->loc))) return NULL; rhs = &expr->node; } @@ -1573,29 +1596,23 @@ static bool intrinsic_abs(struct hlsl_ctx *ctx, static bool intrinsic_clamp(struct hlsl_ctx *ctx, const struct parse_initializer *params, struct vkd3d_shader_location loc) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {params->args[0], params->args[1]}; struct hlsl_ir_expr *max; - if (!(max = add_expr(ctx, params->instrs, HLSL_OP2_MAX, args, &loc))) + if (!(max = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MAX, params->args[0], params->args[1], &loc))) return false; - args[0] = &max->node; - args[1] = params->args[2]; - return !!add_expr(ctx, params->instrs, HLSL_OP2_MIN, args, &loc); + return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MIN, &max->node, params->args[2], &loc); } static bool intrinsic_max(struct hlsl_ctx *ctx, const struct parse_initializer *params, struct vkd3d_shader_location loc) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {params->args[0], params->args[1]}; - - return !!add_expr(ctx, params->instrs, HLSL_OP2_MAX, args, &loc); + return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MAX, params->args[0], params->args[1], &loc); } static bool intrinsic_pow(struct hlsl_ctx *ctx, const struct parse_initializer *params, struct vkd3d_shader_location loc) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {0}; struct hlsl_ir_node *log, *exp; struct hlsl_ir_expr *mul; @@ -1603,9 +1620,7 @@ static bool intrinsic_pow(struct hlsl_ctx *ctx, return false; list_add_tail(params->instrs, &log->entry); - args[0] = params->args[1]; - args[1] = log; - if (!(mul = add_expr(ctx, params->instrs, HLSL_OP2_MUL, args, &loc))) + if (!(mul = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, params->args[1], log, &loc))) return false; if (!(exp = hlsl_new_unary_expr(ctx, HLSL_OP1_EXP2, &mul->node, loc))) @@ -3252,22 +3267,22 @@ mul_expr: unary_expr | mul_expr '*' unary_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_MUL, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_MUL, @2); } | mul_expr '/' unary_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_DIV, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_DIV, @2); } | mul_expr '%' unary_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_MOD, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_MOD, @2); } add_expr: mul_expr | add_expr '+' mul_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_ADD, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_ADD, @2); } | add_expr '-' mul_expr { @@ -3276,7 +3291,7 @@ add_expr: if (!(neg = hlsl_new_unary_expr(ctx, HLSL_OP1_NEG, node_from_list($3), @2))) YYABORT; list_add_tail($3, &neg->entry); - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_ADD, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_ADD, @2); } shift_expr: @@ -3294,30 +3309,30 @@ relational_expr: shift_expr | relational_expr '<' shift_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_LESS, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_LESS, @2); } | relational_expr '>' shift_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_GREATER, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_GREATER, @2); } | relational_expr OP_LE shift_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_LEQUAL, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_LEQUAL, @2); } | relational_expr OP_GE shift_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_GEQUAL, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_GEQUAL, @2); } equality_expr: relational_expr | equality_expr OP_EQ relational_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_EQUAL, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_EQUAL, @2); } | equality_expr OP_NE relational_expr { - $$ = add_binary_expr(ctx, $1, $3, HLSL_OP2_NEQUAL, @2); + $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_NEQUAL, @2); } bitand_expr: -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Fri, Oct 15, 2021 at 4:34 PM Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
This commit moves the logic for casting operands to a common type out of add_expr, so that different helpers can use different logics (corresponding to the different typing rules used by e.g. arithmetic, comparison or bitwise operations).
Technically this isn't quite the case anymore, add_expr() is still there unchanged after this patch. Some rewording might be in order.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 71 ++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 28 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 0ed09732..7bbf0318 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1072,14 +1072,38 @@ static struct list *add_unary_expr(struct hlsl_ctx *ctx, struct list *instrs, return instrs; }
-static struct list *add_binary_expr(struct hlsl_ctx *ctx, struct list *list1, struct list *list2, +static struct hlsl_ir_expr *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, + struct vkd3d_shader_location *loc) +{ + struct hlsl_type *common_type; + enum hlsl_base_type base = expr_common_base_type(arg1->data_type->base_type, arg2->data_type->base_type); + enum hlsl_type_class type; + unsigned int dimx, dimy; + struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {0};
Style nitpick: we usually order declarations according to descending line length when possible.
Hi, Il 21/10/21 16:21, Matteo Bruni ha scritto:
Technically this isn't quite the case anymore, add_expr() is still there unchanged after this patch. Some rewording might be in order.
My reasoning here is that with this patch the casting logic in add_expr() is not doing anything any more, because the caller already feeds add_expr() arguments of the same type. So even if add_expr() wasn't touched, the logic has moved. I wasn't sure of how to say this without getting into a uselessly long sentence. Giovanni.
On Thu, Oct 21, 2021 at 4:58 PM Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
Hi,
Il 21/10/21 16:21, Matteo Bruni ha scritto:
Technically this isn't quite the case anymore, add_expr() is still there unchanged after this patch. Some rewording might be in order.
My reasoning here is that with this patch the casting logic in add_expr() is not doing anything any more, because the caller already feeds add_expr() arguments of the same type. So even if add_expr() wasn't touched, the logic has moved. I wasn't sure of how to say this without getting into a uselessly long sentence.
I guess it might just be a matter of replacing "moves" with "copies" or "starts moving". BTW, I think "logic" is uncountable.
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
The helper doesn't do much, but it is useful to mark operations as arithmetic as opposed to other categories (like bitwise and boolean), which have a different treatment. It also saves an explicit variable to most callers, which can directly pass the argument node instead of creating an array. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 7bbf0318..a50db4ef 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1061,15 +1061,12 @@ static struct hlsl_ir_expr *add_expr(struct hlsl_ctx *ctx, struct list *instrs, return expr; } -static struct list *add_unary_expr(struct hlsl_ctx *ctx, struct list *instrs, - enum hlsl_ir_expr_op op, struct vkd3d_shader_location loc) +static struct hlsl_ir_expr *add_unary_arithmetic_expr(struct hlsl_ctx *ctx, struct list *instrs, + enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg, struct vkd3d_shader_location *loc) { - struct hlsl_ir_node *expr; + struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {arg}; - if (!(expr = hlsl_new_unary_expr(ctx, op, node_from_list(instrs), loc))) - return NULL; - list_add_tail(instrs, &expr->entry); - return instrs; + return add_expr(ctx, instrs, op, args, loc); } static struct hlsl_ir_expr *add_binary_arithmetic_expr(struct hlsl_ctx *ctx, struct list *instrs, @@ -1173,10 +1170,9 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in if (assign_op == ASSIGN_OP_SUB) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {rhs}; struct hlsl_ir_expr *expr; - if (!(expr = add_expr(ctx, instrs, HLSL_OP1_NEG, args, &rhs->loc))) + if (!(expr = add_unary_arithmetic_expr(ctx, instrs, HLSL_OP1_NEG, rhs, &rhs->loc))) return NULL; rhs = &expr->node; assign_op = ASSIGN_OP_ADD; @@ -1588,9 +1584,7 @@ static const struct hlsl_ir_function_decl *find_function_call(struct hlsl_ctx *c static bool intrinsic_abs(struct hlsl_ctx *ctx, const struct parse_initializer *params, struct vkd3d_shader_location loc) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {params->args[0]}; - - return !!add_expr(ctx, params->instrs, HLSL_OP1_ABS, args, &loc); + return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_ABS, params->args[0], &loc); } static bool intrinsic_clamp(struct hlsl_ctx *ctx, @@ -1632,9 +1626,7 @@ static bool intrinsic_pow(struct hlsl_ctx *ctx, static bool intrinsic_saturate(struct hlsl_ctx *ctx, const struct parse_initializer *params, struct vkd3d_shader_location loc) { - struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {params->args[0]}; - - return !!add_expr(ctx, params->instrs, HLSL_OP1_SAT, args, &loc); + return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_SAT, params->args[0], &loc); } static const struct intrinsic_function @@ -3211,15 +3203,18 @@ unary_expr: } | '-' unary_expr { - $$ = add_unary_expr(ctx, $2, HLSL_OP1_NEG, @1); + add_unary_arithmetic_expr(ctx, $2, HLSL_OP1_NEG, node_from_list($2), &@1); + $$ = $2; } | '~' unary_expr { - $$ = add_unary_expr(ctx, $2, HLSL_OP1_BIT_NOT, @1); + add_unary_arithmetic_expr(ctx, $2, HLSL_OP1_BIT_NOT, node_from_list($2), &@1); + $$ = $2; } | '!' unary_expr { - $$ = add_unary_expr(ctx, $2, HLSL_OP1_LOGIC_NOT, @1); + add_unary_arithmetic_expr(ctx, $2, HLSL_OP1_LOGIC_NOT, node_from_list($2), &@1); + $$ = $2; } /* var_modifiers is necessary to avoid shift/reduce conflicts. */ | '(' var_modifiers type arrays ')' unary_expr -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Determining cast types and return type is now delegated to higher level helpers, which are differentiated according to the operation category. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 51 +++------------------------------------- 1 file changed, 3 insertions(+), 48 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index a50db4ef..f18e4e05 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -999,57 +999,12 @@ static bool expr_common_shape(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct return true; } -static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type *t1, struct hlsl_type *t2, - struct vkd3d_shader_location *loc) -{ - enum hlsl_type_class type; - enum hlsl_base_type base; - unsigned int dimx, dimy; - - if (!expr_common_shape(ctx, t1, t2, loc, &type, &dimx, &dimy)) - return NULL; - - base = expr_common_base_type(t1->base_type, t2->base_type); - - return hlsl_get_numeric_type(ctx, type, base, dimx, dimy); -} - static struct hlsl_ir_expr *add_expr(struct hlsl_ctx *ctx, struct list *instrs, enum hlsl_ir_expr_op op, - struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS], struct vkd3d_shader_location *loc) + struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS], struct hlsl_type *type, struct vkd3d_shader_location *loc) { struct hlsl_ir_expr *expr; - struct hlsl_type *type; unsigned int i; - type = operands[0]->data_type; - for (i = 1; i < HLSL_MAX_OPERANDS; ++i) - { - if (!operands[i]) - break; - type = expr_common_type(ctx, type, operands[i]->data_type, loc); - if (!type) - return NULL; - } - for (i = 0; i < HLSL_MAX_OPERANDS; ++i) - { - struct hlsl_ir_expr *cast; - - if (!operands[i]) - break; - if (hlsl_types_are_equal(operands[i]->data_type, type)) - continue; - if (operands[i]->data_type->dimx * operands[i]->data_type->dimy != 1 - && operands[i]->data_type->dimx * operands[i]->data_type->dimy != type->dimx * type->dimy) - hlsl_warning(ctx, operands[i]->loc, VKD3D_SHADER_WARNING_HLSL_IMPLICIT_TRUNCATION, - "Implicit truncation of %s type.", - operands[i]->data_type->type == HLSL_CLASS_VECTOR ? "vector" : "matrix"); - - if (!(cast = hlsl_new_cast(ctx, operands[i], type, &operands[i]->loc))) - return NULL; - list_add_after(&operands[i]->entry, &cast->node.entry); - operands[i] = &cast->node; - } - if (!(expr = hlsl_alloc(ctx, sizeof(*expr)))) return NULL; init_node(&expr->node, HLSL_IR_EXPR, type, *loc); @@ -1066,7 +1021,7 @@ static struct hlsl_ir_expr *add_unary_arithmetic_expr(struct hlsl_ctx *ctx, stru { struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {arg}; - return add_expr(ctx, instrs, op, args, loc); + return add_expr(ctx, instrs, op, args, arg->data_type, loc); } static struct hlsl_ir_expr *add_binary_arithmetic_expr(struct hlsl_ctx *ctx, struct list *instrs, @@ -1090,7 +1045,7 @@ static struct hlsl_ir_expr *add_binary_arithmetic_expr(struct hlsl_ctx *ctx, str if (!(args[1] = add_implicit_conversion(ctx, instrs, arg2, common_type, loc))) return NULL; - return add_expr(ctx, instrs, op, args, loc); + return add_expr(ctx, instrs, op, args, common_type, loc); } static struct list *add_binary_arithmetic_expr_merge(struct hlsl_ctx *ctx, struct list *list1, struct list *list2, -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Fri, Oct 15, 2021 at 1:04 PM Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
Determining cast types and return type is now delegated to higher level helpers, which are differentiated according to the operation category.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 51 +++------------------------------------- 1 file changed, 3 insertions(+), 48 deletions(-)
Very minor comment: for the patch subject I'd go with "vkd3d-shader/hlsl: Do not cast arguments in add_expr()."
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
The only difference from arithmetic operations is that the result has always base type bool. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 48 +++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index f18e4e05..c8991e5d 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1059,6 +1059,42 @@ static struct list *add_binary_arithmetic_expr_merge(struct hlsl_ctx *ctx, struc return list1; } +static struct hlsl_ir_expr *add_binary_comparison_expr(struct hlsl_ctx *ctx, struct list *instrs, + enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg1, struct hlsl_ir_node *arg2, + struct vkd3d_shader_location *loc) +{ + struct hlsl_type *common_type, *return_type; + enum hlsl_base_type base = expr_common_base_type(arg1->data_type->base_type, arg2->data_type->base_type); + enum hlsl_type_class type; + unsigned int dimx, dimy; + struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {0}; + + if (!expr_common_shape(ctx, arg1->data_type, arg2->data_type, loc, &type, &dimx, &dimy)) + return NULL; + + common_type = hlsl_get_numeric_type(ctx, type, base, dimx, dimy); + return_type = hlsl_get_numeric_type(ctx, type, HLSL_TYPE_BOOL, dimx, dimy); + + if (!(args[0] = add_implicit_conversion(ctx, instrs, arg1, common_type, loc))) + return NULL; + + if (!(args[1] = add_implicit_conversion(ctx, instrs, arg2, common_type, loc))) + return NULL; + + return add_expr(ctx, instrs, op, args, return_type, loc); +} + +static struct list *add_binary_comparison_expr_merge(struct hlsl_ctx *ctx, struct list *list1, struct list *list2, + enum hlsl_ir_expr_op op, struct vkd3d_shader_location loc) +{ + struct hlsl_ir_node *arg1 = node_from_list(list1), *arg2 = node_from_list(list2); + + list_move_tail(list1, list2); + vkd3d_free(list2); + add_binary_comparison_expr(ctx, list1, op, arg1, arg2, &loc); + return list1; +} + static enum hlsl_ir_expr_op op_from_assignment(enum parse_assign_op op) { static const enum hlsl_ir_expr_op ops[] = @@ -3259,30 +3295,30 @@ relational_expr: shift_expr | relational_expr '<' shift_expr { - $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_LESS, @2); + $$ = add_binary_comparison_expr_merge(ctx, $1, $3, HLSL_OP2_LESS, @2); } | relational_expr '>' shift_expr { - $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_GREATER, @2); + $$ = add_binary_comparison_expr_merge(ctx, $1, $3, HLSL_OP2_GREATER, @2); } | relational_expr OP_LE shift_expr { - $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_LEQUAL, @2); + $$ = add_binary_comparison_expr_merge(ctx, $1, $3, HLSL_OP2_LEQUAL, @2); } | relational_expr OP_GE shift_expr { - $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_GEQUAL, @2); + $$ = add_binary_comparison_expr_merge(ctx, $1, $3, HLSL_OP2_GEQUAL, @2); } equality_expr: relational_expr | equality_expr OP_EQ relational_expr { - $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_EQUAL, @2); + $$ = add_binary_comparison_expr_merge(ctx, $1, $3, HLSL_OP2_EQUAL, @2); } | equality_expr OP_NE relational_expr { - $$ = add_binary_arithmetic_expr_merge(ctx, $1, $3, HLSL_OP2_NEQUAL, @2); + $$ = add_binary_comparison_expr_merge(ctx, $1, $3, HLSL_OP2_NEQUAL, @2); } bitand_expr: -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> --- I would have composed the helpers the other way around i.e. with the more specific helpers calling the generic one with some hardcoded parameters, but this certainly also works. WRT the patch series, if the patches don't get merged right away I suggest you resend the first 5 or so, incorporating Zeb's and my sign-offs if there are no significant changes.
participants (5)
-
Giovanni Mascellani -
Henri Verbeet -
Matteo Bruni -
Matteo Bruni -
Zebediah Figura (she/her)