[PATCH vkd3d v3 01/11] vkd3d-shader/hlsl: Do not generate infinities in SM1.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_constant_ops.c | 9 ++++++++- tests/arithmetic-float.shader_test | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c index 06b957ba..a1551ba1 100644 --- a/libs/vkd3d-shader/hlsl_constant_ops.c +++ b/libs/vkd3d-shader/hlsl_constant_ops.c @@ -18,6 +18,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#include <math.h> + #include "hlsl.h" static bool fold_cast(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, struct hlsl_ir_constant *src) @@ -270,12 +272,17 @@ static bool fold_div(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, { case HLSL_TYPE_FLOAT: case HLSL_TYPE_HALF: - if (src2->value[k].f == 0) + if (ctx->profile->major_version >= 4 && src2->value[k].f == 0) { hlsl_warning(ctx, &dst->node.loc, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO, "Floating point division by zero"); } dst->value[k].f = src1->value[k].f / src2->value[k].f; + if (ctx->profile->major_version < 4 && isinf(dst->value[k].f)) + { + hlsl_error(ctx, &dst->node.loc, VKD3D_SHADER_ERROR_HLSL_DIVISION_BY_ZERO, + "Infinities are not allowed by the shader model."); + } break; case HLSL_TYPE_DOUBLE: diff --git a/tests/arithmetic-float.shader_test b/tests/arithmetic-float.shader_test index f99b9728..6824c3f1 100644 --- a/tests/arithmetic-float.shader_test +++ b/tests/arithmetic-float.shader_test @@ -25,6 +25,7 @@ todo draw quad probe all rgba (5.0, 5.0, -5.0, 3.0) [require] +% Infinities are not allowed in SM1 shader model >= 4.0 [pixel shader] -- 2.35.2
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_constant_ops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c index a1551ba1..2e16577f 100644 --- a/libs/vkd3d-shader/hlsl_constant_ops.c +++ b/libs/vkd3d-shader/hlsl_constant_ops.c @@ -275,7 +275,7 @@ static bool fold_div(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, if (ctx->profile->major_version >= 4 && src2->value[k].f == 0) { hlsl_warning(ctx, &dst->node.loc, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO, - "Floating point division by zero"); + "Floating point division by zero."); } dst->value[k].f = src1->value[k].f / src2->value[k].f; if (ctx->profile->major_version < 4 && isinf(dst->value[k].f)) @@ -289,7 +289,7 @@ static bool fold_div(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, if (src2->value[k].d == 0) { hlsl_warning(ctx, &dst->node.loc, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO, - "Floating point division by zero"); + "Floating point division by zero."); } dst->value[k].d = src1->value[k].d / src2->value[k].d; break; -- 2.35.2
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> April 22, 2022 6:26 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_constant_ops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c index a1551ba1..2e16577f 100644 --- a/libs/vkd3d-shader/hlsl_constant_ops.c +++ b/libs/vkd3d-shader/hlsl_constant_ops.c @@ -275,7 +275,7 @@ static bool fold_div(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, if (ctx->profile->major_version >= 4 && src2->value[k].f == 0) { hlsl_warning(ctx, &dst->node.loc, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO, - "Floating point division by zero"); + "Floating point division by zero."); } dst->value[k].f = src1->value[k].f / src2->value[k].f; if (ctx->profile->major_version < 4 && isinf(dst->value[k].f)) @@ -289,7 +289,7 @@ static bool fold_div(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, if (src2->value[k].d == 0) { hlsl_warning(ctx, &dst->node.loc, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO, - "Floating point division by zero"); + "Floating point division by zero."); } dst->value[k].d = src1->value[k].d / src2->value[k].d; break; -- 2.35.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- tests/arithmetic-int.shader_test | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/arithmetic-int.shader_test b/tests/arithmetic-int.shader_test index c2eee2ba..14bb6a63 100644 --- a/tests/arithmetic-int.shader_test +++ b/tests/arithmetic-int.shader_test @@ -24,6 +24,19 @@ float4 main() : SV_TARGET draw quad probe all rgba (5.0, 5.0, -5.0, 3.0) +[pixel shader] +float4 main() : SV_TARGET +{ + int x = -2147483648; + int y = -1; + + return x / y; +} + +[test] +draw quad +probe all rgba (-2147483648.0, -2147483648.0, -2147483648.0, -2147483648.0) + [pixel shader fail] float4 main() : SV_TARGET { -- 2.35.2
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> However, April 22, 2022 6:25 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- tests/arithmetic-int.shader_test | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/tests/arithmetic-int.shader_test b/tests/arithmetic-int.shader_test index c2eee2ba..14bb6a63 100644 --- a/tests/arithmetic-int.shader_test +++ b/tests/arithmetic-int.shader_test @@ -24,6 +24,19 @@ float4 main() : SV_TARGET draw quad probe all rgba (5.0, 5.0, -5.0, 3.0)
+[pixel shader] +float4 main() : SV_TARGET +{ + int x = -2147483648; + int y = -1; + + return x / y; +} + +[test] +draw quad +probe all rgba (-2147483648.0, -2147483648.0, -2147483648.0, -2147483648.0) + [pixel shader fail] float4 main() : SV_TARGET { -- 2.35.2
Using ps_3_0, the native (10.0.10240.16384) gives me --- ps_3_0 def c0, 2.14748365e+009, 0, 0, 0 mov oC0, c0.x --- while native (9.29.952.3111) doesn't even parse -2147483648 correctly. I suggest adding --- [require] shader model >= 4.0 --- Best regards, Francisco.
On 4/22/22 05:24, Giovanni Mascellani wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- tests/arithmetic-int.shader_test | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/tests/arithmetic-int.shader_test b/tests/arithmetic-int.shader_test index c2eee2ba..14bb6a63 100644 --- a/tests/arithmetic-int.shader_test +++ b/tests/arithmetic-int.shader_test @@ -24,6 +24,19 @@ float4 main() : SV_TARGET draw quad probe all rgba (5.0, 5.0, -5.0, 3.0)
+[pixel shader] +float4 main() : SV_TARGET +{ + int x = -2147483648; + int y = -1; + + return x / y; +} + +[test] +draw quad +probe all rgba (-2147483648.0, -2147483648.0, -2147483648.0, -2147483648.0) + [pixel shader fail] float4 main() : SV_TARGET {
This fails for me with the d3d9 backend, returning positive 2**32 instead of negative.
Hi, Il 22/04/22 21:09, Zebediah Figura ha scritto:
On 4/22/22 05:24, Giovanni Mascellani wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- tests/arithmetic-int.shader_test | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/tests/arithmetic-int.shader_test b/tests/arithmetic-int.shader_test index c2eee2ba..14bb6a63 100644 --- a/tests/arithmetic-int.shader_test +++ b/tests/arithmetic-int.shader_test @@ -24,6 +24,19 @@ float4 main() : SV_TARGET draw quad probe all rgba (5.0, 5.0, -5.0, 3.0) +[pixel shader] +float4 main() : SV_TARGET +{ + int x = -2147483648; + int y = -1; + + return x / y; +} + +[test] +draw quad +probe all rgba (-2147483648.0, -2147483648.0, -2147483648.0, -2147483648.0) + [pixel shader fail] float4 main() : SV_TARGET {
This fails for me with the d3d9 backend, returning positive 2**32 instead of negative.
Ouch, yes, you're right. SM1 treats ints as floats, so it gets confused when ints that cannot be exactly represented as floats are used, and whenever modular arithmetic is assumed, like in this case. This makes me notice that the problems are more than that; for example, this shader returns 2000002: --- float4 main() : SV_TARGET { int x = 2000001999; int y = 1000; return x / y; } --- but this one returns 2000001: --- float4 main() : SV_TARGET { return 2000001999 / 1000; } --- Notice that 2000001 and 2000002 are different both as ints and as floats, and 2000001999 is approximated to 2000002048 when converted to a float. So it seems that the native compiler does two constant folding passes: one before copy propagation (in which ints are still treated as ints) and one after (and after int operations have been somehow converted to float). This also works with INT_MIN / -1, which becomes negative if you write it as a simple expression (without storing the operands in variables x and y). This looks pretty braindead and I am not sure we want to replicate it, until we find something that really depends on it. And I hope we don't. In practice, for the moment I would suggest to only do constant folding in SM4, and enable it for SM1 once int operations are rewritten correctly. Giovanni.
On 4/23/22 07:25, Giovanni Mascellani wrote:
Hi,
Il 22/04/22 21:09, Zebediah Figura ha scritto:
On 4/22/22 05:24, Giovanni Mascellani wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- tests/arithmetic-int.shader_test | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/tests/arithmetic-int.shader_test b/tests/arithmetic-int.shader_test index c2eee2ba..14bb6a63 100644 --- a/tests/arithmetic-int.shader_test +++ b/tests/arithmetic-int.shader_test @@ -24,6 +24,19 @@ float4 main() : SV_TARGET draw quad probe all rgba (5.0, 5.0, -5.0, 3.0) +[pixel shader] +float4 main() : SV_TARGET +{ + int x = -2147483648; + int y = -1; + + return x / y; +} + +[test] +draw quad +probe all rgba (-2147483648.0, -2147483648.0, -2147483648.0, -2147483648.0) + [pixel shader fail] float4 main() : SV_TARGET {
This fails for me with the d3d9 backend, returning positive 2**32 instead of negative.
Ouch, yes, you're right. SM1 treats ints as floats, so it gets confused when ints that cannot be exactly represented as floats are used, and whenever modular arithmetic is assumed, like in this case.
This makes me notice that the problems are more than that; for example, this shader returns 2000002: --- float4 main() : SV_TARGET { int x = 2000001999; int y = 1000;
return x / y; } --- but this one returns 2000001: --- float4 main() : SV_TARGET { return 2000001999 / 1000; } ---
Notice that 2000001 and 2000002 are different both as ints and as floats, and 2000001999 is approximated to 2000002048 when converted to a float.
So it seems that the native compiler does two constant folding passes: one before copy propagation (in which ints are still treated as ints) and one after (and after int operations have been somehow converted to float). This also works with INT_MIN / -1, which becomes negative if you write it as a simple expression (without storing the operands in variables x and y).
This looks pretty braindead and I am not sure we want to replicate it, until we find something that really depends on it. And I hope we don't.
I doubt we will need to replicate it, no. I don't think it's particularly important to get INT_MIN / -1 right for SM1, either; I just want all the tests to pass.
In practice, for the moment I would suggest to only do constant folding in SM4, and enable it for SM1 once int operations are rewritten correctly.
I don't think that's feasible, we need constant folding for many purposes. Probably better just to drop this test.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 291f8392..ec0e23bb 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -588,12 +588,12 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls struct hlsl_type *data_type; struct hlsl_ir_constant *c; struct hlsl_ir_node *mul; + unsigned int stride; if (expr_type->type == HLSL_CLASS_ARRAY) { data_type = expr_type->e.array.type; - if (!(c = hlsl_new_uint_constant(ctx, hlsl_type_get_array_element_reg_size(data_type), &loc))) - return false; + stride = hlsl_type_get_array_element_reg_size(data_type); } else if (expr_type->type == HLSL_CLASS_MATRIX) { @@ -604,8 +604,7 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls else if (expr_type->type == HLSL_CLASS_VECTOR) { data_type = hlsl_get_scalar_type(ctx, expr_type->base_type); - if (!(c = hlsl_new_uint_constant(ctx, 1, &loc))) - return false; + stride = 1; } else { @@ -616,6 +615,8 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls return false; } + if (!(c = hlsl_new_uint_constant(ctx, stride, &loc))) + return false; list_add_tail(instrs, &c->node.entry); if (!(mul = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, index, &c->node))) return false; -- 2.35.2
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> April 22, 2022 6:25 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 291f8392..ec0e23bb 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -588,12 +588,12 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls struct hlsl_type *data_type; struct hlsl_ir_constant *c; struct hlsl_ir_node *mul; + unsigned int stride;
if (expr_type->type == HLSL_CLASS_ARRAY) { data_type = expr_type->e.array.type; - if (!(c = hlsl_new_uint_constant(ctx, hlsl_type_get_array_element_reg_size(data_type), &loc))) - return false; + stride = hlsl_type_get_array_element_reg_size(data_type); } else if (expr_type->type == HLSL_CLASS_MATRIX) { @@ -604,8 +604,7 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls else if (expr_type->type == HLSL_CLASS_VECTOR) { data_type = hlsl_get_scalar_type(ctx, expr_type->base_type); - if (!(c = hlsl_new_uint_constant(ctx, 1, &loc))) - return false; + stride = 1; } else { @@ -616,6 +615,8 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls return false; }
+ if (!(c = hlsl_new_uint_constant(ctx, stride, &loc))) + return false; list_add_tail(instrs, &c->node.entry); if (!(mul = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, index, &c->node))) return false; -- 2.35.2
On 4/22/22 05:25, Giovanni Mascellani wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 291f8392..ec0e23bb 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -588,12 +588,12 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls struct hlsl_type *data_type; struct hlsl_ir_constant *c; struct hlsl_ir_node *mul; + unsigned int stride;
if (expr_type->type == HLSL_CLASS_ARRAY) { data_type = expr_type->e.array.type; - if (!(c = hlsl_new_uint_constant(ctx, hlsl_type_get_array_element_reg_size(data_type), &loc))) - return false; + stride = hlsl_type_get_array_element_reg_size(data_type); } else if (expr_type->type == HLSL_CLASS_MATRIX) { @@ -604,8 +604,7 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls else if (expr_type->type == HLSL_CLASS_VECTOR) { data_type = hlsl_get_scalar_type(ctx, expr_type->base_type); - if (!(c = hlsl_new_uint_constant(ctx, 1, &loc))) - return false; + stride = 1; } else { @@ -616,6 +615,8 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls return false; }
+ if (!(c = hlsl_new_uint_constant(ctx, stride, &loc))) + return false; list_add_tail(instrs, &c->node.entry); if (!(mul = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, index, &c->node))) return false;
Why not move the MUL into the HLSL_CLASS_ARRAY path instead?
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 73e3b73f..f22841fb 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -567,8 +567,10 @@ static bool fold_redundant_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *inst return false; } -/* Helper for split_array_copies() and split_struct_copies(). Inserts new - * instructions right before "store". */ +/* Copy an element of a complex variable. Helper for + * split_array_copies(), split_struct_copies() and + * split_matrix_copies(). Inserts new instructions right before + * "store". */ static bool split_copy(struct hlsl_ctx *ctx, struct hlsl_ir_store *store, const struct hlsl_ir_load *load, const unsigned int offset, struct hlsl_type *type) { -- 2.35.2
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> Btw, I realized the split copies passes do something very similar to the initialize_var_components() function in the "Support all complex initializers." patch. The reuse of code may be worth exploring. April 22, 2022 6:25 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 73e3b73f..f22841fb 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -567,8 +567,10 @@ static bool fold_redundant_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *inst return false; }
-/* Helper for split_array_copies() and split_struct_copies(). Inserts new - * instructions right before "store". */ +/* Copy an element of a complex variable. Helper for + * split_array_copies(), split_struct_copies() and + * split_matrix_copies(). Inserts new instructions right before + * "store". */ static bool split_copy(struct hlsl_ctx *ctx, struct hlsl_ir_store *store, const struct hlsl_ir_load *load, const unsigned int offset, struct hlsl_type *type) { -- 2.35.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- tests/hlsl-matrix-indexing.shader_test | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/hlsl-matrix-indexing.shader_test b/tests/hlsl-matrix-indexing.shader_test index e2103fe2..8057179e 100644 --- a/tests/hlsl-matrix-indexing.shader_test +++ b/tests/hlsl-matrix-indexing.shader_test @@ -47,18 +47,16 @@ todo draw quad probe all rgba (1.0, 5.0, 7.0, 12.0) [pixel shader] -uniform float4x4 m; +uniform float3x2 m; float4 main() : SV_TARGET { - float4x4 m2 = m; - return float4(m2[0][0], m2[1][0], m2[1][2], m2[2][3]); + float3x2 m2 = m; + return float4(m2[0][0], m2[2][0], m2[1][1], m2[2][1]); } [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 float4 9.0 10.0 11.0 12.0 -uniform 12 float4 13.0 14.0 15.0 16.0 +uniform 0 float4 1.0 2.0 3.0 0.0 +uniform 4 float4 5.0 6.0 7.0 0.0 todo draw quad -probe all rgba (1.0, 2.0, 10.0, 15.0) +probe all rgba (1.0, 3.0, 6.0, 7.0) -- 2.35.2
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> April 22, 2022 6:25 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- tests/hlsl-matrix-indexing.shader_test | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/tests/hlsl-matrix-indexing.shader_test b/tests/hlsl-matrix-indexing.shader_test index e2103fe2..8057179e 100644 --- a/tests/hlsl-matrix-indexing.shader_test +++ b/tests/hlsl-matrix-indexing.shader_test @@ -47,18 +47,16 @@ todo draw quad probe all rgba (1.0, 5.0, 7.0, 12.0)
[pixel shader] -uniform float4x4 m; +uniform float3x2 m;
float4 main() : SV_TARGET { - float4x4 m2 = m; - return float4(m2[0][0], m2[1][0], m2[1][2], m2[2][3]); + float3x2 m2 = m; + return float4(m2[0][0], m2[2][0], m2[1][1], m2[2][1]); }
[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 float4 9.0 10.0 11.0 12.0 -uniform 12 float4 13.0 14.0 15.0 16.0 +uniform 0 float4 1.0 2.0 3.0 0.0 +uniform 4 float4 5.0 6.0 7.0 0.0 todo draw quad -probe all rgba (1.0, 2.0, 10.0, 15.0) +probe all rgba (1.0, 3.0, 6.0, 7.0) -- 2.35.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index f22841fb..f31bfe9f 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -676,7 +676,7 @@ static bool split_struct_copies(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr static unsigned int minor_size(const struct hlsl_type *type) { - if (type->type == HLSL_CLASS_VECTOR || type->modifiers & HLSL_MODIFIER_ROW_MAJOR) + if (type->modifiers & HLSL_MODIFIER_ROW_MAJOR) return type->dimx; else return type->dimy; @@ -684,7 +684,7 @@ static unsigned int minor_size(const struct hlsl_type *type) static unsigned int major_size(const struct hlsl_type *type) { - if (type->type == HLSL_CLASS_VECTOR || type->modifiers & HLSL_MODIFIER_ROW_MAJOR) + if (type->modifiers & HLSL_MODIFIER_ROW_MAJOR) return type->dimy; else return type->dimx; -- 2.35.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- v3: * New --- Makefile.am | 1 + tests/hlsl-initializer-matrix.shader_test | 72 +++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 tests/hlsl-initializer-matrix.shader_test diff --git a/Makefile.am b/Makefile.am index e760a66b..ae53e767 100644 --- a/Makefile.am +++ b/Makefile.am @@ -79,6 +79,7 @@ vkd3d_shader_tests = \ tests/hlsl-initializer-local-array.shader_test \ tests/hlsl-initializer-nested.shader_test \ tests/hlsl-initializer-numeric.shader_test \ + tests/hlsl-initializer-matrix.shader_test \ tests/hlsl-initializer-static-array.shader_test \ tests/hlsl-initializer-struct.shader_test \ tests/hlsl-intrinsic-override.shader_test \ diff --git a/tests/hlsl-initializer-matrix.shader_test b/tests/hlsl-initializer-matrix.shader_test new file mode 100644 index 00000000..c4fafea7 --- /dev/null +++ b/tests/hlsl-initializer-matrix.shader_test @@ -0,0 +1,72 @@ +[pixel shader] +float4 main() : SV_TARGET +{ + float4x3 mat = {11, 12, 13, 21, 22, 23, 31, 32, 33, 41, 42, 43}; + return float4(mat[1], 0); +} + +[test] +todo draw quad +probe all rgba (21, 22, 23, 0) + + +[pixel shader] +float4 main() : SV_TARGET +{ + row_major float4x3 mat = {11, 12, 13, 21, 22, 23, 31, 32, 33, 41, 42, 43}; + return float4(mat[1], 0); +} + +[test] +todo draw quad +probe all rgba (21, 22, 23, 0) + + +[pixel shader] +float4 main() : SV_TARGET +{ + float3x4 mat = {11, 12, 13, 14, 21, 22, 23, 24, 31, 32, 33, 34}; + return float4(mat[1]); +} + +[test] +todo draw quad +probe all rgba (21, 22, 23, 24) + + +[pixel shader] +float4 main() : SV_TARGET +{ + row_major float3x2 mat = {11, 12, 21, 22, 31, 32}; + return float4(mat[1], mat[2]); +} + +[test] +todo draw quad +probe all rgba (21, 22, 31, 32) + + +[pixel shader] +float4 main() : SV_TARGET +{ + row_major float3x2 mat; + mat = float3x2(11, 12, 21, 22, 31, 32); + return float4(mat[1], mat[2]); +} + +[test] +todo draw quad +probe all rgba (21, 22, 31, 32) + + +[pixel shader] +float4 main() : SV_TARGET +{ + float4 x = float4(12, 21, 22, 31); + float3x2 mat = float3x2(11, x, 32); + return float4(mat[1], mat[2]); +} + +[test] +todo draw quad +probe all rgba (21, 22, 31, 32) -- 2.35.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
From: Francisco Casas <fcasas(a)codeweavers.com> --- v3: * Patch reworked from scratch by Francisco * Some tests are still failing because they depend on matrix copying --- libs/vkd3d-shader/hlsl.c | 83 ++++++++++++ libs/vkd3d-shader/hlsl.h | 2 + libs/vkd3d-shader/hlsl.y | 124 +++++------------- libs/vkd3d-shader/hlsl_codegen.c | 2 +- tests/hlsl-initializer-flatten.shader_test | 10 +- ...-initializer-invalid-arg-count.shader_test | 4 +- .../hlsl-initializer-local-array.shader_test | 4 +- tests/hlsl-initializer-nested.shader_test | 4 +- .../hlsl-initializer-static-array.shader_test | 4 +- tests/hlsl-initializer-struct.shader_test | 6 +- 10 files changed, 137 insertions(+), 106 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index eabe189f..66fdd45b 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -232,6 +232,89 @@ static struct hlsl_type *hlsl_new_type(struct hlsl_ctx *ctx, const char *name, e return type; } +/* Returns the register offset of a given component within a type, given its index. + * *comp_type, will be set to the type of the component, unless it is NULL. */ +unsigned int hlsl_compute_component_reg_offset(struct hlsl_ctx *ctx, struct hlsl_type *type, + unsigned int idx, struct hlsl_type **comp_type, const struct vkd3d_shader_location *loc) +{ + switch (type->type) + { + case HLSL_CLASS_SCALAR: + case HLSL_CLASS_VECTOR: + { + assert(idx < type->dimx * type->dimy); + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type); + return idx; + } + case HLSL_CLASS_MATRIX: + { + unsigned int minor, major, x = idx % type->dimx, y = idx / type->dimx; + + assert(idx < type->dimx * type->dimy); + + if (hlsl_type_is_row_major(type)) + { + minor = x; + major = y; + } + else + { + minor = y; + major = x; + } + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type); + return 4 * major + minor; + } + + case HLSL_CLASS_ARRAY: + { + unsigned int elem_comp_count = hlsl_type_component_count(type->e.array.type); + unsigned int array_idx = idx / elem_comp_count; + unsigned int idx_in_elem = idx - array_idx * elem_comp_count; + + assert(array_idx < type->e.array.elements_count); + + return array_idx * hlsl_type_get_array_element_reg_size(type->e.array.type) + + hlsl_compute_component_reg_offset(ctx, type->e.array.type, idx_in_elem, comp_type, loc); + } + + case HLSL_CLASS_STRUCT: + { + struct hlsl_struct_field *field; + + LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) + { + unsigned int elem_comp_count = hlsl_type_component_count(field->type); + + if (idx < elem_comp_count) + { + return field->reg_offset + + hlsl_compute_component_reg_offset(ctx, field->type, idx, comp_type, loc); + } + idx -= elem_comp_count; + } + + assert(0); + return 0; + } + + case HLSL_CLASS_OBJECT: + { + assert(idx == 0); + if (comp_type) + *comp_type = type; + return 0; + } + } + + assert(0); + return 0; +} + struct hlsl_type *hlsl_new_array_type(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, unsigned int array_size) { struct hlsl_type *type; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 802adf87..b50a7f24 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -786,6 +786,8 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old, unsigned int default_majority, unsigned int modifiers); unsigned int hlsl_type_component_count(struct hlsl_type *type); unsigned int hlsl_type_get_array_element_reg_size(const struct hlsl_type *type); +unsigned int hlsl_compute_component_reg_offset(struct hlsl_ctx *ctx, struct hlsl_type *type, + unsigned int idx, struct hlsl_type **comp_type, const struct vkd3d_shader_location *loc); unsigned int hlsl_type_get_sm4_offset(const struct hlsl_type *type, unsigned int offset); bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ec0e23bb..f072aa15 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1454,77 +1454,57 @@ static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrem return true; } -static void initialize_numeric_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, - struct parse_initializer *initializer, unsigned int reg_offset, struct hlsl_type *type, - unsigned int *initializer_offset) +static void initialize_var_components(struct hlsl_ctx *ctx, struct list *instrs, + struct hlsl_ir_var *dst, unsigned int *store_index, struct hlsl_ir_node *src, + const struct vkd3d_shader_location *loc) { - unsigned int i; - - if (type->type == HLSL_CLASS_MATRIX) - hlsl_fixme(ctx, &var->loc, "Matrix initializer."); + unsigned int src_comp_count = hlsl_type_component_count(src->data_type); + unsigned int k; - for (i = 0; i < type->dimx; i++) + for (k = 0; k < src_comp_count; ++k) { + struct hlsl_type *dst_comp_type, *src_comp_type; + unsigned int dst_reg_offset, src_reg_offset; struct hlsl_ir_store *store; struct hlsl_ir_constant *c; - struct hlsl_ir_node *node; + struct hlsl_ir_load *load; + struct hlsl_ir_node *conv; + + dst_reg_offset = hlsl_compute_component_reg_offset(ctx, dst->data_type, *store_index, &dst_comp_type, loc); + src_reg_offset = hlsl_compute_component_reg_offset(ctx, src->data_type, k, &src_comp_type, loc); + + if (!(c = hlsl_new_uint_constant(ctx, src_reg_offset, loc))) + return; + list_add_tail(instrs, &c->node.entry); - node = initializer->args[*initializer_offset]; - *initializer_offset += 1; + if (!(load = add_load(ctx, instrs, src, &c->node, src_comp_type, *loc))) + return; - if (!(node = add_implicit_conversion(ctx, initializer->instrs, node, - hlsl_get_scalar_type(ctx, type->base_type), &node->loc))) + if (!(conv = add_implicit_conversion(ctx, instrs, &load->node, dst_comp_type, loc))) return; - if (!(c = hlsl_new_uint_constant(ctx, reg_offset + i, &node->loc))) + if (!(c = hlsl_new_uint_constant(ctx, dst_reg_offset, loc))) return; - list_add_tail(initializer->instrs, &c->node.entry); + list_add_tail(instrs, &c->node.entry); - if (!(store = hlsl_new_store(ctx, var, &c->node, node, 0, node->loc))) + if (!(store = hlsl_new_store(ctx, dst, &c->node, conv, 0, *loc))) return; + list_add_tail(instrs, &store->node.entry); - list_add_tail(initializer->instrs, &store->node.entry); + ++*store_index; } } -static void struct_var_initializer(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, +static void initialize_var_from_initializer(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, struct parse_initializer *initializer) { - struct hlsl_type *type = var->data_type; - struct hlsl_struct_field *field; - unsigned int i = 0; + unsigned int store_index = 0; + unsigned int i; - if (initializer_size(initializer) != hlsl_type_component_count(type)) + for (i = 0; i < initializer->args_count; ++i) { - hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, - "Expected %u components in initializer, but got %u.", - hlsl_type_component_count(type), initializer_size(initializer)); - return; - } - - LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) - { - struct hlsl_ir_node *node = initializer->args[i]; - struct hlsl_ir_store *store; - struct hlsl_ir_constant *c; - - if (i++ >= initializer->args_count) - break; - - if (hlsl_type_component_count(field->type) == hlsl_type_component_count(node->data_type)) - { - if (!(c = hlsl_new_uint_constant(ctx, field->reg_offset, &node->loc))) - break; - list_add_tail(initializer->instrs, &c->node.entry); - - if (!(store = hlsl_new_store(ctx, var, &c->node, node, 0, node->loc))) - break; - list_add_tail(initializer->instrs, &store->node.entry); - } - else - { - hlsl_fixme(ctx, &node->loc, "Implicit cast in structure initializer."); - } + initialize_var_components(ctx, initializer->instrs, var, &store_index, initializer->args[i], + &initializer->args[i]->loc); } } @@ -1652,52 +1632,18 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t if (v->initializer.braces) { unsigned int size = initializer_size(&v->initializer); - unsigned int initializer_offset = 0; - - if (type->type <= HLSL_CLASS_LAST_NUMERIC && type->dimx * type->dimy != size) - { - hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, - "Expected %u components in numeric initializer, but got %u.", - type->dimx * type->dimy, v->initializer.args_count); - free_parse_initializer(&v->initializer); - vkd3d_free(v); - continue; - } - if ((type->type == HLSL_CLASS_STRUCT || type->type == HLSL_CLASS_ARRAY) - && hlsl_type_component_count(type) != size) + if (hlsl_type_component_count(type) != size) { hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, - "Expected %u components in initializer, but got %u.", hlsl_type_component_count(type), size); + "Expected %u components in initializer, but got %u.", + hlsl_type_component_count(type), size); free_parse_initializer(&v->initializer); vkd3d_free(v); continue; } - if (type->type > HLSL_CLASS_LAST_NUMERIC && type->type != HLSL_CLASS_STRUCT) - { - FIXME("Initializers for non scalar/struct variables not supported yet.\n"); - free_parse_initializer(&v->initializer); - vkd3d_free(v); - continue; - } - - if (type->type == HLSL_CLASS_STRUCT) - { - struct_var_initializer(ctx, var, &v->initializer); - } - else - { - if (v->initializer.args_count != size) - { - hlsl_fixme(ctx, &v->loc, "Flatten initializer."); - free_parse_initializer(&v->initializer); - vkd3d_free(v); - continue; - } - - initialize_numeric_var(ctx, var, &v->initializer, 0, type, &initializer_offset); - } + initialize_var_from_initializer(ctx, var, &v->initializer); } else { diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index f31bfe9f..9eb2fccb 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -1653,7 +1653,7 @@ bool hlsl_offset_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref if (*offset >= deref->var->data_type->reg_size) { hlsl_error(ctx, &deref->offset.node->loc, VKD3D_SHADER_ERROR_HLSL_OFFSET_OUT_OF_BOUNDS, - "Dereference is out of bounds."); + "Dereference is out of bounds. %u/%u", *offset, deref->var->data_type->reg_size); return false; } diff --git a/tests/hlsl-initializer-flatten.shader_test b/tests/hlsl-initializer-flatten.shader_test index 3a430e0d..6b35c6b7 100644 --- a/tests/hlsl-initializer-flatten.shader_test +++ b/tests/hlsl-initializer-flatten.shader_test @@ -6,7 +6,7 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (1, 2, 3, 4) @@ -24,7 +24,7 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (4, 5, 6, 7) @@ -38,7 +38,7 @@ float4 main() : sv_target [test] draw quad -todo probe all rgba (40, 10, 20, 30) +probe all rgba (40, 10, 20, 30) [pixel shader] @@ -56,7 +56,7 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (1.0, 2.0, 3.0, 4.0) @@ -69,5 +69,5 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (1.0, 2.0, 3.0, 4.0) diff --git a/tests/hlsl-initializer-invalid-arg-count.shader_test b/tests/hlsl-initializer-invalid-arg-count.shader_test index acd449af..4332fbe8 100644 --- a/tests/hlsl-initializer-invalid-arg-count.shader_test +++ b/tests/hlsl-initializer-invalid-arg-count.shader_test @@ -10,7 +10,7 @@ float4 main() : sv_target [test] draw quad -todo probe all rgba (17, 18, 19, 20) +probe all rgba (17, 18, 19, 20) [pixel shader fail] @@ -57,7 +57,7 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (22, 23, 24, 25) diff --git a/tests/hlsl-initializer-local-array.shader_test b/tests/hlsl-initializer-local-array.shader_test index 13670dc6..0862d4c9 100644 --- a/tests/hlsl-initializer-local-array.shader_test +++ b/tests/hlsl-initializer-local-array.shader_test @@ -11,7 +11,7 @@ float4 main() : SV_TARGET [test] draw quad -todo probe all rgba (21, 22, 23, 24) +probe all rgba (21, 22, 23, 24) [pixel shader] @@ -32,4 +32,4 @@ float4 main() : SV_TARGET [test] draw quad -todo probe all rgba (71, 72, 73, 74) +probe all rgba (71, 72, 73, 74) diff --git a/tests/hlsl-initializer-nested.shader_test b/tests/hlsl-initializer-nested.shader_test index bcb37cf4..b00259c9 100644 --- a/tests/hlsl-initializer-nested.shader_test +++ b/tests/hlsl-initializer-nested.shader_test @@ -24,7 +24,7 @@ float4 main() : sv_target [test] draw quad -todo probe all rgba (21, 22, 23, 24) +probe all rgba (21, 22, 23, 24) [pixel shader] @@ -52,5 +52,5 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (21, 22, 23, 24) diff --git a/tests/hlsl-initializer-static-array.shader_test b/tests/hlsl-initializer-static-array.shader_test index f276c629..57733502 100644 --- a/tests/hlsl-initializer-static-array.shader_test +++ b/tests/hlsl-initializer-static-array.shader_test @@ -12,7 +12,7 @@ float4 main() : SV_TARGET [test] draw quad -todo probe all rgba (21, 22, 23, 24) +probe all rgba (21, 22, 23, 24) [pixel shader] @@ -34,4 +34,4 @@ float4 main() : SV_TARGET [test] draw quad -todo probe all rgba (61, 62, 63, 64) +probe all rgba (61, 62, 63, 64) diff --git a/tests/hlsl-initializer-struct.shader_test b/tests/hlsl-initializer-struct.shader_test index 2a824e23..f4028b5b 100644 --- a/tests/hlsl-initializer-struct.shader_test +++ b/tests/hlsl-initializer-struct.shader_test @@ -22,7 +22,7 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (41, 42, 43, 44) @@ -52,7 +52,7 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (4311, 4312, 4313, 4314) @@ -80,5 +80,5 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (21, 22, 23, 24) -- 2.35.2
Hi, I am not completely in love with the additional runtime complexity of this solution, but it's true that it's quite clean. Il 22/04/22 12:25, Giovanni Mascellani ha scritto:
+/* Returns the register offset of a given component within a type, given its index. + * *comp_type, will be set to the type of the component, unless it is NULL. */ +unsigned int hlsl_compute_component_reg_offset(struct hlsl_ctx *ctx, struct hlsl_type *type, + unsigned int idx, struct hlsl_type **comp_type, const struct vkd3d_shader_location *loc)
Is the "loc" parameter actually used for something, except being passed around? Also, personally I'd just write "offset" rather than "reg_offset". It seems to be clear enough. This applies also to the variables below.
+{ + switch (type->type) + { + case HLSL_CLASS_SCALAR: + case HLSL_CLASS_VECTOR: + { + assert(idx < type->dimx * type->dimy); + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type);
I think you can assume that comp_type will always be valid and skip this check.
+ return idx; + } + case HLSL_CLASS_MATRIX: + { + unsigned int minor, major, x = idx % type->dimx, y = idx / type->dimx; + + assert(idx < type->dimx * type->dimy); + + if (hlsl_type_is_row_major(type)) + { + minor = x; + major = y; + } + else + { + minor = y; + major = x; + } + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type); + return 4 * major + minor; + } + + case HLSL_CLASS_ARRAY: + { + unsigned int elem_comp_count = hlsl_type_component_count(type->e.array.type); + unsigned int array_idx = idx / elem_comp_count; + unsigned int idx_in_elem = idx - array_idx * elem_comp_count;
BTW, here you are just computing a modulus, you could directlyuse "%". Addressing arrays is not fundamentally different than addressing matrices, except you don't have to decide which one is major and which one is minor.
+ assert(array_idx < type->e.array.elements_count); + + return array_idx * hlsl_type_get_array_element_reg_size(type->e.array.type) + + hlsl_compute_component_reg_offset(ctx, type->e.array.type, idx_in_elem, comp_type, loc); + } + + case HLSL_CLASS_STRUCT: + { + struct hlsl_struct_field *field; + + LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) + { + unsigned int elem_comp_count = hlsl_type_component_count(field->type); + + if (idx < elem_comp_count) + { + return field->reg_offset + + hlsl_compute_component_reg_offset(ctx, field->type, idx, comp_type, loc); + } + idx -= elem_comp_count; + } + + assert(0); + return 0; + } + + case HLSL_CLASS_OBJECT: + { + assert(idx == 0); + if (comp_type) + *comp_type = type; + return 0; + } + } + + assert(0); + return 0; +} +
Thanks, Giovanni.
Hello, April 22, 2022 6:51 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Hi,
I am not completely in love with the additional runtime complexity of this solution, but it's true that it's quite clean.
I have a couple of ideas for lowering the complexity: a) Add a "component_count" field to hlsl_type in order to store/cache the result of hlsl_type_component_count(). Which would also speed up other parts of the compiler. b) Add a "comp_offsets" array to "hlsl_type.e" so that the right field that has a given component can be selected in hlsl_compute_component_reg_offset() using a binary search. I can make new patches for these.
Il 22/04/22 12:25, Giovanni Mascellani ha scritto:
+/* Returns the register offset of a given component within a type, given its index. + * *comp_type, will be set to the type of the component, unless it is NULL. */ +unsigned int hlsl_compute_component_reg_offset(struct hlsl_ctx *ctx, struct hlsl_type *type, + unsigned int idx, struct hlsl_type **comp_type, const struct vkd3d_shader_location *loc)
Is the "loc" parameter actually used for something, except being passed around?
I see, it is just being passed around here. I will remove it.
Also, personally I'd just write "offset" rather than "reg_offset". It seems to be clear enough. This applies also to the variables below.
+{ + switch (type->type) + { + case HLSL_CLASS_SCALAR: + case HLSL_CLASS_VECTOR: + { + assert(idx < type->dimx * type->dimy); + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type);
I think you can assume that comp_type will always be valid and skip this check.
I think hlsl_compute_component_reg_offset() can be used to simplify other functions, like compute_offset_from_index(), and in those cases it could be much cleaner to call it passing a NULL comp_type.
+ return idx; + } + case HLSL_CLASS_MATRIX: + { + unsigned int minor, major, x = idx % type->dimx, y = idx / type->dimx; + + assert(idx < type->dimx * type->dimy); + + if (hlsl_type_is_row_major(type)) + { + minor = x; + major = y; + } + else + { + minor = y; + major = x; + } + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type); + return 4 * major + minor; + } + + case HLSL_CLASS_ARRAY: + { + unsigned int elem_comp_count = hlsl_type_component_count(type->e.array.type); + unsigned int array_idx = idx / elem_comp_count; + unsigned int idx_in_elem = idx - array_idx * elem_comp_count;
BTW, here you are just computing a modulus, you could directlyuse "%". Addressing arrays is not fundamentally different than addressing matrices, except you don't have to decide which one is major and which one is minor.
Okay, I will write it as unsigned int idx_in_elem = idx % elem_comp_count;
+ assert(array_idx < type->e.array.elements_count); + + return array_idx * hlsl_type_get_array_element_reg_size(type->e.array.type) + + hlsl_compute_component_reg_offset(ctx, type->e.array.type, idx_in_elem, comp_type, loc); + } + + case HLSL_CLASS_STRUCT: + { + struct hlsl_struct_field *field; + + LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) + { + unsigned int elem_comp_count = hlsl_type_component_count(field->type); + + if (idx < elem_comp_count) + { + return field->reg_offset + + hlsl_compute_component_reg_offset(ctx, field->type, idx, comp_type, loc); + } + idx -= elem_comp_count; + } + + assert(0); + return 0; + } + + case HLSL_CLASS_OBJECT: + { + assert(idx == 0); + if (comp_type) + *comp_type = type; + return 0; + } + } + + assert(0); + return 0; +} +
Thanks, Giovanni.
On 4/22/22 09:11, Francisco Casas wrote:
Hello,
April 22, 2022 6:51 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Hi,
I am not completely in love with the additional runtime complexity of this solution, but it's true that it's quite clean.
I have a couple of ideas for lowering the complexity:
a) Add a "component_count" field to hlsl_type in order to store/cache the result of hlsl_type_component_count(). Which would also speed up other parts of the compiler.
b) Add a "comp_offsets" array to "hlsl_type.e" so that the right field that has a given component can be selected in hlsl_compute_component_reg_offset() using a binary search.
I can make new patches for these.
I guess you mean time complexity? I don't think there's any reason to worry about that, especially not with initializers.
+{ + switch (type->type) + { + case HLSL_CLASS_SCALAR: + case HLSL_CLASS_VECTOR: + { + assert(idx < type->dimx * type->dimy); + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type);
I think you can assume that comp_type will always be valid and skip this check.
I think hlsl_compute_component_reg_offset() can be used to simplify other functions, like compute_offset_from_index(), and in those cases it could be much cleaner to call it passing a NULL comp_type.
I'm not sure we want to use hlsl_compute_component_reg_offset() in compute_offset_from_index(). In general I'd remove the NULL handling until a patch that actually needs it.
Hello, April 22, 2022 3:45 PM, "Zebediah Figura" <zfigura(a)codeweavers.com> wrote:
On 4/22/22 09:11, Francisco Casas wrote:
Hello, April 22, 2022 6:51 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote: Hi,
I am not completely in love with the additional runtime complexity of this solution, but it's true that it's quite clean.
I have a couple of ideas for lowering the complexity: a) Add a "component_count" field to hlsl_type in order to store/cache the result of hlsl_type_component_count(). Which would also speed up other parts of the compiler. b) Add a "comp_offsets" array to "hlsl_type.e" so that the right field that has a given component can be selected in hlsl_compute_component_reg_offset() using a binary search. I can make new patches for these.
I guess you mean time complexity? I don't think there's any reason to worry about that, especially not with initializers.
Yes, I meant time complexity, sorry. I only see an O(n^2) worst-case scenario, for the initialization of a struct n components. So yes, it is probably not worth worrying about it.
+{ + switch (type->type) + { + case HLSL_CLASS_SCALAR: + case HLSL_CLASS_VECTOR: + { + assert(idx < type->dimx * type->dimy); + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type);
I think you can assume that comp_type will always be valid and skip this check.
I think hlsl_compute_component_reg_offset() can be used to simplify other functions, like compute_offset_from_index(), and in those cases it could be much cleaner to call it passing a NULL comp_type.
I'm not sure we want to use hlsl_compute_component_reg_offset() in compute_offset_from_index(). In general I'd remove the NULL handling until a patch that actually needs it.
Okay.
On 4/22/22 16:51, Francisco Casas wrote:
Hello,
April 22, 2022 3:45 PM, "Zebediah Figura" <zfigura(a)codeweavers.com> wrote:
On 4/22/22 09:11, Francisco Casas wrote:
Hello, April 22, 2022 6:51 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote: Hi,
I am not completely in love with the additional runtime complexity of this solution, but it's true that it's quite clean.
I have a couple of ideas for lowering the complexity: a) Add a "component_count" field to hlsl_type in order to store/cache the result of hlsl_type_component_count(). Which would also speed up other parts of the compiler. b) Add a "comp_offsets" array to "hlsl_type.e" so that the right field that has a given component can be selected in hlsl_compute_component_reg_offset() using a binary search. I can make new patches for these.
I guess you mean time complexity? I don't think there's any reason to worry about that, especially not with initializers.
Yes, I meant time complexity, sorry.
I only see an O(n^2) worst-case scenario, for the initialization of a struct n components. So yes, it is probably not worth worrying about it.
Well, that, but more importantly, n is always going to be small.
Hi, Il 22/04/22 16:11, Francisco Casas ha scritto:
I am not completely in love with the additional runtime complexity of this solution, but it's true that it's quite clean.
I have a couple of ideas for lowering the complexity:
a) Add a "component_count" field to hlsl_type in order to store/cache the result of hlsl_type_component_count(). Which would also speed up other parts of the compiler.
b) Add a "comp_offsets" array to "hlsl_type.e" so that the right field that has a given component can be selected in hlsl_compute_component_reg_offset() using a binary search.
I can make new patches for these.
Actually, that was just a random remark, I don't think it is really worth the additional complexity to do the changes you mention. Giovanni.
On 4/22/22 05:25, Giovanni Mascellani wrote:
+static void initialize_var_from_initializer(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, struct parse_initializer *initializer) {
As long as you're resending anyway, I'd advocate for just inlining this function into its caller.
April 22, 2022 3:57 PM, "Zebediah Figura" <zfigura(a)codeweavers.com> wrote:
On 4/22/22 05:25, Giovanni Mascellani wrote:
+static void initialize_var_from_initializer(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, struct parse_initializer *initializer) {
As long as you're resending anyway, I'd advocate for just inlining this function into its caller.
Okay, the function is used in latter patches but I agree it is short enough to be inlined when needed.
On 4/22/22 16:51, Francisco Casas wrote:
April 22, 2022 3:57 PM, "Zebediah Figura" <zfigura(a)codeweavers.com> wrote:
On 4/22/22 05:25, Giovanni Mascellani wrote:
+static void initialize_var_from_initializer(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, struct parse_initializer *initializer) {
As long as you're resending anyway, I'd advocate for just inlining this function into its caller.
Okay, the function is used in latter patches but I agree it is short enough to be inlined when needed.
I guess if it's used later, it's fine keeping it separate.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- v2: * Use "scalar_type" instead of "scal_type" * Use a vkd3d_string_buffer to format the variable's name * Format the variable name with %u instead of %x * Use hlsl_new_var_load() instead of hlsl_new_load() * Rename compute_matrix_offset() to add_compute_matrix_offset() v3: * Use the string buffer cache. * Rename add_matrix_load() to add_matrix_index() * Rename compute_matrix_offset() to add_matrix_scalar_load() and fold the add_load() call into it --- libs/vkd3d-shader/hlsl.y | 91 ++++++++++++++++++++++- tests/hlsl-initializer-matrix.shader_test | 8 +- tests/hlsl-matrix-indexing.shader_test | 8 +- 3 files changed, 96 insertions(+), 11 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index f072aa15..887bcdc8 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -581,6 +581,93 @@ static bool add_record_load(struct hlsl_ctx *ctx, struct list *instrs, struct hl return !!add_load(ctx, instrs, record, &c->node, field->type, loc); } +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, + const struct vkd3d_shader_location *loc); + +static struct hlsl_ir_node *add_matrix_scalar_load(struct hlsl_ctx *ctx, struct list *instrs, + struct hlsl_ir_node *matrix, struct hlsl_ir_node *x, struct hlsl_ir_node *y, + const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *major, *minor; + struct hlsl_ir_expr *mul, *add; + struct hlsl_ir_constant *four; + struct hlsl_ir_load *load; + struct hlsl_type *type = matrix->data_type, *scalar_type; + + scalar_type = hlsl_get_scalar_type(ctx, type->base_type); + + if (type->modifiers & HLSL_MODIFIER_ROW_MAJOR) + { + minor = x; + major = y; + } + else + { + minor = y; + major = x; + } + + if (!(four = hlsl_new_uint_constant(ctx, 4, loc))) + return NULL; + list_add_tail(instrs, &four->node.entry); + + if (!(mul = add_binary_arithmetic_expr(ctx, instrs, HLSL_OP2_MUL, &four->node, major, loc))) + return NULL; + + if (!(add = add_binary_arithmetic_expr(ctx, instrs, HLSL_OP2_ADD, &mul->node, minor, loc))) + return NULL; + + if (!(load = add_load(ctx, instrs, matrix, &add->node, scalar_type, *loc))) + return NULL; + + return &load->node; +} + +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 vkd3d_string_buffer *name; + static unsigned int counter = 0; + struct hlsl_ir_load *load; + struct hlsl_ir_var *var; + unsigned int i; + + ret_type = hlsl_get_vector_type(ctx, mat_type->base_type, mat_type->dimx); + + name = vkd3d_string_buffer_get(&ctx->string_buffers); + vkd3d_string_buffer_printf(name, "<index-%u>", counter++); + var = hlsl_new_synthetic_var(ctx, name->buffer, ret_type, *loc); + vkd3d_string_buffer_release(&ctx->string_buffers, name); + if (!var) + return false; + + for (i = 0; i < mat_type->dimx; ++i) + { + struct hlsl_ir_store *store; + struct hlsl_ir_node *value; + struct hlsl_ir_constant *c; + + if (!(c = hlsl_new_uint_constant(ctx, i, loc))) + return false; + list_add_tail(instrs, &c->node.entry); + + if (!(value = add_matrix_scalar_load(ctx, instrs, matrix, &c->node, index, loc))) + return false; + + if (!(store = hlsl_new_store(ctx, var, &c->node, value, 0, *loc))) + return false; + list_add_tail(instrs, &store->node.entry); + } + + if (!(load = hlsl_new_var_load(ctx, var, *loc))) + return false; + list_add_tail(instrs, &load->node.entry); + + return true; +} + static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *array, struct hlsl_ir_node *index, const struct vkd3d_shader_location loc) { @@ -597,9 +684,7 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls } else if (expr_type->type == HLSL_CLASS_MATRIX) { - /* This needs to be lowered now, while we still have type information. */ - FIXME("Index of matrix type.\n"); - return false; + return add_matrix_index(ctx, instrs, array, index, &loc); } else if (expr_type->type == HLSL_CLASS_VECTOR) { diff --git a/tests/hlsl-initializer-matrix.shader_test b/tests/hlsl-initializer-matrix.shader_test index c4fafea7..790d402d 100644 --- a/tests/hlsl-initializer-matrix.shader_test +++ b/tests/hlsl-initializer-matrix.shader_test @@ -6,7 +6,7 @@ float4 main() : SV_TARGET } [test] -todo draw quad +draw quad probe all rgba (21, 22, 23, 0) @@ -18,7 +18,7 @@ float4 main() : SV_TARGET } [test] -todo draw quad +draw quad probe all rgba (21, 22, 23, 0) @@ -30,7 +30,7 @@ float4 main() : SV_TARGET } [test] -todo draw quad +draw quad probe all rgba (21, 22, 23, 24) @@ -42,7 +42,7 @@ float4 main() : SV_TARGET } [test] -todo draw quad +draw quad probe all rgba (21, 22, 31, 32) diff --git a/tests/hlsl-matrix-indexing.shader_test b/tests/hlsl-matrix-indexing.shader_test index 8057179e..a0a89829 100644 --- a/tests/hlsl-matrix-indexing.shader_test +++ b/tests/hlsl-matrix-indexing.shader_test @@ -11,7 +11,7 @@ uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 5.0 6.0 7.0 8.0 uniform 8 float4 9.0 10.0 11.0 12.0 uniform 12 float4 13.0 14.0 15.0 16.0 -todo draw quad +draw quad probe all rgba (1.0, 2.0, 10.0, 15.0) [pixel shader] @@ -27,7 +27,7 @@ uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 5.0 6.0 7.0 8.0 uniform 8 float4 9.0 10.0 11.0 12.0 uniform 12 float4 13.0 14.0 15.0 16.0 -todo draw quad +draw quad probe all rgba (1.0, 2.0, 10.0, 15.0) [pixel shader] @@ -43,7 +43,7 @@ uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 5.0 6.0 7.0 8.0 uniform 8 float4 9.0 10.0 11.0 12.0 uniform 12 float4 13.0 14.0 15.0 16.0 -todo draw quad +draw quad probe all rgba (1.0, 5.0, 7.0, 12.0) [pixel shader] @@ -58,5 +58,5 @@ float4 main() : SV_TARGET [test] uniform 0 float4 1.0 2.0 3.0 0.0 uniform 4 float4 5.0 6.0 7.0 0.0 -todo draw quad +draw quad probe all rgba (1.0, 3.0, 6.0, 7.0) -- 2.35.2
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- v2: * Do not use the comma operator inside a "for" statement. * Prepend "add_" to the name of functions that mutate the instruction stream. * Add a vector and a matrix to the constructor test. v3: * Move the "idx++" expression in a dedicated statement. * Rewritten in the same philosphy as Francisco's initializers work --- libs/vkd3d-shader/hlsl.y | 46 ++++++++++++------- tests/hlsl-matrix-indexing.shader_test | 16 +++++++ ...numeric-constructor-truncation.shader_test | 2 +- ...lsl-return-implicit-conversion.shader_test | 8 ++-- 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 887bcdc8..f2384d64 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2035,16 +2035,12 @@ static struct list *add_call(struct hlsl_ctx *ctx, const char *name, static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type *type, struct parse_initializer *params, struct vkd3d_shader_location loc) { - unsigned int i, writemask_offset = 0; - struct hlsl_ir_store *store; static unsigned int counter; + unsigned int i, j, idx = 0; struct hlsl_ir_load *load; struct hlsl_ir_var *var; char name[23]; - if (type->type == HLSL_CLASS_MATRIX) - hlsl_fixme(ctx, &loc, "Matrix constructor."); - sprintf(name, "<constructor-%x>", counter++); if (!(var = hlsl_new_synthetic_var(ctx, name, type, loc))) return NULL; @@ -2066,22 +2062,38 @@ static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type *type } width = hlsl_type_component_count(arg->data_type); - if (width > 4) + for (j = 0; j < width; ++j) { - FIXME("Constructor argument with %u components.\n", width); - continue; - } + struct hlsl_ir_constant *src_offset_const, *dst_offset_const; + struct hlsl_type *src_type, *dst_type; + unsigned int src_offset, dst_offset; + struct hlsl_ir_node *converted; + struct hlsl_ir_store *store; + struct hlsl_ir_load *value; - if (!(arg = add_implicit_conversion(ctx, params->instrs, arg, - hlsl_get_vector_type(ctx, type->base_type, width), &arg->loc))) - continue; + src_offset = hlsl_compute_component_reg_offset(ctx, arg->data_type, j, &src_type, &arg->loc); + dst_offset = hlsl_compute_component_reg_offset(ctx, type, idx, &dst_type, &arg->loc); - if (!(store = hlsl_new_store(ctx, var, NULL, arg, - ((1u << width) - 1) << writemask_offset, arg->loc))) - return NULL; - list_add_tail(params->instrs, &store->node.entry); + if (!(src_offset_const = hlsl_new_uint_constant(ctx, src_offset, &loc))) + return NULL; + list_add_tail(params->instrs, &src_offset_const->node.entry); - writemask_offset += width; + if (!(dst_offset_const = hlsl_new_uint_constant(ctx, dst_offset, &loc))) + return NULL; + list_add_tail(params->instrs, &dst_offset_const->node.entry); + + if (!(value = add_load(ctx, params->instrs, arg, &src_offset_const->node, src_type, loc))) + return NULL; + + if (!(converted = add_implicit_conversion(ctx, params->instrs, &value->node, dst_type, &arg->loc))) + return NULL; + + if (!(store = hlsl_new_store(ctx, var, &dst_offset_const->node, converted, 0, arg->loc))) + return NULL; + list_add_tail(params->instrs, &store->node.entry); + + idx++; + } } if (!(load = hlsl_new_var_load(ctx, var, loc))) diff --git a/tests/hlsl-matrix-indexing.shader_test b/tests/hlsl-matrix-indexing.shader_test index a0a89829..9336a67a 100644 --- a/tests/hlsl-matrix-indexing.shader_test +++ b/tests/hlsl-matrix-indexing.shader_test @@ -60,3 +60,19 @@ uniform 0 float4 1.0 2.0 3.0 0.0 uniform 4 float4 5.0 6.0 7.0 0.0 draw quad probe all rgba (1.0, 3.0, 6.0, 7.0) + +[pixel shader] +float4 main() : SV_TARGET +{ + float2x2 a = float2x2(11.0, 12.0, 13.0, 14.0); + float4x4 m = float4x4(1.0, 2.0, 3.0, 4.0, + float3(5.0, 6.0, 7.0), 8.0, + 9.0, 10.0, + a, + 15.0, 16.0); + return float4(m[0][0], m[1][0], m[1][2], m[2][3]); +} + +[test] +todo draw quad +probe all rgba (1.0, 5.0, 7.0, 12.0) diff --git a/tests/hlsl-numeric-constructor-truncation.shader_test b/tests/hlsl-numeric-constructor-truncation.shader_test index 27768dcc..f18b34d6 100644 --- a/tests/hlsl-numeric-constructor-truncation.shader_test +++ b/tests/hlsl-numeric-constructor-truncation.shader_test @@ -29,5 +29,5 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe all rgba (1.0, 2.0, 3.0, 5.0) diff --git a/tests/hlsl-return-implicit-conversion.shader_test b/tests/hlsl-return-implicit-conversion.shader_test index 38d21633..bf99d9cb 100644 --- a/tests/hlsl-return-implicit-conversion.shader_test +++ b/tests/hlsl-return-implicit-conversion.shader_test @@ -98,7 +98,7 @@ float4 main() : sv_target todo draw quad probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader fail todo] +[pixel shader fail] float3x1 func() { return float1x3(0.4, 0.3, 0.2); @@ -109,7 +109,7 @@ float4 main() : sv_target return float4(func(), 0.0); } -[pixel shader fail todo] +[pixel shader fail] float1x3 func() { return float3x1(0.4, 0.3, 0.2); @@ -191,7 +191,7 @@ float4 main() : sv_target todo draw quad probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader fail todo] +[pixel shader fail] float3x1 func() { return float1x4(0.4, 0.3, 0.2, 0.1); @@ -217,7 +217,7 @@ float4 main() : sv_target todo draw quad probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader fail todo] +[pixel shader fail] float1x3 func() { return float4x1(0.4, 0.3, 0.2, 0.1); -- 2.35.2
Hi, Il 22/04/22 12:25, Giovanni Mascellani ha scritto:
- if (!(arg = add_implicit_conversion(ctx, params->instrs, arg, - hlsl_get_vector_type(ctx, type->base_type, width), &arg->loc))) - continue; + src_offset = hlsl_compute_component_reg_offset(ctx, arg->data_type, j, &src_type, &arg->loc); + dst_offset = hlsl_compute_component_reg_offset(ctx, type, idx, &dst_type, &arg->loc);
- if (!(store = hlsl_new_store(ctx, var, NULL, arg, - ((1u << width) - 1) << writemask_offset, arg->loc))) - return NULL; - list_add_tail(params->instrs, &store->node.entry); + if (!(src_offset_const = hlsl_new_uint_constant(ctx, src_offset, &loc))) + return NULL; + list_add_tail(params->instrs, &src_offset_const->node.entry);
- writemask_offset += width; + if (!(dst_offset_const = hlsl_new_uint_constant(ctx, dst_offset, &loc))) + return NULL; + list_add_tail(params->instrs, &dst_offset_const->node.entry); + + if (!(value = add_load(ctx, params->instrs, arg, &src_offset_const->node, src_type, loc))) + return NULL; + + if (!(converted = add_implicit_conversion(ctx, params->instrs, &value->node, dst_type, &arg->loc))) + return NULL; + + if (!(store = hlsl_new_store(ctx, var, &dst_offset_const->node, converted, 0, arg->loc))) + return NULL; + list_add_tail(params->instrs, &store->node.entry); + + idx++;
I just realized that here I am basically reimplementing Francisco's initialize_var_components(). I should probably reuse that one instead. Giovanni.
Hello, April 22, 2022 6:25 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_constant_ops.c | 9 ++++++++- tests/arithmetic-float.shader_test | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c index 06b957ba..a1551ba1 100644 --- a/libs/vkd3d-shader/hlsl_constant_ops.c +++ b/libs/vkd3d-shader/hlsl_constant_ops.c @@ -18,6 +18,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
+#include <math.h> + #include "hlsl.h"
static bool fold_cast(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, struct hlsl_ir_constant *src) @@ -270,12 +272,17 @@ static bool fold_div(struct hlsl_ctx *ctx, struct hlsl_ir_constant *dst, { case HLSL_TYPE_FLOAT: case HLSL_TYPE_HALF: - if (src2->value[k].f == 0) + if (ctx->profile->major_version >= 4 && src2->value[k].f == 0) { hlsl_warning(ctx, &dst->node.loc, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO, "Floating point division by zero"); } dst->value[k].f = src1->value[k].f / src2->value[k].f; + if (ctx->profile->major_version < 4 && isinf(dst->value[k].f)) + { + hlsl_error(ctx, &dst->node.loc, VKD3D_SHADER_ERROR_HLSL_DIVISION_BY_ZERO, + "Infinities are not allowed by the shader model."); + } break;
In the 0/0 case, the result is NAN and isinf(NAN) returns 0, and the error is not detected.
case HLSL_TYPE_DOUBLE: diff --git a/tests/arithmetic-float.shader_test b/tests/arithmetic-float.shader_test index f99b9728..6824c3f1 100644 --- a/tests/arithmetic-float.shader_test +++ b/tests/arithmetic-float.shader_test @@ -25,6 +25,7 @@ todo draw quad probe all rgba (5.0, 5.0, -5.0, 3.0)
[require] +% Infinities are not allowed in SM1 shader model >= 4.0
[pixel shader] -- 2.35.2
participants (4)
-
Francisco Casas -
Giovanni Mascellani -
Henri Verbeet -
Zebediah Figura