vkd3d-shader/ir: Refactor VSIR passes.
Merge request reports
Activity
requested review from @giomasce
mentioned in merge request !568 (merged)
2233 2233 struct vkd3d_shader_instruction_array *instructions = &parser->instructions; 2234 2234 enum vkd3d_result result = VKD3D_OK; 2235 2235 2236 if (parser->shader_desc.is_dxil) 2237 return result; 2238 2239 if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL) 2236 if (!parser->shader_desc.is_dxil) I have an alternate version of this in !579, using separate
vkd3d_shader_normalise_sm4()
andvkd3d_shader_normalise_sm6()
per your suggestion.The fact that we can potentially have passes that should be called on both
vkd3d_shader_normalise_sm4()
andvkd3d_shader_normalise_sm6()
, such asremove_dcl_temps()
in 3/3 may be an argument against separating the function, but I don't know if we will end up with enough for the repetition to be a problem.The fact that we can potentially have passes that should be called on both
vkd3d_shader_normalise_sm4()
andvkd3d_shader_normalise_sm6()
, such asremove_dcl_temps()
in 3/3 may be an argument against separating the function, but I don't know if we will end up with enough for the repetition to be a problem.Yeah, I guess it's still a bit early to commit on that decision. Right now both alternatives look essentially equivalent.
mentioned in merge request !514 (merged)
Votes welcome.
I don't hate either version. I imagine that ultimately we may want to move to a scheme like this though:
if (program->cf_type == CF_TYPE_STRUCTURED) flatten_control_flow_constructs(program); [...]
where "cf_type" would originally be set by the parser. I.e., explicitly checking for properties of the vsir instead of having those be implied by the shader model or bytecode format.
I don't hate either version. I imagine that ultimately we may want to move to a scheme like this though:
if (program->cf_type == CF_TYPE_STRUCTURED) flatten_control_flow_constructs(program); [...]
where "cf_type" would originally be set by the parser. I.e., explicitly checking for properties of the vsir instead of having those be implied by the shader model or bytecode format.
Yes, that's my idea too. Relatedly, as was already discussed, I expect support for different dialects/flags to eventually appear in order to validate language features that do not necessarily hold at every step, but which are still worthwhile to check at every step where they should hold.
2658 2636 case VKD3DSIH_DCL_TEMPS: 2659 2637 vsir_validate_dst_count(ctx, instruction, 0); 2660 2638 vsir_validate_src_count(ctx, instruction, 0); 2661 if (ctx->dcl_temps_found) 2662 validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_DUPLICATE_DCL_TEMPS, "Duplicate DCL_TEMPS instruction."); In theory yes, but for the moment I mostly see the validator as a device to ease development (by providing a reference on the VSIR syntax and therefore an interface between frontend, passes and backend) rather than a picky language checker for the sake of adhering to a specific standard. Since basically developing passes and backends is much easier using
temp_count
, I currently see checking specifically that DCL_TEMPS is not duplicate as out of scope.changed this line in version 2 of the diff
added 1 commit
- 36774d6a - vkd3d-shader/ir: Remove DCL_TEMPS instructions.
added 59 commits
-
36774d6a...72e2eeaf - 56 commits from branch
wine:master
- cb22b4c2 - vkd3d-shader/ir: Refactor return code checking in vkd3d_shader_normalise().
- 6e822817 - vkd3d-shader/ir: Run tracing and validation for DXIL code too.
- c06214ca - vkd3d-shader/ir: Remove DCL_TEMPS instructions.
Toggle commit list-
36774d6a...72e2eeaf - 56 commits from branch