[PATCH v2 0/6] MR489: vkd3d-shader: Prepare for 64-bit integer support.
This is ready for review, but may not go upstream until after the release. -- v2: vkd3d-shader/spirv: Add a parameter name for int64 capability. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489
From: Conor McCarthy <cmccarthy(a)codeweavers.com> DXIL does not use implicit masking of shift counts. --- libs/vkd3d-shader/d3d_asm.c | 6 ++++++ libs/vkd3d-shader/dxil.c | 6 ++++++ libs/vkd3d-shader/spirv.c | 4 ++-- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/d3d_asm.c b/libs/vkd3d-shader/d3d_asm.c index 82d1d71d9..d7b406acb 100644 --- a/libs/vkd3d-shader/d3d_asm.c +++ b/libs/vkd3d-shader/d3d_asm.c @@ -1577,6 +1577,12 @@ static void shader_dump_instruction_flags(struct vkd3d_d3d_asm_compiler *compile shader_addline(buffer, "p"); break; + case VKD3DSIH_ISHL: + case VKD3DSIH_ISHR: + case VKD3DSIH_USHR: + if (ins->flags & VKD3DSI_SHIFT_UNMASKED) + shader_addline(buffer, "_unmasked"); + /* fall through */ default: shader_dump_precise_flags(compiler, ins->flags); break; diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 1709212fa..41c3c677e 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -3166,6 +3166,12 @@ static void sm6_parser_emit_binop(struct sm6_parser *sm6, const struct dxil_reco } else { + if (handler_idx == VKD3DSIH_ISHL || handler_idx == VKD3DSIH_ISHR || handler_idx == VKD3DSIH_USHR) + { + /* DXC emits AND instructions where necessary to mask shift counts. Shift binops + * do not imply masking the shift as the TPF equivalents do. */ + ins->flags |= VKD3DSI_SHIFT_UNMASKED; + } instruction_dst_param_init_ssa_scalar(ins, sm6); } } diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index be149a0cf..1714dae3e 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -6709,8 +6709,8 @@ static enum vkd3d_result spirv_compiler_emit_alu_instruction(struct spirv_compil * Microsoft fxc will compile immediate constants larger than 5 bits. * Fixing up the constants would be more elegant, but the simplest way is * to let this handle constants too. */ - if (instruction->handler_idx == VKD3DSIH_ISHL || instruction->handler_idx == VKD3DSIH_ISHR - || instruction->handler_idx == VKD3DSIH_USHR) + if (!(instruction->flags & VKD3DSI_SHIFT_UNMASKED) && (instruction->handler_idx == VKD3DSIH_ISHL + || instruction->handler_idx == VKD3DSIH_ISHR || instruction->handler_idx == VKD3DSIH_USHR)) { uint32_t mask_id = spirv_compiler_get_constant_vector(compiler, VKD3D_SHADER_COMPONENT_UINT, vkd3d_write_mask_component_count(dst->write_mask), 0x1f); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index d3989672b..faea815f1 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -740,6 +740,7 @@ enum vkd3d_tessellator_domain #define VKD3DSI_RESINFO_UINT 0x2 #define VKD3DSI_SAMPLE_INFO_UINT 0x1 #define VKD3DSI_SAMPLER_COMPARISON_MODE 0x1 +#define VKD3DSI_SHIFT_UNMASKED 0x1 #define VKD3DSI_PRECISE_X 0x100 #define VKD3DSI_PRECISE_Y 0x200 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 8 ++++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 9 insertions(+) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 1714dae3e..74e6d2063 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -5438,6 +5438,14 @@ static void spirv_compiler_emit_dcl_global_flags(struct spirv_compiler *compiler flags &= ~(VKD3DSGF_ENABLE_DOUBLE_PRECISION_FLOAT_OPS | VKD3DSGF_ENABLE_11_1_DOUBLE_EXTENSIONS); } + if (flags & VKD3DSGF_ENABLE_INT64) + { + FIXME("Unsupported 64-bit integer ops.\n"); + spirv_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INT64_UNSUPPORTED, + "Support for 64-bit integers is not implemented."); + flags &= ~VKD3DSGF_ENABLE_INT64; + } + if (flags & ~(VKD3DSGF_REFACTORING_ALLOWED | VKD3DSGF_ENABLE_RAW_AND_STRUCTURED_BUFFERS)) FIXME("Unhandled global flags %#"PRIx64".\n", (uint64_t)flags); else diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index faea815f1..fe57cc6cf 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -96,6 +96,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_SPV_INVALID_TYPE = 2006, VKD3D_SHADER_ERROR_SPV_INVALID_HANDLER = 2007, VKD3D_SHADER_ERROR_SPV_NOT_IMPLEMENTED = 2008, + VKD3D_SHADER_ERROR_SPV_INT64_UNSUPPORTED = 2009, VKD3D_SHADER_WARNING_SPV_INVALID_SWIZZLE = 2300, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- tests/shader_runner.c | 5 +++++ tests/shader_runner.h | 1 + tests/shader_runner_d3d12.c | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 2b875440e..090857a87 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -191,6 +191,10 @@ static void parse_require_directive(struct shader_runner *runner, const char *li runner->compile_options |= options[i].option; } } + else if (match_string(line, "shader int64", &line)) + { + runner->require_int64 = true; + } else { fatal_error("Unknown require directive '%s'.\n", line); @@ -1312,6 +1316,7 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o state = STATE_REQUIRE; runner->minimum_shader_model = minimum_shader_model; runner->maximum_shader_model = maximum_shader_model; + runner->require_int64 = false; runner->compile_options = 0; skip_tests = false; } diff --git a/tests/shader_runner.h b/tests/shader_runner.h index a4f0d2fd5..1caf4bbf5 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -122,6 +122,7 @@ struct shader_runner char *fx_source; enum shader_model minimum_shader_model; enum shader_model maximum_shader_model; + bool require_int64; bool last_render_failed; diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c index a05dfd059..857f3679a 100644 --- a/tests/shader_runner_d3d12.c +++ b/tests/shader_runner_d3d12.c @@ -52,6 +52,8 @@ struct d3d12_shader_runner ID3D12GraphicsCommandList *compute_list; IDxcCompiler3 *dxc_compiler; + + D3D12_FEATURE_DATA_D3D12_OPTIONS1 options1; }; static struct d3d12_shader_runner *d3d12_shader_runner(struct shader_runner *r) @@ -96,6 +98,16 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons return blob; } +static bool d3d12_runner_check_requirements(struct shader_runner *r) +{ + struct d3d12_shader_runner *runner = d3d12_shader_runner(r); + + if (runner->r.require_int64 && !runner->options1.Int64ShaderOps) + return false; + + return true; +} + #define MAX_RESOURCE_DESCRIPTORS (MAX_RESOURCES * 2) static struct resource *d3d12_runner_create_resource(struct shader_runner *r, const struct resource_params *params) @@ -561,6 +573,7 @@ static void d3d12_runner_release_readback(struct shader_runner *r, struct resour static const struct shader_runner_ops d3d12_runner_ops = { + .check_requirements = d3d12_runner_check_requirements, .create_resource = d3d12_runner_create_resource, .destroy_resource = d3d12_runner_destroy_resource, .dispatch = d3d12_runner_dispatch, @@ -602,6 +615,12 @@ void run_shader_tests_d3d12(void *dxc_compiler, enum shader_model minimum_shader runner.compute_allocator, NULL, &IID_ID3D12GraphicsCommandList, (void **)&runner.compute_list); ok(hr == S_OK, "Failed to create command list, hr %#x.\n", hr); + hr = ID3D12Device_CheckFeatureSupport(device, D3D12_FEATURE_D3D12_OPTIONS1, + &runner.options1, sizeof(runner.options1)); + ok(hr == S_OK, "Failed to check feature options1 support, hr %#x.\n", hr); + if (maximum_shader_model >= SHADER_MODEL_6_0) + trace("Int64ShaderOps: %u.\n", runner.options1.Int64ShaderOps); + run_shader_tests(&runner.r, &d3d12_runner_ops, dxc_compiler, minimum_shader_model, maximum_shader_model); ID3D12GraphicsCommandList_Release(runner.compute_list); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- .../hlsl/arithmetic-float-uniform.shader_test | 49 ++++++++++++ tests/hlsl/arithmetic-int-uniform.shader_test | 77 +++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/tests/hlsl/arithmetic-float-uniform.shader_test b/tests/hlsl/arithmetic-float-uniform.shader_test index 4812d053a..bf6f92e29 100644 --- a/tests/hlsl/arithmetic-float-uniform.shader_test +++ b/tests/hlsl/arithmetic-float-uniform.shader_test @@ -90,3 +90,52 @@ float4 main() : SV_TARGET uniform 0 float4 1.0 0.0 0.0 0.0 draw quad probe all rgba (1e99, 1e99, 1e99, 1e99) + +[require] +shader model >= 5.0 + +[pixel shader todo] +uniform double2 a; + +float4 main() : SV_TARGET +{ + double x = a.x; + double y = a.y; + return float4(x + y, x - y, x * y, x / y); +} + +[test] +uniform 0 double2 7.5 -2.5 +todo(sm<6) draw quad +probe all rgba (5.0, 10.0, -18.75, -3.0) + +[pixel shader todo] +uniform double2 a; + +float4 main() : SV_TARGET +{ + double x = a.x; + double y = a.y; + return x * y; +} + +[test] +uniform 0 double2 3.0e-300 2.5e300 +todo(sm<6) draw quad +probe all rgba (7.5, 7.5, 7.5, 7.5) + +% Note: DXC does not support modulo on doubles. +[pixel shader todo] +uniform double2 a; + +float4 main() : SV_TARGET +{ + double x = a.x; + double y = a.y; + return x / y; +} + +[test] +uniform 0 double2 1.5e300 2.0e299 +todo(sm<6) draw quad +probe all rgba (7.5, 7.5, 7.5, 7.5) diff --git a/tests/hlsl/arithmetic-int-uniform.shader_test b/tests/hlsl/arithmetic-int-uniform.shader_test index 7f5cdaaa6..f27aff037 100644 --- a/tests/hlsl/arithmetic-int-uniform.shader_test +++ b/tests/hlsl/arithmetic-int-uniform.shader_test @@ -119,3 +119,80 @@ uniform 0 float4 45.0 5.0 50.0 10.0 uniform 4 float4 3.0 8.0 2.0 5.0 draw quad probe all rgba (9.0, 5.0, 1.0, 3.0) + +[require] +shader model >= 6.0 +shader int64 + +[pixel shader] +uniform int64_t2 a; + +float4 main() : SV_TARGET +{ + int64_t x = a.x; + int64_t y = a.y; + return float4(x + y, x - y, x * (y >> 4), x / y); +} + +[test] +uniform 0 int64_t2 5000000000 16000000000 +todo draw quad +probe all rgba (21.0e9, -11.0e9, 5.0e18, 0.0) 1 + +[pixel shader] +uniform int64_t2 a; + +float4 main() : SV_TARGET +{ + int64_t x = a.x; + int64_t y = a.y; + return float4(x % y, +x, -x, y / x); +} + +[test] +uniform 0 int64_t2 5000000000 16000000000 +todo draw quad +probe all rgba (5.0e9, 5.0e9, -5.0e9, 3.0) + +[pixel shader] +uniform int64_t2 a; + +float4 main() : SV_TARGET +{ + int64_t x = a.x; + int64_t y = a.y; + return float4(x / y, -x / y, x / -y, -x / -y); +} + +[test] +uniform 0 int64_t2 42000000000 5000000000 +todo draw quad +probe all rgba (8.0, -8.0, -8.0, 8.0) + +[pixel shader] +uniform int64_t2 a; + +float4 main() : SV_TARGET +{ + int64_t x = a.x; + int64_t y = a.y; + return float4(x % y, -x % y, x % -y, -x % -y); +} + +[test] +uniform 0 int64_t2 42000000000 5000000000 +todo draw quad +probe all rgba (2.0e9, -2.0e9, 2.0e9, -2.0e9) + +[pixel shader] +uniform int64_t2 a; + +float4 main() : SV_TARGET +{ + return float4(abs(a), 0, 0); +} + +[test] +uniform 0 int64_t2 5000000000 -7000000000 +todo draw quad +probe all rgba (5.0, 7.0, 0.0, 10.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- tests/hlsl/bitwise.shader_test | 85 ++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/hlsl/bitwise.shader_test b/tests/hlsl/bitwise.shader_test index 20a9db55f..095c9f77d 100644 --- a/tests/hlsl/bitwise.shader_test +++ b/tests/hlsl/bitwise.shader_test @@ -160,3 +160,88 @@ float4 main() : SV_TARGET [test] draw quad probe all rgba (0.0, 1.0, 1.0, 0.0) + + +[require] +shader model >= 6.0 +shader int64 + +[pixel shader] +int64_t2 a; +int2 s; + +float4 main() : sv_target +{ + int64_t x = a.x; + int64_t y = a.y; + int z = s.x; + int w = s.y; + + return float4(x >> y, x >> -y, x >> z, x >> w); +} + +[test] +uniform 0 int64_t2 9223372036854775807 -1 +uniform 4 int4 34 66 0 0 +todo draw quad +probe all rgba (0.0, 4.611686018e18, 536870912.0, 2.305843009e18) 1 +uniform 0 int64_t2 -1 -1 +uniform 4 int4 34 66 0 0 +todo draw quad +probe all rgba (-1.0, -1.0, -1.0, -1.0) 1 + +[pixel shader] +uint64_t2 a; +uint2 s; + +float4 main() : sv_target +{ + uint64_t x = a.x; + uint64_t y = a.y; + uint z = s.x; + uint w = s.y; + + return float4(x >> y, x >> -y, x >> z, x >> w); +} + +[test] +uniform 0 uint64_t2 0xffffffffffffffff 1 +uniform 4 uint4 34 66 0 0 +todo draw quad +probe all rgba (9.223372036e18, 1.0, 1073741823.0, 4.611686018e18) 1 + +[pixel shader] +uint64_t2 a; +uint2 s; + +float4 main() : sv_target +{ + uint64_t x = a.x; + uint64_t y = a.y; + uint z = s.x; + uint w = s.y; + + return float4(x << y, x << -y, x << z, x << w); +} + +[test] +uniform 0 uint64_t2 0x83 1 +uniform 4 uint4 34 66 0 0 +todo draw quad +probe all rgba (262.0, 9.223372036e18, 2250562863104.0, 524.0) 1 + +[pixel shader] +uint64_t2 a; + +float4 main() : sv_target +{ + uint64_t x = a.x; + uint64_t y = a.y; + + return float4(x ^ y, x & y, x | y, ~x); +} + +[test] +uniform 0 uint64_t2 0x300000000 0x500000000 +todo draw quad +probe all rgba (25769803776.0, 4294967296.0, 30064771072.0, 1.844674404e19) 1 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- include/vkd3d_shader.h | 3 +++ libs/vkd3d-shader/spirv.c | 32 +++++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 66a7de9b5..fa99849b7 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -376,6 +376,7 @@ enum vkd3d_shader_parameter_data_type { VKD3D_SHADER_PARAMETER_DATA_TYPE_UNKNOWN, VKD3D_SHADER_PARAMETER_DATA_TYPE_UINT32, + VKD3D_SHADER_PARAMETER_DATA_TYPE_BOOL, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_PARAMETER_DATA_TYPE), }; @@ -384,6 +385,7 @@ enum vkd3d_shader_parameter_name { VKD3D_SHADER_PARAMETER_NAME_UNKNOWN, VKD3D_SHADER_PARAMETER_NAME_RASTERIZER_SAMPLE_COUNT, + VKD3D_SHADER_PARAMETER_NAME_CAPABILITY_INT64, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_PARAMETER_NAME), }; @@ -393,6 +395,7 @@ struct vkd3d_shader_parameter_immediate_constant union { uint32_t u32; + bool b; } u; }; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 74e6d2063..46e961774 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2342,6 +2342,7 @@ struct spirv_compiler uint32_t *descriptor_offset_ids; struct vkd3d_push_constant_buffer_binding *push_constants; const struct vkd3d_shader_spirv_target_info *spirv_target_info; + bool capability_int64; bool main_block_open; bool after_declarations_section; @@ -5440,9 +5441,16 @@ static void spirv_compiler_emit_dcl_global_flags(struct spirv_compiler *compiler if (flags & VKD3DSGF_ENABLE_INT64) { - FIXME("Unsupported 64-bit integer ops.\n"); - spirv_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INT64_UNSUPPORTED, - "Support for 64-bit integers is not implemented."); + if (!compiler->capability_int64) + { + WARN("Unsupported 64-bit integer ops.\n"); + spirv_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INT64_UNSUPPORTED, + "The target environment does not support 64-bit integers."); + } + else + { + vkd3d_spirv_enable_capability(&compiler->spirv_builder, SpvCapabilityInt64); + } flags &= ~VKD3DSGF_ENABLE_INT64; } @@ -7087,6 +7095,13 @@ static void spirv_compiler_emit_int_div(struct spirv_compiler *compiler, div_op = instruction->handler_idx == VKD3DSIH_IDIV ? SpvOpSDiv : SpvOpUDiv; mod_op = instruction->handler_idx == VKD3DSIH_IDIV ? SpvOpSRem : SpvOpUMod; + if (dst[0].reg.data_type == VKD3D_DATA_UINT64 || dst[1].reg.data_type == VKD3D_DATA_UINT64) + { + FIXME("Unsupported 64-bit result.\n"); + spirv_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INT64_UNSUPPORTED, + "Bool cast to 64-bit integer is not supported."); + } + if (dst[0].reg.type != VKD3DSPR_NULL) { component_count = vkd3d_write_mask_component_count(dst[0].write_mask); @@ -9795,6 +9810,7 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; struct vkd3d_shader_desc *shader_desc = &parser->shader_desc; struct vkd3d_shader_instruction_array instructions; + const struct vkd3d_shader_parameter *parameter; enum vkd3d_result result = VKD3D_OK; unsigned int i; @@ -9825,6 +9841,16 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, compiler->input_control_point_count = shader_desc->input_control_point_count; compiler->output_control_point_count = shader_desc->output_control_point_count; + if ((parameter = spirv_compiler_get_shader_parameter(compiler, VKD3D_SHADER_PARAMETER_NAME_CAPABILITY_INT64))) + { + if (parameter->data_type != VKD3D_SHADER_PARAMETER_DATA_TYPE_BOOL) + { + ERR("Invalid data type %#x for parameter CAPABILITY_INT64.\n", parameter->data_type); + return VKD3D_ERROR_INVALID_ARGUMENT; + } + compiler->capability_int64 = parameter->u.immediate_constant.u.b; + } + if (compiler->shader_type != VKD3D_SHADER_TYPE_HULL) spirv_compiler_emit_shader_signature_outputs(compiler); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489
Giovanni Mascellani (@giomasce) commented about tests/shader_runner.c:
runner->compile_options |= options[i].option; } } + else if (match_string(line, "shader int64", &line))
@etang-cw proposed a more flexible syntax for this kind of needs, which I would be more inclined to adopt, in !416 (specifically https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/416/diffs?commit_id=3e...). That's probably something on which @zfigura and @fcasas might have something to say. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53503
Giovanni Mascellani (@giomasce) commented about tests/hlsl/arithmetic-float-uniform.shader_test:
uniform 0 float4 1.0 0.0 0.0 0.0 draw quad probe all rgba (1e99, 1e99, 1e99, 1e99) + +[require] +shader model >= 5.0 + +[pixel shader todo] +uniform double2 a; + +float4 main() : SV_TARGET +{ + double x = a.x; + double y = a.y; + return float4(x + y, x - y, x * y, x / y); This is fine for me, but here and below `asuint()` with a `uint4` return type could be used to return an actual double. I don't know if SM6 has something similar for 64 bit integers, but it can be emulated pretty easily with bitshifts anyway.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53505
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
{ VKD3D_SHADER_PARAMETER_NAME_UNKNOWN, VKD3D_SHADER_PARAMETER_NAME_RASTERIZER_SAMPLE_COUNT, + VKD3D_SHADER_PARAMETER_NAME_CAPABILITY_INT64, It's not completely obvious to me that a parameter is what is appropriate here (a parameter would, for instance, allow that data item to be a specialization constant, which does not make any sense here). Even if this is not technically a SPIR-V extension, it seems that the `extensions` array is more appropriate for defining which features are allowed by the execution environment. It's not even clear to me whether this is something that should be explicitly enabled: all the other SPIR-V capabilities do not require explicit user confirmation for vkd3d-shader to use them, I can't see why int64 is different. And, after all, if a shader requires it there is little that can be done: we either enable it or fail; so we could directly assume it's available and generate a shader, and the execution environment will then decide whether that shader is usable or not (though, to be honest, the same could be told of `EXT_DESCRIPTOR_INDEXING` and `EXT_STENCIL_EXPORT`). Accepting an input parameter mo stly seems to make sense when we can generate different code depending on whether some feature is available or not (as for `EXT_DEMOTE_TO_HELPER_INVOCATION`), which could even theoretically be the case for int64, but I don't think there is currently any intention to provide an alternative int64 implementation on top of int32.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53506
And, after all, if a shader requires it there is little that can be done: we either enable it or fail; so we could directly assume it's available and generate a shader, and the execution environment will then decide whether that shader is usable or not
One problem with this approach is Vulkan pipeline creation functions do not return errors; if they reject a SPIR-V binary they assert, so all the user sees is a crash. I'm not sure what the drivers will do if a binary uses 64-bit ints and they are not supported by the hardware, but crashing is one possibility. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53519
On Thu Nov 23 13:42:47 2023 +0000, Conor McCarthy wrote:
And, after all, if a shader requires it there is little that can be done: we either enable it or fail; so we could directly assume it's available and generate a shader, and the execution environment will then decide whether that shader is usable or not One problem with this approach is Vulkan pipeline creation functions do not return errors; if they reject a SPIR-V binary they assert, so all the user sees is a crash. I'm not sure what the drivers will do if a binary uses 64-bit ints and they are not supported by the hardware, but crashing is one possibility. Yeah, that's a sensible concern. At any rate, my feeling is that despite the name `extensions` is better suited for passing this kind of information.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53520
On Thu Nov 23 13:13:22 2023 +0000, Giovanni Mascellani wrote:
This is fine for me, but here and below `asuint()` with a `uint4` return type could be used to return an actual double. I don't know if SM6 has something similar for 64 bit integers, but it can be emulated pretty easily with bitshifts anyway. It has a SplitDouble intrinsic, which I'll probably upstream soon after the release. In shader runner we would need a way to check double value readbacks or at least uints. Maybe we'll need that eventually but I think it's not justified for this.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53522
On Thu Nov 23 13:47:36 2023 +0000, Conor McCarthy wrote:
It has a SplitDouble intrinsic, which I'll probably upstream soon after the release. In shader runner we would need a way to check double value readbacks or at least uints. Maybe we'll need that eventually but I think it's not justified for this. Yeah, I'd say that it makes sense to be able to check double readbacks anyway. I agree it's not required for this.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53523
On Thu Nov 23 13:13:21 2023 +0000, Giovanni Mascellani wrote:
@etang-cw proposed a more flexible syntax for this kind of needs, which I would be more inclined to adopt, in !416 (specifically https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/416/diffs?commit_id=3e...). That's probably something on which @zfigura and @fcasas might have something to say. It will be awhile before this goes upstream so I'll see what develops there.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53524
On Thu Nov 23 13:49:53 2023 +0000, Conor McCarthy wrote:
It will be awhile before this goes upstream so I'll see what develops there. I'm inclined to prefer this patch, it's far simpler. We don't need a second level.
I'd even get rid of the word "shader"; it's rather redundant. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53578
On Thu Nov 23 18:09:10 2023 +0000, Zebediah Figura wrote:
I'm inclined to prefer this patch, it's far simpler. We don't need a second level. I'd even get rid of the word "shader"; it's rather redundant. I like of Evan's solution that it's similar to the `options:` syntax that we already have. And yes, it requires some boilerplate code the first time, but then every further option is just an array item.
However, it's not such a strong opinion. I can live with Zeb's proposal. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53645
I like of Evan's solution that it's similar to the `options:` syntax that we already have. And yes, it requires some boilerplate code the first time, but then every further option is just an array item.
However, it's not such a strong opinion. I can live with Zeb's proposal.
Yeah, though if I'd thought about it I would have said the same thing there (or possibly I thought about it but didn't want to bother bikeshedding.) -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_53727
On Thu Nov 23 13:47:07 2023 +0000, Giovanni Mascellani wrote:
Yeah, that's a sensible concern. At any rate, my feeling is that despite the name `extensions` is better suited for passing this kind of information. For features, not extensions, which match up with a d3d12 global flag, it may be better for vkd3d-shader to send these to the client as a set of flags in a new scan struct, so the client can determine if a particular shader is supported before attempting to include it in a pipeline build. This places responsibility on the client not to compile unsupported shaders, but that is true of graphics libraries generally.
In addition to int64, this would cover wave ops, native 16-bit, int64 atomics and probably others, and new additions would only require updating the flag enum, instead of passing yet another extension or parameter and reading it in the backend. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/489#note_54816
participants (4)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)
-
Zebediah Figura (＠zfigura)