[PATCH v4 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.
-- v4: vkd3d-shader/spirv: Implement SLT and SGE. vkd3d-shader/spirv: Implement CMP. tests: Check that -0.0f is not less than 0.0f. tests: Test comparison and logic operators on SM1 vertex shaders. vkd3d-shader/hlsl: Cast slt before multiplying on ternary operator. tests/shader-runner: Pass uniforms to vertex shaders in d3d9 and d3d11.
From: Francisco Casas fcasas@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);
From: Francisco Casas fcasas@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_codegen.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
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);
From: Francisco Casas fcasas@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 06fd43ab0..29527e490 100644 --- a/Makefile.am +++ b/Makefile.am @@ -213,6 +213,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..428808f1b --- /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 than 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)
From: Francisco Casas fcasas@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 619e62f85..c351ac098 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 428808f1b..34d496727 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;
From: Zebediah Figura zfigura@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 657219cf6..f6a5e84e4 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7205,8 +7205,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); @@ -9628,6 +9635,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:
From: Zebediah Figura zfigura@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 f6a5e84e4..744f5f594 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7745,6 +7745,36 @@ static void spirv_compiler_emit_orderedness_instruction(struct spirv_compiler *c spirv_compiler_emit_store_dst(compiler, dst, val_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) { @@ -9760,6 +9790,10 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_UNO: spirv_compiler_emit_orderedness_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:
This merge request was approved by Henri Verbeet.
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura