[PATCH v3 0/10] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
I am still working on parsing the remaining features of the effects framework, such as the FX functions for the state block entries -- such as SetBlendState() -- and the "compile" and "Compileshader()" syntax. However, after adding the many tests included in 2/7 and reading the feedback from !708, I think that this first batch of patches are going in the right direction in terms of parsing the state blocks and how to represent them internally. As Nikolay mentioned in https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/708#note_64421 and https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/708#note_64444 there are many types of state blocks entries, which should be identified for writing the effect metadata. A part that may cause discussion on this series is that I kept the representation of `struct hlsl_state_block_entry` using a `hlsl_block` to keep it general enough to represent all these types of state block entries, thinking on later implementing a helper to identify which type of entry we are dealing with. Even though, as Nikolay pointed out, the instruction set of fx shaders is different, I still think that HLSL IR should be enough to represent the rhs of state blocks, and hopefully we won't need to pollute it too much (apart from the introduction of hlsl_ir_undeclared_load in 4/7 to avoid creating a new variable) if we find operations that are fx-specific, since I intend to represent calls to FX functions with the same `struct hlsl_state_block_entry`, given that they cannot be called in regular HLSL code. There are many validations that are applied on regular HLSL that still should be applied to state blocks, such as the use of valid swizzles and the correct use of operators. -- v3: vkd3d-shader/hlsl: Allow KW_PIXELSHADER and KW_VERTEXSHADER as stateblock lhs. vkd3d-shader/hlsl: Store state block on pass variables. vkd3d-shader/hlsl: Parse list of state blocks. vkd3d-shader/hlsl: Introduce hlsl_ir_stateblock_constant. vkd3d-shader/hlsl: Parse and store state blocks on variables. tests: Add tests for "compile" and CompileShader() syntax. tests: Add tests for fxgroup syntax. tests: Test function call syntax for state blocks. tests: Add more state block syntax tests. vkd3d-shader/hlsl: Also call dce before lowering deref paths. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 35 +++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index ff349ab49..2764201fb 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3797,6 +3797,22 @@ static void allocate_register_reservations(struct hlsl_ctx *ctx) } } +static void deref_mark_last_read(struct hlsl_deref *deref, unsigned int last_read) +{ + unsigned int i; + + if (hlsl_deref_is_lowered(deref)) + { + if (deref->rel_offset.node) + deref->rel_offset.node->last_read = last_read; + } + else + { + for (i = 0; i < deref->path_len; ++i) + deref->path[i].node->last_read = last_read; + } +} + /* Compute the earliest and latest liveness for each variable. In the case that * a variable is accessed inside of a loop, we promote its liveness to extend * to at least the range of the entire loop. We also do this for nodes, so that @@ -3825,8 +3841,7 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop if (!var->first_write) var->first_write = loop_first ? min(instr->index, loop_first) : instr->index; store->rhs.node->last_read = last_read; - if (store->lhs.rel_offset.node) - store->lhs.rel_offset.node->last_read = last_read; + deref_mark_last_read(&store->lhs, last_read); break; } case HLSL_IR_EXPR: @@ -3853,8 +3868,7 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop var = load->src.var; var->last_read = max(var->last_read, last_read); - if (load->src.rel_offset.node) - load->src.rel_offset.node->last_read = last_read; + deref_mark_last_read(&load->src, last_read); break; } case HLSL_IR_LOOP: @@ -3871,14 +3885,12 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop var = load->resource.var; var->last_read = max(var->last_read, last_read); - if (load->resource.rel_offset.node) - load->resource.rel_offset.node->last_read = last_read; + deref_mark_last_read(&load->resource, last_read); if ((var = load->sampler.var)) { var->last_read = max(var->last_read, last_read); - if (load->sampler.rel_offset.node) - load->sampler.rel_offset.node->last_read = last_read; + deref_mark_last_read(&load->sampler, last_read); } if (load->coords.node) @@ -3903,8 +3915,7 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop var = store->resource.var; var->last_read = max(var->last_read, last_read); - if (store->resource.rel_offset.node) - store->resource.rel_offset.node->last_read = last_read; + deref_mark_last_read(&store->resource, last_read); store->coords.node->last_read = last_read; store->value.node->last_read = last_read; break; @@ -5433,6 +5444,10 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry lower_ir(ctx, validate_nonconstant_vector_store_derefs, body); + do + compute_liveness(ctx, entry_func); + while (hlsl_transform_ir(ctx, dce, body, NULL)); + /* TODO: move forward, remove when no longer needed */ transform_derefs(ctx, replace_deref_path_with_offset, body); while (hlsl_transform_ir(ctx, hlsl_fold_constant_exprs, body, NULL)); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/hlsl/state-block-syntax.shader_test | 451 ++++++++++++++++++++++ 1 file changed, 451 insertions(+) diff --git a/tests/hlsl/state-block-syntax.shader_test b/tests/hlsl/state-block-syntax.shader_test index 1534fffd8..b1071c908 100644 --- a/tests/hlsl/state-block-syntax.shader_test +++ b/tests/hlsl/state-block-syntax.shader_test @@ -171,3 +171,454 @@ float4 main() : sv_target [test] todo(glsl) draw quad probe all rgba (0, 1, 0, 1) + + +% Arbitrary names are allowed in the lhs of state block entries. +[pixel shader] +sampler sam +{ + Foobar = 3; +}; + +float4 main() : sv_target { return 0; } + + +% Undefined identifiers are allowed in state blocks. +[pixel shader] +sampler sam +{ + Filter = arbitrary_identifier; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks can be empty +[pixel shader todo] +sampler sams[2] +{ + { + }, + { + } +}; + +float4 main() : sv_target { return 0; } + + +% Multiple state blocks for array variables, as a list, are a thing. +[pixel shader todo] +sampler sams[2] +{ + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + } +}; + +float4 main() : sv_target { return 0; } + + +% Multiple state blocks for multi-component variables, as a list, are a thing. +[pixel shader todo] +float2 val +{ + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + } +}; + +float4 main() : sv_target { return 0; } + + +% The number of state blocks in the state block list must match the number of components. +[pixel shader fail(sm<6)] +sampler sams[2] +{ + { + } +}; + +float4 main() : sv_target { return 0; } + +[pixel shader fail(sm<6)] +sampler sams[2][2] +{ + { + }, + { + } +}; + +float4 main() : sv_target { return 0; } + + +[pixel shader fail(sm<6)] +float2 arr[2] +{ + { + }, + { + } +}; + +float4 main() : sv_target { return 0; } + + +[pixel shader todo] +float3 arr[2] +{ + { + }, + { + }, + { + }, + { + }, + { + }, + { + } +}; + +float4 main() : sv_target { return 0; } + + +[pixel shader fail(sm<6)] +sampler sams[2] +{ + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + }, // trailing comma not allowed. +}; + +float4 main() : sv_target { return 0; } + + +% Multiple state blocks for multi-dimensional array variables are a thing. +[pixel shader todo] +sampler sams[2][2] +{ + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + } +}; + +float4 main() : sv_target { return 0; } + + +% State blocks cannot be nested further than one level, regardless of multi-dimensionality. +[pixel shader fail(sm<6)] +sampler sams[2][2] +{ + { + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + } + }, + { + { + Filter = ANISOTROPIC; + }, + { + Filter = ANISOTROPIC; + } + } +}; + +float4 main() : sv_target { return 0; } + + +% Variables of 1 component can still use a single state block without the need to put it inside a list. +[pixel shader] +sampler sams[1] +{ + Filter = ANISOTROPIC; +}; + +float4 main() : sv_target { return 0; } + +[pixel shader todo] +sampler sam +{ + { + Filter = ANISOTROPIC; + } +}; + +float4 main() : sv_target { return 0; } + + +% It is possible to declare an empty state block +[pixel shader] +float f +{ +}; + +float4 main() : sv_target { return 0; } + + +% State block entries may have indexes. +[pixel shader todo] +sampler sam +{ + dogs[3] = 5; +}; + +float4 main() : sv_target { return 0; } + + +% State block entry indexes can only be integers, not even constant expressions are allowed. +[pixel shader fail(sm<6)] +sampler sam +{ + dogs[3 + 4] = 10; +}; + +float4 main() : sv_target { return 0; } + + +% State block entry indexes can not be negative integers. +[pixel shader fail(sm<6)] +sampler sam +{ + dogs[-2] = 10; +}; + +float4 main() : sv_target { return 0; } + +[pixel shader fail(sm<6)] +static const int a = 5; + +sampler sam +{ + dogs[a] = 5; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks may have bracket initializers on the rhs. +[pixel shader todo] +sampler sam +{ + MaxAnisotropy = 3; + cat = {1, 2, {3, "string"}}; + dogs[3] = {1, {2, {4}}, 3, any_identifier}; +}; + +float4 main() : sv_target { return 0; } + + +% Even though using undefined identifiers is allowed, calls to undefined functions are not. +[pixel shader fail(sm<6)] +sampler sam +{ + cat = fun(); +}; + +float4 main() : sv_target { return 0; } + + +% PixelShader and VertexShader are valid identifiers for the lhs +[pixel shader todo] +sampler sam +{ + pixelShader = 20; + PixelShader = 25; + VertexShader = 30; + vertexshader = 35; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks are valid for numeric types. +[pixel shader] +float f +{ + MaxAnisotropy = 3; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks are valid for texture types. +[pixel shader] +Texture2D tex +{ + MaxAnisotropy = 3; +}; + +float4 main() : sv_target { return 0; } + + +% Same rules apply for technique passes +[pixel shader todo] +technique +{ + pass + { + cat = {1, 2, {3, "string"}}; + dogs[3] = {1, {2, {4}}, 3, any_identifier}; + } +} + +float4 main() : sv_target { return 0; } + + +% Multi-dimensional arrays on the lhs on state blocks are syntax errors. +[pixel shader fail(sm<6)] +sampler sam +{ + dogs[1][1] = 1; +}; + +float4 main() : sv_target { return 0; } + +[pixel shader fail(sm<6)] +technique +{ + pass + { + dogs[1][1] = 1; + } +} + +float4 main() : sv_target { return 0; } + + +% Test complex expression on the rhs, including function calls. +[pixel shader] +float4 addition(float4 a, float4 b) +{ + return a + b; +} + +sampler sam +{ + cat = addition(foo, bar) + p * q; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks are valid for DepthStencilState +[pixel shader todo] +DepthStencilState dss1 +{ + DepthEnable = false; + DepthWriteMask = Zero; + DepthFunc = Less; + random_field = 12; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks are valid for BlendState. +[pixel shader todo] +BlendState bs1 +{ + random_field = 1; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks are valid for VertexShader and PixelShader +[pixel shader] +PixelShader ps1 +{ + random_field = 1; +}; + +VertexShader vs1 +{ + random_field = 1; +}; + +float4 main() : sv_target { return 0; } + + +% State blocks are valid for RasterizerState +[pixel shader todo] +RasterizerState rs +{ + random_field = 1; +}; + +float4 main() : sv_target { return 0; } + + +% Undefined identifiers cannot be indexed. +[pixel shader fail(sm<6)] +float4 main() : sv_target { return 0; } + +DepthStencilState dss1 +{ + RandomField = foobar[2]; +}; + + +% Undefined identifiers can be swizzled with .x which proves that they are considered scalar +[pixel shader todo] +float4 main() : sv_target { return 0; } + +DepthStencilState dss1 +{ + RandomField = foobar.x; +}; + +[pixel shader fail(sm<6)] +float4 main() : sv_target { return 0; } + +DepthStencilState dss1 +{ + RandomField = foobar.y; +}; + + +% The type of previously defined variables is respected, but array indexes are not checked. +[pixel shader todo] +float4 arr[3]; + +float4 main() : sv_target { return 0; } + +DepthStencilState dss1 +{ + RandomField = arr[90]; +}; + + +% The type of previously defined variables is respected, and swizzles are checked. +[pixel shader fail(sm<6)] +float3 vec; + +float4 main() : sv_target { return 0; } + +DepthStencilState dss1 +{ + RandomField = vec.w; +}; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- Makefile.am | 1 + .../state-block-function-syntax.shader_test | 89 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 tests/hlsl/state-block-function-syntax.shader_test diff --git a/Makefile.am b/Makefile.am index a9ea14338..976589808 100644 --- a/Makefile.am +++ b/Makefile.am @@ -181,6 +181,7 @@ vkd3d_shader_tests = \ tests/hlsl/smoothstep.shader_test \ tests/hlsl/sqrt.shader_test \ tests/hlsl/srv-buffers.shader_test \ + tests/hlsl/state-block-function-syntax.shader_test \ tests/hlsl/state-block-syntax.shader_test \ tests/hlsl/static-initializer.shader_test \ tests/hlsl/step.shader_test \ diff --git a/tests/hlsl/state-block-function-syntax.shader_test b/tests/hlsl/state-block-function-syntax.shader_test new file mode 100644 index 000000000..e722c46c3 --- /dev/null +++ b/tests/hlsl/state-block-function-syntax.shader_test @@ -0,0 +1,89 @@ +% Unlike assignment syntax, only these names are allowed. +% The parameter count is also checked. +[pixel shader todo] +sampler sam +{ + SetBlendState(foo, bar, baz); // 3 parameters + SetDepthStencilState(foo, 2); // 2 parameters + SetRasterizerState(foo); // 1 parameter + SetVertexShader(vs); // 1 parameter + SetDomainShader(ds); // 1 paramter + SetHullShader(100); // 1 parameter + SetGeometryShader(foo + bar); // 1 parameter + SetPixelShader(ps1); // 1 parameter + SetComputeShader("random string"); // 1 parameter + OMSetRenderTargets(RTV0, RTV1, RTV2, RTV3, RTV4, RTV5, RTV6, RTV7, DSV); // 2 to 9 parameters +}; + +float4 main() : sv_target { return 0; } + + +[pixel shader fail(sm<6)] +sampler sam +{ + SetSomeotherState(); +}; + +float4 main() : sv_target { return 0; } + + +% Test complex expression on the arguments, including function calls. +[pixel shader todo] +float4 addition(float4 a, float4 b) +{ + return a + b; +} + +sampler sam +{ + SetBlendState(addition(foo, bar), p + q, p / q); +}; + +float4 main() : sv_target { return 0; } + + +% Test the same thing on technique passes +[pixel shader todo] +technique tech1 +{ + pass pass1 + { + SetBlendState(foo, bar, baz); + SetDepthStencilState(foo, 2); + SetRasterizerState(foo); + SetVertexShader(vs); + } +} + +float4 main() : sv_target { return 0; } + + +% It is not allowed to call the functions to set state blocks on the rhs using the assignment syntax +% for state groups or passes. +[pixel shader fail(sm<6)] +float4 main() : sv_target { return 0; } + +technique +{ + pass + { + cat = SetPixelShader(foobar); + } +} + + +% It not allowed to call the functions to set states outside state blocks or passes. +[pixel shader fail] +DepthStencilState dss1 +{ + DepthEnable = false; + DepthWriteMask = Zero; + DepthFunc = Less; +}; + +float4 main() : sv_target +{ + SetDepthStencilState(NULL, dss1); + return 0; +} + -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl/fxgroup-syntax.shader_test | 39 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 tests/hlsl/fxgroup-syntax.shader_test diff --git a/Makefile.am b/Makefile.am index 976589808..18b6667da 100644 --- a/Makefile.am +++ b/Makefile.am @@ -106,6 +106,7 @@ vkd3d_shader_tests = \ tests/hlsl/function-return.shader_test \ tests/hlsl/function.shader_test \ tests/hlsl/fwidth.shader_test \ + tests/hlsl/fxgroup-syntax.shader_test \ tests/hlsl/gather-offset.shader_test \ tests/hlsl/gather.shader_test \ tests/hlsl/getdimensions.shader_test \ diff --git a/tests/hlsl/fxgroup-syntax.shader_test b/tests/hlsl/fxgroup-syntax.shader_test new file mode 100644 index 000000000..e8e5fbf59 --- /dev/null +++ b/tests/hlsl/fxgroup-syntax.shader_test @@ -0,0 +1,39 @@ +% Test complex effect groups syntax +[pixel shader fail(sm>=6) todo] +fxgroup group1 +{ + technique10 + { + pass + { + TurboEncabulator = prefabulated + aluminite; + malleable = logarithmic - casing; + } + pass pass1 + { + spurving_bearings = pentametric_fan; + hydrocoptic = marzlevanes; + } + } + technique11 tech1 + { + pass + { + Lunar = Waneshaft; + } + } +} + +float4 main() : sv_target { return 0; } + + +% Effect groups cannot have a "technique" without version +[pixel shader fail] +fxgroup group1 +{ + technique + { + } +} + +float4 main() : sv_target { return 0; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl/effect-compile.shader_test | 142 ++++++++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 tests/hlsl/effect-compile.shader_test diff --git a/Makefile.am b/Makefile.am index 18b6667da..bdbf06018 100644 --- a/Makefile.am +++ b/Makefile.am @@ -91,6 +91,7 @@ vkd3d_shader_tests = \ tests/hlsl/distance.shader_test \ tests/hlsl/dot.shader_test \ tests/hlsl/duplicate-modifiers.shader_test \ + tests/hlsl/effect-compile.shader_test \ tests/hlsl/effect-shader-objects-fx_2.shader_test \ tests/hlsl/effect-shader-objects-fx_5.shader_test \ tests/hlsl/entry-point-semantics.shader_test \ diff --git a/tests/hlsl/effect-compile.shader_test b/tests/hlsl/effect-compile.shader_test new file mode 100644 index 000000000..d79aeec51 --- /dev/null +++ b/tests/hlsl/effect-compile.shader_test @@ -0,0 +1,142 @@ +% Test special "compile" keyword syntax to compile pixel and vertex shaders +[pixel shader todo] +float4 fun() : sv_target +{ + return 0; +} + +sampler sam +{ + cat = compile ps_2_0 fun(); +}; + +float4 main() : sv_target { return 0; } + + +[pixel shader todo] +float4 fun() : sv_target +{ + return 0; +} + +technique +{ + pass + { + cat = compile ps_2_0 fun(); + } +} + +float4 main() : sv_target { return 0; } + + +% Only uniform arguments are expected, even if undefined identifiers are used. +[pixel shader todo] +float4 fun(uniform float4 a, float4 b, uniform float4 c) : sv_target +{ + return a + b + c; +} + +technique +{ + pass + { + cat = compile ps_2_0 fun(foobar, foobar); // Notice 2 arguments, not 3 + } +} + +float4 main() : sv_target { return 0; } + + +[pixel shader fail(sm<6)] +float4 fun(uniform float4 a, float4 b, uniform float4 c) : sv_target +{ + return a + b + c; +} + +technique +{ + pass + { + // passing 3 arguments here is not valid because the function has only 2 uniforms. + cat = compile ps_2_0 fun(1, 2, 3); + } +} + +float4 main() : sv_target { return 0; } + + +% Test the CompileShader() syntax. +[pixel shader todo fail(sm>=6)] +float arg1, arg2; + +float4 main_vertex(uniform float a) : sv_position { return a; } + +float4 main(uniform float a) : sv_target { return a; } + // ^ dxc expects a semantic here. + +PixelShader ps1 = CompileShader(ps_2_0, main(arg2)); +VertexShader vs1 = CompileShader(vs_2_0, main_vertex(arg1)); + +technique10 tech1 +{ + pass pass1 + { + SetVertexShader(vs1); + SetPixelShader(ps1); + } +} + + +% Undefined identifiers are not allowed if CompileShader() is used outside a state block. +[pixel shader fail] +float4 main(uniform float a) : sv_target { return a; } + +PixelShader ps1 = CompileShader(ps_2_0, main(foobar)); + + +% But undefined identifiers are allowed if inside a state block. +[pixel shader todo fail(sm>=6)] +float4 main_vertex(uniform float a) : sv_position { return a; } + +float4 main(uniform float a) : sv_target { return a; } + // ^ dxc expects a semantic here. + +technique tech1 +{ + pass pass1 + { + SetVertexShader(CompileShader(vs_2_0, main_vertex(foo))); + SetPixelShader(CompileShader(ps_2_0, main(bar))); + } +} + + +% Again only uniform parameters are expected +[pixel shader fail] +float aa, bb; + +float4 main(uniform float a, float b) : sv_target { return a; } + +PixelShader ps1 = CompileShader(ps_2_0, main(aa, bb)); + + +% Only a set of target profiles are allowed +[pixel shader fail(sm<6)] +float4 main() : sv_target { return 0; } + +PixelShader ps1 = CompileShader(ps_6_0, main()); + +[pixel shader fail(sm<6)] +float4 main() : sv_target { return 0; } + +PixelShader ps1 = CompileShader(fs_2_0, main()); + + +% Shaders cannot be passed around to another variable: "Initializer must be literal expressions.". +[pixel shader fail(sm<6)] +float4 main() : sv_target { return 0; } + +PixelShader ps1 = CompileShader(ps_2_0, main()); +PixelShader ps2 = ps1; + -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 39 ++++++++++++ libs/vkd3d-shader/hlsl.h | 37 +++++++++++ libs/vkd3d-shader/hlsl.y | 77 ++++++++++++++++++++--- tests/hlsl/state-block-syntax.shader_test | 2 +- 4 files changed, 147 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index a82334e58..641054631 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -134,6 +134,26 @@ struct hlsl_ir_var *hlsl_get_var(struct hlsl_scope *scope, const char *name) return hlsl_get_var(scope->upper, name); } +static void free_state_block_entry(struct hlsl_state_block_entry *entry) +{ + vkd3d_free(entry->name); + vkd3d_free(entry->args); + hlsl_block_cleanup(entry->instrs); + vkd3d_free(entry->instrs); + vkd3d_free(entry); +} + +void hlsl_free_state_block(struct hlsl_state_block *state_block) +{ + unsigned int k; + + assert(state_block); + for (k = 0; k < state_block->count; ++k) + free_state_block_entry(state_block->entries[k]); + vkd3d_free(state_block->entries); + vkd3d_free(state_block); +} + void hlsl_free_var(struct hlsl_ir_var *decl) { unsigned int k; @@ -142,6 +162,11 @@ void hlsl_free_var(struct hlsl_ir_var *decl) hlsl_cleanup_semantic(&decl->semantic); for (k = 0; k <= HLSL_REGSET_LAST_OBJECT; ++k) vkd3d_free((void *)decl->objects_usage[k]); + if (decl->state_block) + { + hlsl_free_state_block(decl->state_block); + decl->state_block = NULL; + } vkd3d_free(decl); } @@ -3633,6 +3658,20 @@ static void hlsl_ctx_cleanup(struct hlsl_ctx *ctx) rb_destroy(&ctx->functions, free_function_rb, NULL); + /* State blocks must be free before the variables, because they contain instructions that may + * refer to them. */ + LIST_FOR_EACH_ENTRY_SAFE(scope, next_scope, &ctx->scopes, struct hlsl_scope, entry) + { + LIST_FOR_EACH_ENTRY_SAFE(var, next_var, &scope->vars, struct hlsl_ir_var, scope_entry) + { + if (var->state_block) + { + hlsl_free_state_block(var->state_block); + var->state_block = NULL; + } + } + } + LIST_FOR_EACH_ENTRY_SAFE(scope, next_scope, &ctx->scopes, struct hlsl_scope, entry) { LIST_FOR_EACH_ENTRY_SAFE(var, next_var, &scope->vars, struct hlsl_ir_var, scope_entry) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 93ec67d0a..1fce5d12d 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -422,6 +422,10 @@ struct hlsl_ir_var /* Scope that contains annotations for this variable. */ struct hlsl_scope *annotations; + /* The state block on the variable's declaration, if any. + * These are only really used for effect profiles. */ + struct hlsl_state_block *state_block; + /* Indexes of the IR instructions where the variable is first written and last read (liveness * range). The IR instructions are numerated starting from 2, because 0 means unused, and 1 * means function entry. */ @@ -457,6 +461,38 @@ struct hlsl_ir_var uint32_t is_separated_resource : 1; }; +/* This struct is used to represent assignments in state block entries: + * name = {args[0], args[1], ...}; + * - or - + * name = args[0] + * - or - + * name[lhs_index] = args[0] + * - or - + * name[lhs_index] = {args[0], args[1], ...}; + */ +struct hlsl_state_block_entry +{ + /* For assignments, the name in the lhs. */ + char *name; + + /* Whether the lhs in the assignment is indexed and, in that case, its index. */ + bool lhs_has_index; + unsigned int lhs_index; + + /* Instructions present in the rhs. */ + struct hlsl_block *instrs; + + /* For assignments, arguments of the rhs initializer. */ + struct hlsl_ir_node **args; + unsigned int args_count; +}; + +struct hlsl_state_block +{ + struct hlsl_state_block_entry **entries; + size_t count, capacity; +}; + /* Sized array of variables representing a function's parameters. */ struct hlsl_func_parameters { @@ -1206,6 +1242,7 @@ void hlsl_replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new); void hlsl_free_attribute(struct hlsl_attribute *attr); void hlsl_free_instr(struct hlsl_ir_node *node); void hlsl_free_instr_list(struct list *list); +void hlsl_free_state_block(struct hlsl_state_block *state_block); void hlsl_free_type(struct hlsl_type *type); void hlsl_free_var(struct hlsl_ir_var *decl); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ec8b3d22a..e3dd07bcb 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -77,6 +77,8 @@ struct parse_variable_def struct hlsl_type *basic_type; uint32_t modifiers; struct vkd3d_shader_location modifiers_loc; + + struct hlsl_state_block *state_block; }; struct parse_function @@ -114,6 +116,12 @@ struct parse_attribute_list const struct hlsl_attribute **attrs; }; +struct state_block_index +{ + bool has_index; + unsigned int index; +}; + } %code provides @@ -925,6 +933,8 @@ static void free_parse_variable_def(struct parse_variable_def *v) vkd3d_free(v->arrays.sizes); vkd3d_free(v->name); hlsl_cleanup_semantic(&v->semantic); + if (v->state_block) + hlsl_free_state_block(v->state_block); vkd3d_free(v); } @@ -2349,6 +2359,9 @@ static struct hlsl_block *initialize_vars(struct hlsl_ctx *ctx, struct list *var free_parse_variable_def(v); continue; } + + var->state_block = v->state_block; + v->state_block = NULL; type = var->data_type; if (v->initializer.args_count) @@ -5282,6 +5295,16 @@ static void validate_uav_type(struct hlsl_ctx *ctx, enum hlsl_sampler_dim dim, hlsl_release_string_buffer(ctx, string); } +static bool state_block_add_entry(struct hlsl_state_block *state_block, struct hlsl_state_block_entry *entry) +{ + if (!vkd3d_array_reserve((void **)&state_block->entries, &state_block->capacity, state_block->count + 1, + sizeof(*state_block->entries))) + return false; + + state_block->entries[state_block->count++] = entry; + return true; +} + } %locations @@ -5322,6 +5345,8 @@ static void validate_uav_type(struct hlsl_ctx *ctx, enum hlsl_sampler_dim dim, struct parse_attribute_list attr_list; struct hlsl_ir_switch_case *switch_case; struct hlsl_scope *scope; + struct hlsl_state_block *state_block; + struct state_block_index state_block_index; } %token KW_BLENDSTATE @@ -5522,6 +5547,7 @@ static void validate_uav_type(struct hlsl_ctx *ctx, enum hlsl_sampler_dim dim, %type <name> var_identifier %type <name> name_opt + %type <parameter> parameter %type <parameters> param_list @@ -5534,6 +5560,10 @@ static void validate_uav_type(struct hlsl_ctx *ctx, enum hlsl_sampler_dim dim, %type <semantic> semantic +%type <state_block> state_block + +%type <state_block_index> state_block_index_opt + %type <switch_case> switch_case %type <type> field_type @@ -6678,22 +6708,54 @@ variable_decl: $$->reg_reservation = $3.reg_reservation; } -state: - any_identifier '=' expr ';' +state_block_start: + %empty { - vkd3d_free($1); - destroy_block($3); + ctx->in_state_block = 1; } -state_block_start: +state_block_index_opt: %empty { - ctx->in_state_block = 1; + $$.has_index = false; + $$.index = 0; } + | '[' C_INTEGER ']' + { + if ($2 < 0) + { + hlsl_error(ctx, &@2, VKD3D_SHADER_ERROR_HLSL_INVALID_INDEX, + "State block array index is not a positive integer constant."); + YYABORT; + } + $$.has_index = true; + $$.index = $2; + } state_block: %empty - | state_block state + { + if (!($$ = hlsl_alloc(ctx, sizeof(*$$)))) + YYABORT; + } + | state_block any_identifier state_block_index_opt '=' complex_initializer ';' + { + struct hlsl_state_block_entry *entry; + + if (!(entry = hlsl_alloc(ctx, sizeof(*entry)))) + YYABORT; + + entry->name = $2; + entry->lhs_has_index = $3.has_index; + entry->lhs_index = $3.index; + + entry->instrs = $5.instrs; + entry->args = $5.args; + entry->args_count = $5.args_count; + + $$ = $1; + state_block_add_entry($$, entry); + } variable_def: variable_decl @@ -6705,6 +6767,7 @@ variable_def: | variable_decl '{' state_block_start state_block '}' { $$ = $1; + $$->state_block = $4; ctx->in_state_block = 0; } diff --git a/tests/hlsl/state-block-syntax.shader_test b/tests/hlsl/state-block-syntax.shader_test index b1071c908..7521516b9 100644 --- a/tests/hlsl/state-block-syntax.shader_test +++ b/tests/hlsl/state-block-syntax.shader_test @@ -380,7 +380,7 @@ float4 main() : sv_target { return 0; } % State block entries may have indexes. -[pixel shader todo] +[pixel shader] sampler sam { dogs[3] = 5; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 50 ++++++++++++++++++++++++++++++++ libs/vkd3d-shader/hlsl.h | 17 +++++++++++ libs/vkd3d-shader/hlsl.y | 13 ++++----- libs/vkd3d-shader/hlsl_codegen.c | 6 ++++ 4 files changed, 79 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 641054631..f3a3e67be 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1585,6 +1585,27 @@ struct hlsl_ir_node *hlsl_new_swizzle(struct hlsl_ctx *ctx, uint32_t s, unsigned return &swizzle->node; } +struct hlsl_ir_node *hlsl_new_stateblock_constant(struct hlsl_ctx *ctx, const char *name, + struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_stateblock_constant *constant; + struct hlsl_type *type = hlsl_get_scalar_type(ctx, HLSL_TYPE_INT); + + if (!(constant = hlsl_alloc(ctx, sizeof(*constant)))) + return NULL; + + init_node(&constant->node, HLSL_IR_STATEBLOCK_CONSTANT, type, loc); + + if (!(constant->name = hlsl_alloc(ctx, strlen(name) + 1))) + { + vkd3d_free(constant); + return NULL; + } + strcpy(constant->name, name); + + return &constant->node; +} + bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index) { struct hlsl_type *type = index->val.node->data_type; @@ -1905,6 +1926,12 @@ static struct hlsl_ir_node *clone_index(struct hlsl_ctx *ctx, struct clone_instr return dst; } +static struct hlsl_ir_node *clone_stateblock_constant(struct hlsl_ctx *ctx, + struct clone_instr_map *map, struct hlsl_ir_stateblock_constant *constant) +{ + return hlsl_new_stateblock_constant(ctx, constant->name, &constant->node.loc); +} + void hlsl_free_ir_switch_case(struct hlsl_ir_switch_case *c) { hlsl_block_cleanup(&c->body); @@ -2000,6 +2027,9 @@ static struct hlsl_ir_node *clone_instr(struct hlsl_ctx *ctx, case HLSL_IR_SWIZZLE: return clone_swizzle(ctx, map, hlsl_ir_swizzle(instr)); + + case HLSL_IR_STATEBLOCK_CONSTANT: + return clone_stateblock_constant(ctx, map, hlsl_ir_stateblock_constant(instr)); } vkd3d_unreachable(); @@ -2830,6 +2860,12 @@ static void dump_ir_index(struct vkd3d_string_buffer *buffer, const struct hlsl_ vkd3d_string_buffer_printf(buffer, "]"); } +static void dump_ir_stateblock_constant(struct vkd3d_string_buffer *buffer, + const struct hlsl_ir_stateblock_constant *constant) +{ + vkd3d_string_buffer_printf(buffer, "%s", constant->name); +} + static void dump_ir_switch(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, const struct hlsl_ir_switch *s) { struct hlsl_ir_switch_case *c; @@ -2918,6 +2954,10 @@ static void dump_instr(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, case HLSL_IR_SWIZZLE: dump_ir_swizzle(buffer, hlsl_ir_swizzle(instr)); break; + + case HLSL_IR_STATEBLOCK_CONSTANT: + dump_ir_stateblock_constant(buffer, hlsl_ir_stateblock_constant(instr)); + break; } } @@ -3090,6 +3130,12 @@ static void free_ir_index(struct hlsl_ir_index *index) vkd3d_free(index); } +static void free_ir_stateblock_constant(struct hlsl_ir_stateblock_constant *constant) +{ + vkd3d_free(constant->name); + vkd3d_free(constant); +} + void hlsl_free_instr(struct hlsl_ir_node *node) { assert(list_empty(&node->uses)); @@ -3147,6 +3193,10 @@ void hlsl_free_instr(struct hlsl_ir_node *node) case HLSL_IR_SWITCH: free_ir_switch(hlsl_ir_switch(node)); break; + + case HLSL_IR_STATEBLOCK_CONSTANT: + free_ir_stateblock_constant(hlsl_ir_stateblock_constant(node)); + break; } } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 1fce5d12d..72f485c84 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -298,6 +298,7 @@ enum hlsl_ir_node_type HLSL_IR_STORE, HLSL_IR_SWIZZLE, HLSL_IR_SWITCH, + HLSL_IR_STATEBLOCK_CONSTANT, }; /* Common data for every type of IR instruction node. */ @@ -789,6 +790,14 @@ struct hlsl_ir_constant struct hlsl_reg reg; }; +/* Stateblock constants are undeclared values found on state blocks or technique passes descriptions, + * that do not concern regular pixel, vertex, or compute shaders, except for parsing. */ +struct hlsl_ir_stateblock_constant +{ + struct hlsl_ir_node node; + char *name; +}; + struct hlsl_scope { /* Item entry for hlsl_ctx.scopes. */ @@ -1050,6 +1059,12 @@ static inline struct hlsl_ir_switch *hlsl_ir_switch(const struct hlsl_ir_node *n return CONTAINING_RECORD(node, struct hlsl_ir_switch, node); } +static inline struct hlsl_ir_stateblock_constant *hlsl_ir_stateblock_constant(const struct hlsl_ir_node *node) +{ + assert(node->type == HLSL_IR_STATEBLOCK_CONSTANT); + return CONTAINING_RECORD(node, struct hlsl_ir_stateblock_constant, node); +} + static inline void hlsl_block_init(struct hlsl_block *block) { list_init(&block->instrs); @@ -1323,6 +1338,8 @@ struct hlsl_type *hlsl_new_struct_type(struct hlsl_ctx *ctx, const char *name, struct hlsl_struct_field *fields, size_t field_count); struct hlsl_ir_node *hlsl_new_swizzle(struct hlsl_ctx *ctx, uint32_t s, unsigned int components, struct hlsl_ir_node *val, const struct vkd3d_shader_location *loc); +struct hlsl_ir_node *hlsl_new_stateblock_constant(struct hlsl_ctx *ctx, const char *name, + struct vkd3d_shader_location *loc); struct hlsl_ir_var *hlsl_new_synthetic_var(struct hlsl_ctx *ctx, const char *template, struct hlsl_type *type, const struct vkd3d_shader_location *loc); struct hlsl_ir_var *hlsl_new_synthetic_var_named(struct hlsl_ctx *ctx, const char *name, diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index e3dd07bcb..5299a9e1a 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1303,6 +1303,7 @@ static unsigned int evaluate_static_expression_as_uint(struct hlsl_ctx *ctx, str case HLSL_IR_RESOURCE_STORE: case HLSL_IR_STORE: case HLSL_IR_SWITCH: + case HLSL_IR_STATEBLOCK_CONSTANT: hlsl_error(ctx, &node->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, "Expected literal expression."); } @@ -7390,15 +7391,13 @@ primary_expr: { if (ctx->in_state_block) { - struct hlsl_ir_load *load; - struct hlsl_ir_var *var; + struct hlsl_ir_node *constant; - if (!(var = hlsl_new_synthetic_var(ctx, "state_block_expr", - hlsl_get_scalar_type(ctx, HLSL_TYPE_INT), &@1))) + if (!(constant = hlsl_new_stateblock_constant(ctx, $1, &@1))) YYABORT; - if (!(load = hlsl_new_var_load(ctx, var, &@1))) - YYABORT; - if (!($$ = make_block(ctx, &load->node))) + vkd3d_free($1); + + if (!($$ = make_block(ctx, constant))) YYABORT; } else diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 2764201fb..32e199b1f 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3706,6 +3706,9 @@ static bool dce(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) case HLSL_IR_RESOURCE_STORE: case HLSL_IR_SWITCH: break; + case HLSL_IR_STATEBLOCK_CONSTANT: + /* Stateblock constants should not appear in the shader program. */ + vkd3d_unreachable(); } return false; @@ -3832,6 +3835,9 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop case HLSL_IR_CALL: /* We should have inlined all calls before computing liveness. */ vkd3d_unreachable(); + case HLSL_IR_STATEBLOCK_CONSTANT: + /* Stateblock constants should not appear in the shader program. */ + vkd3d_unreachable(); case HLSL_IR_STORE: { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 23 ++++---- libs/vkd3d-shader/hlsl.h | 7 ++- libs/vkd3d-shader/hlsl.y | 66 ++++++++++++++++++++--- tests/hlsl/state-block-syntax.shader_test | 12 ++--- 4 files changed, 83 insertions(+), 25 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index f3a3e67be..4307a29cc 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -156,17 +156,17 @@ void hlsl_free_state_block(struct hlsl_state_block *state_block) void hlsl_free_var(struct hlsl_ir_var *decl) { - unsigned int k; + unsigned int k, i; vkd3d_free((void *)decl->name); hlsl_cleanup_semantic(&decl->semantic); for (k = 0; k <= HLSL_REGSET_LAST_OBJECT; ++k) vkd3d_free((void *)decl->objects_usage[k]); - if (decl->state_block) - { - hlsl_free_state_block(decl->state_block); - decl->state_block = NULL; - } + + for (i = 0; i < decl->state_block_count; ++i) + hlsl_free_state_block(decl->state_blocks[i]); + vkd3d_free(decl->state_blocks); + vkd3d_free(decl); } @@ -3714,11 +3714,12 @@ static void hlsl_ctx_cleanup(struct hlsl_ctx *ctx) { LIST_FOR_EACH_ENTRY_SAFE(var, next_var, &scope->vars, struct hlsl_ir_var, scope_entry) { - if (var->state_block) - { - hlsl_free_state_block(var->state_block); - var->state_block = NULL; - } + for (i = 0; i < var->state_block_count; ++i) + hlsl_free_state_block(var->state_blocks[i]); + vkd3d_free(var->state_blocks); + var->state_blocks = NULL; + var->state_block_count = 0; + var->state_block_capacity = 0; } } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 72f485c84..661012eee 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -423,9 +423,12 @@ struct hlsl_ir_var /* Scope that contains annotations for this variable. */ struct hlsl_scope *annotations; - /* The state block on the variable's declaration, if any. + /* A dynamic array containing the state block on the variable's declaration, if any. + * An array variable may contain multiple state blocks. * These are only really used for effect profiles. */ - struct hlsl_state_block *state_block; + struct hlsl_state_block **state_blocks; + unsigned int state_block_count; + size_t state_block_capacity; /* Indexes of the IR instructions where the variable is first written and last read (liveness * range). The IR instructions are numerated starting from 2, because 0 means unused, and 1 diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 5299a9e1a..eed9e32ea 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -78,7 +78,9 @@ struct parse_variable_def uint32_t modifiers; struct vkd3d_shader_location modifiers_loc; - struct hlsl_state_block *state_block; + struct hlsl_state_block **state_blocks; + unsigned int state_block_count; + size_t state_block_capacity; }; struct parse_function @@ -933,8 +935,7 @@ static void free_parse_variable_def(struct parse_variable_def *v) vkd3d_free(v->arrays.sizes); vkd3d_free(v->name); hlsl_cleanup_semantic(&v->semantic); - if (v->state_block) - hlsl_free_state_block(v->state_block); + assert(!v->state_blocks); vkd3d_free(v); } @@ -2361,10 +2362,24 @@ static struct hlsl_block *initialize_vars(struct hlsl_ctx *ctx, struct list *var continue; } - var->state_block = v->state_block; - v->state_block = NULL; type = var->data_type; + var->state_blocks = v->state_blocks; + var->state_block_count = v->state_block_count; + var->state_block_capacity = v->state_block_capacity; + v->state_block_count = 0; + v->state_block_capacity = 0; + v->state_blocks = NULL; + + if (var->state_blocks && hlsl_type_component_count(type) != var->state_block_count) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, + "Expected %u state blocks, but got %u.", + hlsl_type_component_count(type), var->state_block_count); + free_parse_variable_def(v); + continue; + } + if (v->initializer.args_count) { if (v->initializer.braces) @@ -5575,6 +5590,7 @@ static bool state_block_add_entry(struct hlsl_state_block *state_block, struct h %type <type> type_no_void %type <type> typedef_type +%type <variable_def> state_block_list %type <variable_def> type_spec %type <variable_def> variable_decl %type <variable_def> variable_def @@ -6758,6 +6774,27 @@ state_block: state_block_add_entry($$, entry); } +state_block_list: + '{' state_block '}' + { + if (!($$ = hlsl_alloc(ctx, sizeof(*$$)))) + YYABORT; + + if(!(vkd3d_array_reserve((void **)&$$->state_blocks, &$$->state_block_capacity, + $$->state_block_count + 1, sizeof(*$$->state_blocks)))) + YYABORT; + $$->state_blocks[$$->state_block_count++] = $2; + } + | state_block_list ',' '{' state_block '}' + { + $$ = $1; + + if(!(vkd3d_array_reserve((void **)&$$->state_blocks, &$$->state_block_capacity, + $$->state_block_count + 1, sizeof(*$$->state_blocks)))) + YYABORT; + $$->state_blocks[$$->state_block_count++] = $4; + } + variable_def: variable_decl | variable_decl '=' complex_initializer @@ -6768,8 +6805,25 @@ variable_def: | variable_decl '{' state_block_start state_block '}' { $$ = $1; - $$->state_block = $4; ctx->in_state_block = 0; + + if(!(vkd3d_array_reserve((void **)&$$->state_blocks, &$$->state_block_capacity, + $$->state_block_count + 1, sizeof(*$$->state_blocks)))) + YYABORT; + $$->state_blocks[$$->state_block_count++] = $4; + } + | variable_decl '{' state_block_start state_block_list '}' + { + $$ = $1; + ctx->in_state_block = 0; + + $$->state_blocks = $4->state_blocks; + $$->state_block_count = $4->state_block_count; + $$->state_block_capacity = $4->state_block_capacity; + $4->state_blocks = NULL; + $4->state_block_count = 0; + $4->state_block_capacity = 0; + free_parse_variable_def($4); } variable_def_typed: diff --git a/tests/hlsl/state-block-syntax.shader_test b/tests/hlsl/state-block-syntax.shader_test index 7521516b9..5c2d6f9b7 100644 --- a/tests/hlsl/state-block-syntax.shader_test +++ b/tests/hlsl/state-block-syntax.shader_test @@ -194,7 +194,7 @@ float4 main() : sv_target { return 0; } % State blocks can be empty -[pixel shader todo] +[pixel shader] sampler sams[2] { { @@ -207,7 +207,7 @@ float4 main() : sv_target { return 0; } % Multiple state blocks for array variables, as a list, are a thing. -[pixel shader todo] +[pixel shader] sampler sams[2] { { @@ -222,7 +222,7 @@ float4 main() : sv_target { return 0; } % Multiple state blocks for multi-component variables, as a list, are a thing. -[pixel shader todo] +[pixel shader] float2 val { { @@ -270,7 +270,7 @@ float2 arr[2] float4 main() : sv_target { return 0; } -[pixel shader todo] +[pixel shader] float3 arr[2] { { @@ -305,7 +305,7 @@ float4 main() : sv_target { return 0; } % Multiple state blocks for multi-dimensional array variables are a thing. -[pixel shader todo] +[pixel shader] sampler sams[2][2] { { @@ -359,7 +359,7 @@ sampler sams[1] float4 main() : sv_target { return 0; } -[pixel shader todo] +[pixel shader] sampler sam { { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl.y | 11 ++++++++--- tests/hlsl/fxgroup-syntax.shader_test | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 661012eee..7c275202e 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -425,6 +425,7 @@ struct hlsl_ir_var /* A dynamic array containing the state block on the variable's declaration, if any. * An array variable may contain multiple state blocks. + * A technique pass will always contain one. * These are only really used for effect profiles. */ struct hlsl_state_block **state_blocks; unsigned int state_block_count; diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index eed9e32ea..efb1ce631 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1126,7 +1126,7 @@ static bool add_func_parameter(struct hlsl_ctx *ctx, struct hlsl_func_parameters } static bool add_pass(struct hlsl_ctx *ctx, const char *name, struct hlsl_scope *annotations, - const struct vkd3d_shader_location *loc) + struct hlsl_state_block *state_block, const struct vkd3d_shader_location *loc) { struct hlsl_ir_var *var; struct hlsl_type *type; @@ -1136,6 +1136,11 @@ static bool add_pass(struct hlsl_ctx *ctx, const char *name, struct hlsl_scope * return false; var->annotations = annotations; + var->state_blocks = hlsl_alloc(ctx, sizeof(*var->state_blocks)); + var->state_blocks[0] = state_block; + var->state_block_count = 1; + var->state_block_capacity = 1; + if (!hlsl_add_var(ctx, var, false)) { struct hlsl_ir_var *old = hlsl_get_var(ctx->cur_scope, var->name); @@ -5621,9 +5626,9 @@ name_opt: | any_identifier pass: - KW_PASS name_opt annotations_opt '{' '}' + KW_PASS name_opt annotations_opt '{' state_block_start state_block '}' { - if (!add_pass(ctx, $2, $3, &@1)) + if (!add_pass(ctx, $2, $3, $6, &@1)) YYABORT; } diff --git a/tests/hlsl/fxgroup-syntax.shader_test b/tests/hlsl/fxgroup-syntax.shader_test index e8e5fbf59..aedbbfb11 100644 --- a/tests/hlsl/fxgroup-syntax.shader_test +++ b/tests/hlsl/fxgroup-syntax.shader_test @@ -1,5 +1,5 @@ % Test complex effect groups syntax -[pixel shader fail(sm>=6) todo] +[pixel shader fail(sm>=6)] fxgroup group1 { technique10 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 20 ++++++++++++++++++-- tests/hlsl/state-block-syntax.shader_test | 2 +- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index efb1ce631..6848ea609 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -5566,9 +5566,9 @@ static bool state_block_add_entry(struct hlsl_state_block *state_block, struct h %type <name> any_identifier %type <name> var_identifier +%type <name> stateblock_lhs_identifier %type <name> name_opt - %type <parameter> parameter %type <parameters> param_list @@ -6736,6 +6736,22 @@ state_block_start: ctx->in_state_block = 1; } +stateblock_lhs_identifier: + any_identifier + { + $$ = $1; + } + | KW_PIXELSHADER + { + if (!($$ = hlsl_strdup(ctx, "pixelshader"))) + YYABORT; + } + | KW_VERTEXSHADER + { + if (!($$ = hlsl_strdup(ctx, "vertexshader"))) + YYABORT; + } + state_block_index_opt: %empty { @@ -6760,7 +6776,7 @@ state_block: if (!($$ = hlsl_alloc(ctx, sizeof(*$$)))) YYABORT; } - | state_block any_identifier state_block_index_opt '=' complex_initializer ';' + | state_block stateblock_lhs_identifier state_block_index_opt '=' complex_initializer ';' { struct hlsl_state_block_entry *entry; diff --git a/tests/hlsl/state-block-syntax.shader_test b/tests/hlsl/state-block-syntax.shader_test index 5c2d6f9b7..fb0e73122 100644 --- a/tests/hlsl/state-block-syntax.shader_test +++ b/tests/hlsl/state-block-syntax.shader_test @@ -442,7 +442,7 @@ float4 main() : sv_target { return 0; } % PixelShader and VertexShader are valid identifiers for the lhs -[pixel shader todo] +[pixel shader] sampler sam { pixelShader = 20; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739
On Tue Mar 26 04:36:31 2024 +0000, Francisco Casas wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/739/diffs?diff_id=107171&start_sha=f81152537041ebb341d645ee531fefb6cdc61f48#b4b2f0df64f7fbb515484cb602382012595ddfb6_302_301) I changed it to `stateblock_constant`.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66084
On Tue Mar 26 04:36:30 2024 +0000, Francisco Casas wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/739/diffs?diff_id=107171&start_sha=f81152537041ebb341d645ee531fefb6cdc61f48#0b890a3b204382f3632b9aa8007a5efad9f29fcf_476_0) You are right. I just removed this test.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66085
On Tue Mar 26 04:36:28 2024 +0000, Francisco Casas wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/739/diffs?diff_id=107171&start_sha=f81152537041ebb341d645ee531fefb6cdc61f48#0b890a3b204382f3632b9aa8007a5efad9f29fcf_414_0) There is a test, if this is what you mean:
% It is not allowed to call the functions to set state blocks on the rhs using the assignment syntax
% for state groups or passes.
[pixel shader fail(sm<6)]
float4 main() : sv_target { return 0; }
technique
{
pass
{
cat = SetPixelShader(foobar);
}
}
But other types of function calls on the rhs are allowed. I added some more tests. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66086
On Tue Mar 26 04:36:26 2024 +0000, Francisco Casas wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/739/diffs?diff_id=107171&start_sha=f81152537041ebb341d645ee531fefb6cdc61f48#9155b9453b4ec8ea0b9b025dfb55c061bd931610_6733_6779) I added a test, good catch.
You could probably avoid the duplication with a separate "%empty | \[int\]" rule. Or alternatively use "arrays" and add an explicit check for count \> 1.
I added the state_block_index_opt rule. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66087
On Tue Mar 26 04:36:32 2024 +0000, Francisco Casas wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/739/diffs?diff_id=107171&start_sha=f81152537041ebb341d645ee531fefb6cdc61f48#b4b2f0df64f7fbb515484cb602382012595ddfb6_431_430) I changed it to an array of pointers.
The list makes it easier to pass it around across the different parse structs and finally the variable. With arrays the parsing code is a little uglier, for instance, I had to make state_block_list a `struct parse_variable_def` instead of a mere `struct list`, but I agree it can make the rest of the code that uses this field simpler. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66088
On Fri Mar 22 21:12:58 2024 +0000, Zebediah Figura wrote:
Does this need to be a pointer? Well, as I mentioned in the other MR, I have a preference for having the structs that own other structs through pointers, as pointers themselves. For instance, if we put this struct in an array (as in the new version of that patch) without doing it through pointers, a reallocation will invalidate previous pointers to them. Granted, this doesn't happen right now, and probably will never happen for this struct, but I don't think there is any downside either? I can change it to a flat struct if you prefer.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66089
On Fri Mar 22 23:02:08 2024 +0000, Zebediah Figura wrote:
I would solve that problem by running it again, then, earlier. That makes sense!
I implemented it but there is a detail: the current implementation of compute_liveness_recurse() (required by dce) only works with offset nodes. So I extended it to work with both paths and offsets. I don't think it looks bad with the introduction of the deref_mark_last_read() helper. We may eventually remove the rel_offset parts when we complete the translation to vsir, because we would be able to use remove_dead_code() there. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66090
This is especially dead code if we aren't even parsing functions yet.
Okay, I removed it for now. I also removed `rhs_braces` following that policy.
And... do we really want to do it this way? Could we store them in a separate array?
I think it is convenient given the similarities of both formats. Also, I think it was suggested in the other MR. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66091
On Tue Mar 26 04:36:19 2024 +0000, Francisco Casas wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/739/diffs?diff_id=107171&start_sha=f81152537041ebb341d645ee531fefb6cdc61f48#b4b2f0df64f7fbb515484cb602382012595ddfb6_489_483) Interesting, when compiling the following shader with with native and `fx_4_0`:
BlendState myBS
{
BlendEnable = false;
};
technique
{
}
I get: ``` blendenable1.hlsl(3,5-24): Index is required for state 'BlendEnable' ``` meaning that it is an index, and for some state members it is required at least for `fx_4_0`, it is not for `fx_2_0`. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66092
On Fri Mar 22 21:12:57 2024 +0000, Zebediah Figura wrote:
Does this need to be a pointer? Only for consistency with `hlsl_ir_var.state_block` (and then `hlsl_ir_var.state_blocks`).
When I have to copy a struct that contains pointers I prefer to do it by copying the pointer than doing a copy-on-assignment (or having to specify the fields to be copied individually), but it is just personal preference. If you want I can change it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66093
What if the count is mismatched?
That should be an error. I added the check.
Can an array of size 1 take interior braces? Can a non-array take interior braces?
Yes they can, seems that braces lose meaning after parsing. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66094
On Tue Mar 26 04:38:04 2024 +0000, Zebediah Figura wrote:
+% For now, these tests are just to check proper parsing of effects syntax. +% Most of these tests are useless as effects. This file is very large; I'd propose splitting it into state-block-syntax (which already exists) and something more like effect-state and effect-shaders? I spread the tests between state-block-syntax.shader_test, state-block-function-syntax.shader_test, fxgroup-syntax.shader_test, and effect-compile.shader_test.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66095
On Tue Mar 26 04:38:01 2024 +0000, Francisco Casas wrote:
There is a test, if this is what you mean: ``` % It is not allowed to call the functions to set state blocks on the rhs using the assignment syntax % for state groups or passes. [pixel shader fail(sm<6)] float4 main() : sv_target { return 0; } technique { pass { cat = SetPixelShader(foobar); } } ``` But other types of function calls on the rhs are allowed. I added some more tests. No, I mean like {a = b; SetBlendState(c);}.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66146
I implemented it but there is a detail: the current implementation of compute_liveness_recurse() (required by dce) only works with offset nodes. So I extended it to work with both paths and offsets. I don't think it looks bad with the introduction of the deref_mark_last_read() helper.
Yeah, that sounds right. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66147
On Tue Mar 26 04:38:03 2024 +0000, Francisco Casas wrote:
Interesting, when compiling the following shader with with native and `fx_4_0`: ``` BlendState myBS { BlendEnable = false; }; technique { } ``` I get: ``` blendenable1.hlsl(3,5-24): Index is required for state 'BlendEnable' ``` meaning that it is an index, and for some state members it is required at least for `fx_4_0`, it is not for `fx_2_0`. I was curious and did a couple more tests, and it turns out that for fx_4_1, "SrcBlend = src_color" will actually emit all 8 indices separately, but "SrcBlend[0] = src_color" just emits index 0. So that's interesting.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66148
On Tue Mar 26 04:38:03 2024 +0000, Francisco Casas wrote:
Well, as I mentioned in the other MR, I have a preference for having the structs that own other structs through pointers, as pointers themselves. For instance, if we put this struct in an array (as in the new version of that patch) without doing it through pointers, a reallocation will invalidate previous pointers to them. Granted, this doesn't happen right now, and probably will never happen for this struct, but I don't think there is any downside either? I can change it to a flat struct if you prefer. Well, the downside is we have to do more malloc/free, mainly. I don't know that it's a strong downside, but it is the kind of thing I tend to avoid. I at least can't promise I won't convert it back to a flat struct at some point if we leave it like this.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66151
On Tue Mar 26 04:38:03 2024 +0000, Francisco Casas wrote:
This is especially dead code if we aren't even parsing functions yet. Okay, I removed it for now. I also removed `rhs_braces` following that policy. And... do we really want to do it this way? Could we store them in a separate array? I think it is convenient given the similarities of both formats. Also, I think it was suggested in the other MR. I dunno. It seems awkward to me, and it's not clear to me that defining a separate struct is actually going to complicate the code at all?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66152
On Tue Mar 26 17:47:34 2024 +0000, Zebediah Figura wrote:
I was curious and did a couple more tests, and it turns out that for fx_4_1, "SrcBlend = src_color" will actually emit all 8 indices separately, but "SrcBlend[0] = src_color" just emits index 0. So that's interesting. This is not related to how this should be parsed though.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66153
The list makes it easier to pass it around across the different parse structs and finally the variable. With arrays the parsing code is a little uglier, for instance, I had to make state_block_list a `struct parse_variable_def` instead of a mere `struct list`, but I agree it can make the rest of the code that uses this field simpler.
I find that preferable actually; I don't like passing untyped lists around. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66154
This is not related to how this should be parsed though.
Not as such. That said, if there were evidence that "SrcBlend[0]" behaved exactly like "SrcBlend", that would imply we don't even need a has_index field. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66161
On Tue Mar 26 18:08:06 2024 +0000, Zebediah Figura wrote:
This is not related to how this should be parsed though. Not as such. That said, if there were evidence that "SrcBlend[0]" behaved exactly like "SrcBlend", that would imply we don't even need a has_index field. Didn't your test confirm that it doesn't? For this particular field, it's a change in d3d10.1 that added per-target values. But anyway, this does not concern the parsing part at all, we'll emit errors only for fx profiles. For regular profiles validity of field name or index bounds are not validated. Regarding has_index, we probably need something like that unfortunately, because index limits are [0,4294967295]. We could still use -1 as special value, because actual limits will be lower, but it will be less readable I suspect.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66167
Didn't your test confirm that it doesn't?
Yes, exactly. I'm just trying to explain why I wanted to test it in the first place. *If* the results has been otherwise, that would imply a slight change in how we parse things. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_66169
participants (4)
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)