[PATCH 0/6] MR694: vkd3d-shader/hlsl: Improve SM1 support for non-float operations, part 2.
Patches with tests and other miscellaneous changes required before the remaining features in my [sm1_logic](https://gitlab.winehq.org/fcasas/vkd3d/-/commits/sm1_logic) branch. Includes some rebased spirv.c patches by Zeb. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/shader_runner_d3d11.c | 1 + tests/shader_runner_d3d9.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/tests/shader_runner_d3d11.c b/tests/shader_runner_d3d11.c index 528c04e9d..26ce81ed7 100644 --- a/tests/shader_runner_d3d11.c +++ b/tests/shader_runner_d3d11.c @@ -620,6 +620,7 @@ static bool d3d11_runner_draw(struct shader_runner *r, { cb = create_buffer(device, D3D11_BIND_CONSTANT_BUFFER, runner->r.uniform_count * sizeof(*runner->r.uniforms), 0, runner->r.uniforms); + ID3D11DeviceContext_VSSetConstantBuffers(context, 0, 1, &cb); ID3D11DeviceContext_PSSetConstantBuffers(context, 0, 1, &cb); } diff --git a/tests/shader_runner_d3d9.c b/tests/shader_runner_d3d9.c index 4ddb8ffe2..e4e1c6435 100644 --- a/tests/shader_runner_d3d9.c +++ b/tests/shader_runner_d3d9.c @@ -345,6 +345,10 @@ static bool d3d9_runner_draw(struct shader_runner *r, if (runner->r.uniform_count) { + hr = IDirect3DDevice9_SetVertexShaderConstantF(device, 0, + (const float *)runner->r.uniforms, runner->r.uniform_count / 4); + ok(hr == D3D_OK, "Failed to set uniforms, hr %#lx.\n", hr); + hr = IDirect3DDevice9_SetPixelShaderConstantF(device, 0, (const float *)runner->r.uniforms, runner->r.uniform_count / 4); ok(hr == D3D_OK, "Failed to set uniforms, hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694
From: Francisco Casas <fcasas(a)codeweavers.com> Otherwise we may get a failing "hlsl_types_are_equal(arg1->data_type, arg2->data_type)" assertion on hlsl_new_binary_expr() when creating the MUL. This happens in the test introducing in the following patch: int a, b, c; void main(out float4 res : COLOR1, in float4 pos : position, out float4 out_pos : sv_position) { out_pos = pos; res = a ? b/1000.0 : c/1000.0; } --- libs/vkd3d-shader/hlsl.c | 2 ++ libs/vkd3d-shader/hlsl_codegen.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 45d02ce2b..4fd43d776 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1344,6 +1344,8 @@ struct hlsl_ir_node *hlsl_new_binary_expr(struct hlsl_ctx *ctx, enum hlsl_ir_exp { struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = {arg1, arg2}; + FIXME("op %u\n", op); + assert(hlsl_types_are_equal(arg1->data_type, arg2->data_type)); return hlsl_new_expr(ctx, op, operands, arg1->data_type, &arg1->loc); } diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 8434a921a..9eb65dc01 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2948,7 +2948,7 @@ static bool lower_ternary(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, stru } else if (ctx->profile->major_version < 4 && ctx->profile->type == VKD3D_SHADER_TYPE_VERTEX) { - struct hlsl_ir_node *neg, *slt, *sum, *mul, *cond2; + struct hlsl_ir_node *neg, *slt, *sum, *cond2, *slt_cast, *mul; /* Expression used here is "slt(<cond>) * (first - second) + second". */ @@ -2980,7 +2980,11 @@ static bool lower_ternary(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, stru return false; hlsl_block_add_instr(block, sum); - if (!(mul = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, slt, sum))) + if (!(slt_cast = hlsl_new_cast(ctx, slt, sum->data_type, &instr->loc))) + return false; + hlsl_block_add_instr(block, slt_cast); + + if (!(mul = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, slt_cast, sum))) return false; hlsl_block_add_instr(block, mul); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694
From: Francisco Casas <fcasas(a)codeweavers.com> Currently these are failing for SM1 because VKD3DSIH_SLT is not being handled by spirv.c. --- Makefile.am | 1 + tests/hlsl/vertex-shader-ops.shader_test | 95 ++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 tests/hlsl/vertex-shader-ops.shader_test diff --git a/Makefile.am b/Makefile.am index b963e4c03..6dc7623a2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -212,6 +212,7 @@ vkd3d_shader_tests = \ tests/hlsl/uniform-semantics.shader_test \ tests/hlsl/vector-indexing-uniform.shader_test \ tests/hlsl/vector-indexing.shader_test \ + tests/hlsl/vertex-shader-ops.shader_test \ tests/hlsl/writemask-assignop-0.shader_test \ tests/hlsl/writemask-assignop-1.shader_test \ tests/hlsl/writemask-assignop-2.shader_test \ diff --git a/tests/hlsl/vertex-shader-ops.shader_test b/tests/hlsl/vertex-shader-ops.shader_test new file mode 100644 index 000000000..8e7b63c8d --- /dev/null +++ b/tests/hlsl/vertex-shader-ops.shader_test @@ -0,0 +1,95 @@ +% The main objective of this test is to see if operations for SM1 vertex shaders work. +% Because they have a different instruction set that pixel shaders. + +% Considerations so that the vertex shader output matches the pixel shader input properly on all +% backends: +% - sv_position must be the last field in the output signature. +% - the d3d9 backend, on Windows, clamps the value of the COLORX semantics before passing it to +% the pixel shader, so their values must be between 0.0 and 1.0. + +[pixel shader] +float4 main(in float4 res : COLOR1) : sv_target +{ + return res; +} + + +[vertex shader todo(sm<4)] +int a, b; + +void main(out float4 res : COLOR1, in float4 pos : position, out float4 out_pos : sv_position) +{ + out_pos = pos; + + res.x = a == b; + res.y = a != b; + res.z = a > b; + res.w = b >= a; +} + +[test] +if(sm<4) uniform 0 float 3 +if(sm<4) uniform 4 float 4 +if(sm>=4) uniform 0 int4 3 4 0 0 +todo(sm<4) 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 -2 +if(sm>=4) uniform 0 int4 -2 -2 0 0 +todo(sm<4) draw quad +probe all rgba (1.0, 0.0, 0.0, 1.0) + + +[vertex shader todo(sm<4)] +int a, b; + +void main(out float4 res : COLOR1, in float4 pos : position, out float4 out_pos : sv_position) +{ + out_pos = pos; + + res.x = a && b; + res.y = a || b; + res.z = b && a; + res.w = b || a; +} + +[test] +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 +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 +probe all rgba (1.0, 1.0, 1.0, 1.0) + + +[require] +% The ternary operator works differently in sm6. See sm6-ternary.shader_test. +shader model < 6.0 + +[vertex shader todo(sm<4)] +int a, b, c; + +void main(out float4 res : COLOR1, in float4 pos : position, out float4 out_pos : sv_position) +{ + out_pos = pos; + + res = a ? b/1000.0 : c/1000.0; +} + +[test] +if(sm<4) uniform 0 float 0 +if(sm<4) uniform 4 float 100 +if(sm<4) uniform 8 float 200 +if(sm>=4) uniform 0 int4 0 100 200 0 +todo(sm<4) draw quad +probe all rgba (0.2, 0.2, 0.2, 0.2) +if(sm<4) uniform 0 float -4 +if(sm<4) uniform 4 float 100 +if(sm<4) uniform 8 float 200 +if(sm>=4) uniform 0 int4 -4 100 200 0 +todo(sm<4) draw quad +probe all rgba (0.1, 0.1, 0.1, 0.1) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/hlsl/float-comparison.shader_test | 15 +++++++++++++++ tests/hlsl/vertex-shader-ops.shader_test | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/tests/hlsl/float-comparison.shader_test b/tests/hlsl/float-comparison.shader_test index b66543443..48e7c4872 100644 --- a/tests/hlsl/float-comparison.shader_test +++ b/tests/hlsl/float-comparison.shader_test @@ -1,3 +1,18 @@ +% Check that -0.0f is not less than 0.0f +[pixel shader todo(sm<4)] +float a; + +float4 main() : sv_target +{ + return -0.0f < a; +} + +[test] +uniform 0 float 0.0 +todo(sm<4) draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0) + + [pixel shader todo(sm<4)] uniform float4 f; diff --git a/tests/hlsl/vertex-shader-ops.shader_test b/tests/hlsl/vertex-shader-ops.shader_test index 8e7b63c8d..0afaaa525 100644 --- a/tests/hlsl/vertex-shader-ops.shader_test +++ b/tests/hlsl/vertex-shader-ops.shader_test @@ -14,6 +14,24 @@ float4 main(in float4 res : COLOR1) : sv_target } +% Check that -0.0f is not less than 0.0f +[vertex shader todo(sm<4)] +float a; + +void main(out float4 res : COLOR1, in float4 pos : position, out float4 out_pos : sv_position) +{ + out_pos = pos; + + res = -0.0f < a; +} + +[test] +if(sm<4) uniform 0 float 0.0 +if(sm>=4) uniform 0 float4 0.0 0.0 0.0 0.0 +todo(sm<4) draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0) + + [vertex shader todo(sm<4)] int a, b; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Zeb originally wrote a comment with the concern on how this handles the comparisn between -0.0f and 0.0f, but I tested that. --- libs/vkd3d-shader/spirv.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index c0cbdccf6..c8acc73dc 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7173,8 +7173,15 @@ static void spirv_compiler_emit_movc(struct spirv_compiler *compiler, type_id = spirv_compiler_get_type_id_for_dst(compiler, dst); if (src[0].reg.data_type != VKD3D_DATA_BOOL) - condition_id = spirv_compiler_emit_int_to_bool(compiler, - VKD3D_SHADER_CONDITIONAL_OP_NZ, src[0].reg.data_type, component_count, condition_id); + { + if (instruction->handler_idx == VKD3DSIH_CMP) + condition_id = vkd3d_spirv_build_op_tr2(builder, &builder->function_stream, SpvOpFOrdGreaterThanEqual, + vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_BOOL, component_count), condition_id, + spirv_compiler_get_constant_float_vector(compiler, 0.0f, component_count)); + else + condition_id = spirv_compiler_emit_int_to_bool(compiler, + VKD3D_SHADER_CONDITIONAL_OP_NZ, src[0].reg.data_type, component_count, condition_id); + } val_id = vkd3d_spirv_build_op_select(builder, type_id, condition_id, src1_id, src2_id); spirv_compiler_emit_store_dst(compiler, dst, val_id); @@ -9551,6 +9558,7 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, break; case VKD3DSIH_DMOVC: case VKD3DSIH_MOVC: + case VKD3DSIH_CMP: spirv_compiler_emit_movc(compiler, instruction); break; case VKD3DSIH_SWAPC: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Note that I set the signedness argument to false when rebasing this patch, which I think is correct, but it wouldn't be bad to be sure. --- libs/vkd3d-shader/spirv.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index c8acc73dc..14d40ad81 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7693,6 +7693,36 @@ static void spirv_compiler_emit_comparison_instruction(struct spirv_compiler *co spirv_compiler_emit_store_reg(compiler, &dst->reg, dst->write_mask, result_id); } +static void spirv_compiler_emit_float_comparison_instruction(struct spirv_compiler *compiler, + const struct vkd3d_shader_instruction *instruction) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + const struct vkd3d_shader_dst_param *dst = instruction->dst; + const struct vkd3d_shader_src_param *src = instruction->src; + uint32_t src0_id, src1_id, type_id, result_id; + unsigned int component_count; + SpvOp op; + + switch (instruction->handler_idx) + { + case VKD3DSIH_SLT: op = SpvOpFOrdLessThan; break; + case VKD3DSIH_SGE: op = SpvOpFOrdGreaterThanEqual; break; + default: + vkd3d_unreachable(); + } + + component_count = vsir_write_mask_component_count(dst->write_mask); + + src0_id = spirv_compiler_emit_load_src(compiler, &src[0], dst->write_mask); + src1_id = spirv_compiler_emit_load_src(compiler, &src[1], dst->write_mask); + + type_id = vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_BOOL, component_count); + result_id = vkd3d_spirv_build_op_tr2(builder, &builder->function_stream, op, type_id, src0_id, src1_id); + + result_id = spirv_compiler_emit_bool_to_float(compiler, component_count, result_id, false); + spirv_compiler_emit_store_reg(compiler, &dst->reg, dst->write_mask, result_id); +} + static uint32_t spirv_compiler_emit_conditional_branch(struct spirv_compiler *compiler, const struct vkd3d_shader_instruction *instruction, uint32_t target_block_id) { @@ -9679,6 +9709,10 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_ULT: spirv_compiler_emit_comparison_instruction(compiler, instruction); break; + case VKD3DSIH_SLT: + case VKD3DSIH_SGE: + spirv_compiler_emit_float_comparison_instruction(compiler, instruction); + break; case VKD3DSIH_BFI: case VKD3DSIH_IBFE: case VKD3DSIH_UBFE: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694
2/6 seems to have a stray FIXME in it. I think 5/6 and 6/6 are fine but per discussion we may want them to be earlier lowering passes anyway? Note also you'll need to manually accept this one since it has my commits in it.
+% The main objective of this test is to see if operations for SM1 vertex shaders work. +% Because they have a different instruction set that pixel shaders.
Dependent clauses should be separated by a comma, and "that" seems to be a typographical error. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/694#note_63864
participants (4)
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)