[PATCH v2 0/7] MR724: vkd3d-shader/hlsl: Improve SM1 support for non-float operations, part 4.
SM1 support for LOGIC_NOT, LOGIC_OR, and LOGIC_AND.
-- v2: vkd3d-shader/spirv: Throw compiler error on unmapped VKD3DSPR_MISCTYPE register. vkd3d-shader/spirv: Implement VKD3DSIH_ABS. vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_AND for SM1. vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_OR for SM1. vkd3d-shader/hlsl: Cast to bool before applying LOGIC_NOT.
From: Francisco Casas fcasas@codeweavers.com
--- tests/hlsl/logic-operations.shader_test | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tests/hlsl/logic-operations.shader_test b/tests/hlsl/logic-operations.shader_test index 360ca03b3..797e86797 100644 --- a/tests/hlsl/logic-operations.shader_test +++ b/tests/hlsl/logic-operations.shader_test @@ -156,3 +156,26 @@ float4 main() : SV_TARGET [test] draw quad probe all rgba (0.0, 1.0, 1.0, 1.0) + + +[pixel shader todo(sm<4)] +int a, b; + +float4 main() : SV_TARGET +{ + return float4(!(a && b), !!(a || b), !!!a, !(a < b)); +} + +[test] +if(sm<4) uniform 0 float 5 +if(sm<4) uniform 4 float 0 +if(sm>=4) uniform 0 int 5 +if(sm>=4) uniform 1 int 0 +todo(sm<4) draw quad +probe all rgba (1.0, 1.0, 0.0, 1.0) +if(sm<4) uniform 0 float -1 +if(sm<4) uniform 4 float 3 +if(sm>=4) uniform 0 int -1 +if(sm>=4) uniform 1 int 3 +todo(sm<4) draw quad +probe all rgba (0.0, 1.0, 0.0, 0.0)
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl_codegen.c | 47 ++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index ff349ab49..808627e28 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2902,6 +2902,52 @@ static bool lower_floor(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct return true; }
+static bool lower_logic_not(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct hlsl_block *block) +{ + struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS]; + struct hlsl_ir_node *arg, *arg_cast, *neg, *one, *sub, *res; + struct hlsl_constant_value one_value; + struct hlsl_type *float_type; + struct hlsl_ir_expr *expr; + + if (instr->type != HLSL_IR_EXPR) + return false; + expr = hlsl_ir_expr(instr); + if (expr->op != HLSL_OP1_LOGIC_NOT) + return false; + + arg = expr->operands[0].node; + float_type = hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, arg->data_type->dimx); + + if (!(arg_cast = hlsl_new_cast(ctx, arg, float_type, &arg->loc))) + return false; + hlsl_block_add_instr(block, arg_cast); + + if (!(neg = hlsl_new_unary_expr(ctx, HLSL_OP1_NEG, arg_cast, &instr->loc))) + return false; + hlsl_block_add_instr(block, neg); + + one_value.u[0].f = 1.0; + one_value.u[1].f = 1.0; + one_value.u[2].f = 1.0; + one_value.u[3].f = 1.0; + if (!(one = hlsl_new_constant(ctx, float_type, &one_value, &instr->loc))) + return false; + hlsl_block_add_instr(block, one); + + if (!(sub = hlsl_new_binary_expr(ctx, HLSL_OP2_ADD, one, neg))) + return false; + hlsl_block_add_instr(block, sub); + + memset(operands, 0, sizeof(operands)); + operands[0] = sub; + if (!(res = hlsl_new_expr(ctx, HLSL_OP1_REINTERPRET, operands, instr->data_type, &instr->loc))) + return false; + hlsl_block_add_instr(block, res); + + return true; +} + /* Use movc/cmp for the ternary operator. */ static bool lower_ternary(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct hlsl_block *block) { @@ -5420,6 +5466,7 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry lower_ir(ctx, lower_ceil, body); lower_ir(ctx, lower_floor, body); lower_ir(ctx, lower_comparison_operators, body); + lower_ir(ctx, lower_logic_not, body); if (ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL) lower_ir(ctx, lower_slt, body); else
From: Francisco Casas fcasas@codeweavers.com
Before this commit, it is possible for one of the tests of cf-cond-types.shader_test to pass a non-bool to LOGIC_NOT, which should not be allowed. --- libs/vkd3d-shader/hlsl.y | 10 ++++++++-- libs/vkd3d-shader/hlsl_codegen.c | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ec8b3d22a..2824e26dc 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -438,8 +438,9 @@ static uint32_t add_modifiers(struct hlsl_ctx *ctx, uint32_t modifiers, uint32_t
static bool append_conditional_break(struct hlsl_ctx *ctx, struct hlsl_block *cond_block) { - struct hlsl_ir_node *condition, *not, *iff, *jump; + struct hlsl_ir_node *condition, *cast, *not, *iff, *jump; struct hlsl_block then_block; + struct hlsl_type *bool_type;
/* E.g. "for (i = 0; ; ++i)". */ if (list_empty(&cond_block->instrs)) @@ -449,7 +450,12 @@ static bool append_conditional_break(struct hlsl_ctx *ctx, struct hlsl_block *co
check_condition_type(ctx, condition);
- if (!(not = hlsl_new_unary_expr(ctx, HLSL_OP1_LOGIC_NOT, condition, &condition->loc))) + bool_type = hlsl_get_scalar_type(ctx, HLSL_TYPE_BOOL); + if (!(cast = hlsl_new_cast(ctx, condition, bool_type, &condition->loc))) + return false; + hlsl_block_add_instr(cond_block, cast); + + if (!(not = hlsl_new_unary_expr(ctx, HLSL_OP1_LOGIC_NOT, cast, &condition->loc))) return false; hlsl_block_add_instr(cond_block, not);
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 808627e28..a53474f74 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2919,6 +2919,9 @@ static bool lower_logic_not(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, st arg = expr->operands[0].node; float_type = hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, arg->data_type->dimx);
+ /* If this is happens, it means we failed to cast the argument to boolean somewhere. */ + assert(arg->data_type->base_type == HLSL_TYPE_BOOL); + if (!(arg_cast = hlsl_new_cast(ctx, arg, float_type, &arg->loc))) return false; hlsl_block_add_instr(block, arg_cast);
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/d3dbc.c | 4 ++++ libs/vkd3d-shader/hlsl_codegen.c | 1 + 2 files changed, 5 insertions(+)
diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 9f153a1da..710f88361 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -2333,6 +2333,10 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b } break;
+ case HLSL_OP2_LOGIC_OR: + write_sm1_binary_op(ctx, buffer, D3DSIO_MAX, &instr->reg, &arg1->reg, &arg2->reg); + break; + case HLSL_OP2_SLT: if (ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL) hlsl_fixme(ctx, &instr->loc, "Lower SLT instructions for pixel shaders."); diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index a53474f74..38f92a99b 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3622,6 +3622,7 @@ static bool lower_nonfloat_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *inst case HLSL_OP1_NEG: case HLSL_OP2_ADD: case HLSL_OP2_DIV: + case HLSL_OP2_LOGIC_OR: case HLSL_OP2_MAX: case HLSL_OP2_MIN: case HLSL_OP2_MUL:
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/d3dbc.c | 4 ++++ libs/vkd3d-shader/hlsl_codegen.c | 1 + tests/hlsl/logic-operations.shader_test | 6 +++--- tests/hlsl/vertex-shader-ops.shader_test | 6 +++--- 4 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 710f88361..e44f20e2f 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -2333,6 +2333,10 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b } break;
+ case HLSL_OP2_LOGIC_AND: + write_sm1_binary_op(ctx, buffer, D3DSIO_MIN, &instr->reg, &arg1->reg, &arg2->reg); + break; + case HLSL_OP2_LOGIC_OR: write_sm1_binary_op(ctx, buffer, D3DSIO_MAX, &instr->reg, &arg1->reg, &arg2->reg); break; diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 38f92a99b..26de593e1 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3622,6 +3622,7 @@ static bool lower_nonfloat_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *inst case HLSL_OP1_NEG: case HLSL_OP2_ADD: case HLSL_OP2_DIV: + case HLSL_OP2_LOGIC_AND: case HLSL_OP2_LOGIC_OR: case HLSL_OP2_MAX: case HLSL_OP2_MIN: diff --git a/tests/hlsl/logic-operations.shader_test b/tests/hlsl/logic-operations.shader_test index 797e86797..120187783 100644 --- a/tests/hlsl/logic-operations.shader_test +++ b/tests/hlsl/logic-operations.shader_test @@ -158,7 +158,7 @@ draw quad probe all rgba (0.0, 1.0, 1.0, 1.0)
-[pixel shader todo(sm<4)] +[pixel shader] int a, b;
float4 main() : SV_TARGET @@ -171,11 +171,11 @@ if(sm<4) uniform 0 float 5 if(sm<4) uniform 4 float 0 if(sm>=4) uniform 0 int 5 if(sm>=4) uniform 1 int 0 -todo(sm<4) draw quad +draw quad probe all rgba (1.0, 1.0, 0.0, 1.0) if(sm<4) uniform 0 float -1 if(sm<4) uniform 4 float 3 if(sm>=4) uniform 0 int -1 if(sm>=4) uniform 1 int 3 -todo(sm<4) draw quad +draw quad probe all rgba (0.0, 1.0, 0.0, 0.0) diff --git a/tests/hlsl/vertex-shader-ops.shader_test b/tests/hlsl/vertex-shader-ops.shader_test index 38f3db658..4783b4823 100644 --- a/tests/hlsl/vertex-shader-ops.shader_test +++ b/tests/hlsl/vertex-shader-ops.shader_test @@ -58,7 +58,7 @@ draw quad probe all rgba (1.0, 0.0, 0.0, 1.0)
-[vertex shader todo(sm<4)] +[vertex shader] int a, b;
void main(out float4 res : COLOR1, in float4 pos : position, out float4 out_pos : sv_position) @@ -75,12 +75,12 @@ void main(out float4 res : COLOR1, in float4 pos : position, out float4 out_pos if(sm<4) uniform 0 float 0 if(sm<4) uniform 4 float 2 if(sm>=4) uniform 0 int4 0 2 0 0 -todo(sm<4) draw quad +draw quad probe all rgba (0.0, 1.0, 0.0, 1.0) if(sm<4) uniform 0 float -2 if(sm<4) uniform 4 float 8 if(sm>=4) uniform 0 int4 -2 8 0 0 -todo(sm<4) draw quad +draw quad probe all rgba (1.0, 1.0, 1.0, 1.0)
From: Zebediah Figura zfigura@codeweavers.com
fcasas: Note that the tests in fwidth.shader_test passes compilation but fail in retrieving the correct result because spirv.c is not capable of mapping the vPos register. --- libs/vkd3d-shader/spirv.c | 2 ++ tests/hlsl/abs.shader_test | 4 ++-- tests/hlsl/float-comparison.shader_test | 2 +- tests/hlsl/fwidth.shader_test | 12 ++++++------ tests/hlsl/inverse-trig.shader_test | 20 ++++++++++---------- tests/hlsl/ternary.shader_test | 4 ++-- 6 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 5403e5443..43d79da9e 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7123,6 +7123,7 @@ static enum GLSLstd450 spirv_compiler_map_ext_glsl_instruction( } glsl_insts[] = { + {VKD3DSIH_ABS, GLSLstd450FAbs}, {VKD3DSIH_ACOS, GLSLstd450Acos}, {VKD3DSIH_ASIN, GLSLstd450Asin}, {VKD3DSIH_ATAN, GLSLstd450Atan}, @@ -9808,6 +9809,7 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_ISFINITE: spirv_compiler_emit_isfinite(compiler, instruction); break; + case VKD3DSIH_ABS: case VKD3DSIH_ACOS: case VKD3DSIH_ASIN: case VKD3DSIH_ATAN: diff --git a/tests/hlsl/abs.shader_test b/tests/hlsl/abs.shader_test index 4d1d1e33e..738e69312 100644 --- a/tests/hlsl/abs.shader_test +++ b/tests/hlsl/abs.shader_test @@ -8,8 +8,8 @@ float4 main() : sv_target
[test] uniform 0 float4 0.1 0.7 0.0 0.0 -todo(sm<4 | sm>=6) draw quad +todo(sm>=6) draw quad probe all rgba (0.1, 0.7, 0.4, 0.4) uniform 0 float4 -0.7 0.1 0.0 0.0 -todo(sm<4 | sm>=6) draw quad +todo(sm>=6) draw quad probe all rgba (0.7, 0.1, 1.2, 0.4) diff --git a/tests/hlsl/float-comparison.shader_test b/tests/hlsl/float-comparison.shader_test index 9a5ec93c7..eb58c7a74 100644 --- a/tests/hlsl/float-comparison.shader_test +++ b/tests/hlsl/float-comparison.shader_test @@ -107,7 +107,7 @@ float4 main() : sv_target [test] uniform 0 float4 1e-37 1e-37 1e+38 1e+38 uniform 4 float4 0 -1e-37 1e+38 -1e+38 -todo(sm<4) draw quad +draw quad probe all rgba (0.0, 0.0, 1.0, 0.0)
diff --git a/tests/hlsl/fwidth.shader_test b/tests/hlsl/fwidth.shader_test index 10ed712d2..be3ad0925 100644 --- a/tests/hlsl/fwidth.shader_test +++ b/tests/hlsl/fwidth.shader_test @@ -18,9 +18,9 @@ float4 main(float4 pos : sv_position) : sv_target }
[test] -todo(sm<4 | sm>=6) draw quad -probe (10, 10) rgba (8.0, 8.0, 8.0, 8.0) -probe (11, 10) rgba (8.0, 8.0, 8.0, 8.0) -probe (12, 10) rgba (10.0, 10.0, 10.0, 10.0) -probe (16, 16) rgba (12.0, 12.0, 12.0, 12.0) -probe (150, 150) rgba (92.0, 92.0, 92.0, 92.0) +todo(sm>=6) draw quad +todo(sm<4) probe (10, 10) rgba (8.0, 8.0, 8.0, 8.0) +todo(sm<4) probe (11, 10) rgba (8.0, 8.0, 8.0, 8.0) +todo(sm<4) probe (12, 10) rgba (10.0, 10.0, 10.0, 10.0) +todo(sm<4) probe (16, 16) rgba (12.0, 12.0, 12.0, 12.0) +todo(sm<4) probe (150, 150) rgba (92.0, 92.0, 92.0, 92.0) diff --git a/tests/hlsl/inverse-trig.shader_test b/tests/hlsl/inverse-trig.shader_test index 27a5025c2..beb62492f 100644 --- a/tests/hlsl/inverse-trig.shader_test +++ b/tests/hlsl/inverse-trig.shader_test @@ -13,23 +13,23 @@ float4 main() : sv_target
[test] uniform 0 float4 -1.0 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (3.14159274, 0.0, 0.0, 0.0) 128
uniform 0 float4 -0.5 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (2.094441441, 0.0, 0.0, 0.0) 256
uniform 0 float4 0.0 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (1.57072878, 0.0, 0.0, 0.0) 1024
uniform 0 float4 0.5 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (1.04715133, 0.0, 0.0, 0.0) 512
uniform 0 float4 1.0 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (0.0, 0.0, 0.0, 0.0) 128
[pixel shader] @@ -44,7 +44,7 @@ float4 main() : sv_target
[test] uniform 0 float4 -1.0 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (-31416.0, 0.0, 0.0, 0.0)
[require] @@ -52,15 +52,15 @@ shader model < 6.0
[test] uniform 0 float4 -0.5 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (-10473.0, 0.0, 0.0, 0.0)
uniform 0 float4 0.0 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (1.0, 0.0, 0.0, 0.0)
uniform 0 float4 0.5 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (10473.0, 0.0, 0.0, 0.0)
[require] @@ -88,7 +88,7 @@ probe all rgba (10472.0, 0.0, 0.0, 0.0) 4096
[test] uniform 0 float4 1.0 0.0 0.0 0.0 -todo(sm<4) draw quad +draw quad probe all rgba (31416.0, 0.0, 0.0, 0.0)
diff --git a/tests/hlsl/ternary.shader_test b/tests/hlsl/ternary.shader_test index b61a10a6c..995dfd56c 100644 --- a/tests/hlsl/ternary.shader_test +++ b/tests/hlsl/ternary.shader_test @@ -67,7 +67,7 @@ float4 main() : sv_target uniform 0 float4 0.0 1.0 0.0 -3.0 uniform 4 float4 1.0 2.0 3.0 4.0 uniform 8 float4 5.0 6.0 7.0 8.0 -todo(sm<4) draw quad +draw quad probe all rgba (5.0, 2.0, 7.0, 4.0)
@@ -295,7 +295,7 @@ float4 main() : sv_target uniform 0 float4 1.0 0.0 1.0 0.0 uniform 4 float4 1.0 2.0 3.0 4.0 uniform 8 float4 5.0 6.0 7.0 8.0 -todo(sm<4) draw quad +draw quad probe all rgba (1.0, 5.0, 1.0, 5.0)
From: Francisco Casas fcasas@codeweavers.com
Otherwise we will transpile d3dbc shaders that use the vPos (or other of these special registers) without properly handling them, which gives invalid results.
Some SM1 tests must be set back to "todo" but they only work because, by coincidence, we are assigning vPos the input register with index 0. Propper mapping of these registers is required. --- libs/vkd3d-shader/spirv.c | 8 +++++++- tests/hlsl/ddxddy.shader_test | 6 +++--- tests/hlsl/fwidth.shader_test | 12 ++++++------ 3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 43d79da9e..df06314da 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -3396,7 +3396,7 @@ struct vkd3d_shader_register_info bool is_aggregate; };
-static bool spirv_compiler_get_register_info(const struct spirv_compiler *compiler, +static bool spirv_compiler_get_register_info(struct spirv_compiler *compiler, const struct vkd3d_shader_register *reg, struct vkd3d_shader_register_info *register_info) { struct vkd3d_symbol reg_symbol, *symbol; @@ -3419,6 +3419,12 @@ static bool spirv_compiler_get_register_info(const struct spirv_compiler *compil return true; }
+ if (reg->type == VKD3DSPR_MISCTYPE) + { + spirv_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_TYPE, + "Unmapped VKD3DSPR_MISCTYPE register."); + } + vkd3d_symbol_make_register(®_symbol, reg); if (!(entry = rb_get(&compiler->symbol_table, ®_symbol))) { diff --git a/tests/hlsl/ddxddy.shader_test b/tests/hlsl/ddxddy.shader_test index 72925b975..f61b560c9 100644 --- a/tests/hlsl/ddxddy.shader_test +++ b/tests/hlsl/ddxddy.shader_test @@ -8,8 +8,8 @@ float4 main(float4 pos : sv_position) : sv_target }
[test] -draw quad -todo(sm<4) probe all rgba (1.0, 1.0, 0.0, 0.0) +todo(sm<4) draw quad +probe all rgba (1.0, 1.0, 0.0, 0.0)
[pixel shader] @@ -29,7 +29,7 @@ float4 main(float4 pos : sv_position) : sv_target }
[test] -draw quad +todo(sm<4) draw quad todo(sm<4) probe (10, 10) rgba (-16.0, -5.0, 3.0, 0.0) todo(sm<4) probe (11, 10) rgba (-21.0, -5.0, 3.0, 0.0) todo(sm<4) probe (10, 11) rgba (-13.0, -5.0, 3.0, 0.0) diff --git a/tests/hlsl/fwidth.shader_test b/tests/hlsl/fwidth.shader_test index be3ad0925..10ed712d2 100644 --- a/tests/hlsl/fwidth.shader_test +++ b/tests/hlsl/fwidth.shader_test @@ -18,9 +18,9 @@ float4 main(float4 pos : sv_position) : sv_target }
[test] -todo(sm>=6) draw quad -todo(sm<4) probe (10, 10) rgba (8.0, 8.0, 8.0, 8.0) -todo(sm<4) probe (11, 10) rgba (8.0, 8.0, 8.0, 8.0) -todo(sm<4) probe (12, 10) rgba (10.0, 10.0, 10.0, 10.0) -todo(sm<4) probe (16, 16) rgba (12.0, 12.0, 12.0, 12.0) -todo(sm<4) probe (150, 150) rgba (92.0, 92.0, 92.0, 92.0) +todo(sm<4 | sm>=6) draw quad +probe (10, 10) rgba (8.0, 8.0, 8.0, 8.0) +probe (11, 10) rgba (8.0, 8.0, 8.0, 8.0) +probe (12, 10) rgba (10.0, 10.0, 10.0, 10.0) +probe (16, 16) rgba (12.0, 12.0, 12.0, 12.0) +probe (150, 150) rgba (92.0, 92.0, 92.0, 92.0)
On Fri Mar 15 16:43:10 2024 +0000, Giovanni Mascellani wrote:
Is the commit message for Zeb's commit intended?
Yes, it explains the rationale for the next commit. I am not sure if that is the best format to add a comment in other person's commit though.
On Fri Mar 15 16:42:58 2024 +0000, Francisco Casas wrote:
changed this line in [version 2 of the diff](/wine/vkd3d/-/merge_requests/724/diffs?diff_id=105363&start_sha=9ea6ad9855078861b4c5e123c8a9e3e865c22112#65224f45149d44504eb7d8073b46ace7b7a7a4ce_2337_2337)
That makes sense, I changed it to MIN.
On Fri Mar 15 16:42:57 2024 +0000, Francisco Casas wrote:
changed this line in [version 2 of the diff](/wine/vkd3d/-/merge_requests/724/diffs?diff_id=105363&start_sha=9ea6ad9855078861b4c5e123c8a9e3e865c22112#9155b9453b4ec8ea0b9b025dfb55c061bd931610_454_453)
Done.
On Sat Mar 16 04:34:38 2024 +0000, Zebediah Figura wrote:
Wait, how does vPos get into the symbol table in the first place? That doesn't seem right.
vPos doesn't get into `context->symbol_table`.
We enter the ```c if (!(entry = rb_get(&compiler->symbol_table, ®_symbol))) { FIXME("Unrecognized register (%s).\n", debug_vkd3d_symbol(®_symbol)); memset(register_info, 0, sizeof(*register_info)); return false; } ``` path.
But because this is a FIXME and not a compiler error, compilation continues using a zeroed `register_info` which, I think, results in a misscompilation.
---
In the fwidth.shader_test case in particular, we have a ``` mov r0, vPos ``` and when `spirv_compiler_emit_mov()` is called, we go through this path: ```c if (vkd3d_swizzle_is_equal(dst_reg_info.write_mask, src->swizzle, src_reg_info.write_mask)) { dst_id = spirv_compiler_get_register_id(compiler, &dst->reg); src_id = spirv_compiler_get_register_id(compiler, &src->reg);
vkd3d_spirv_build_op_copy_memory(builder, dst_id, src_id, SpvMemoryAccessMaskNone); return; } ```
and `spirv_compiler_get_register_id()` just ends up emitting a `SpvStorageClassPrivate` variable for the `src` register, instead of using the register in the input signature.
---
So patch 7/7 makes it throw a compiler error to avoid the misscompilation.
I guess a proper solution would be to map the registers whose type is `VKD3DSPR_MISCTYPE` to the right symbol in the input|output signature. In this case "VPOS", which gets assigned to type `VKD3DSPR_INPUT` and index 0. But I felt that that is out of the scope of this MR and maybe something for someone with more experience in spirv.c to handle.
(please tell me if I am missing something in my analysis!).
vPos doesn't get into `context->symbol_table`.
I meant, it doesn't get into the symbol table as `vPos` per-se, but it does get into the symbol table as a register with `reg->type=VKD3DSPR_INPUT` and `reg->idx=0`.
But because this is a FIXME and not a compiler error, compilation continues using a zeroed `register_info` which, I think, results in a misscompilation.
Shouldn't we turn that from a FIXME to a compiler error, then? The problem isn't going to be specific to MISCTYPE.
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)