vkd3d-shader/msl: Generate shader I/O declaration and entrypoint.
Merge request reports
Activity
- Resolved by Feifan He
- Resolved by Feifan He
It looks like that by now your backend should be able to handle some very simple shader, shouldn't it? If so, it's probably a good time to start upstreaming the Metal shader runner, so it's easier for reviewers to see what the generated code looks like.
While not a hard requirements, it would be nice if each patch could fix one or more todos. You can also add the appropriate test cases that make this happen (but do that in separate patches from those that fix the todos).
- Resolved by Feifan He
- Resolved by Feifan He
shader_glsl_get_prefix()
already exists, maybe it can be moved to ir.c and shared?Yeah, I wouldn't be opposed to that. This helper is also straightforward enough that I'm not too worried about keeping it the way it is though.
I'm not sure of what Henri prefers, but usually I find the control flow easier to read if a
switch
construct is used in cases like this. Similarly for Vertex Shaders.Yeah, a switch (like msl_generate_input_struct_declarations() does, actually) may be nicer here. I'm tempted to suggest moving it to a helper function, even.
It looks like that by now your backend should be able to handle some very simple shader, shouldn't it? If so, it's probably a good time to start upstreaming the Metal shader runner, so it's easier for reviewers to see what the generated code looks like.
Yeah, I think it should be fairly close. I think for the GLSL backend, the minimum was something like being able to handle MOVs, temps, I/O registers, and either constant buffers or immediate constants.
While not a hard requirements, it would be nice if each patch could fix one or more todos. You can also add the appropriate test cases that make this happen (but do that in separate patches from those that fix the todos).
In case it helps, I tried to do that as much as possible with the GLSL backend as well; you may find the commit order in which it implements instructions helpful as a reference.
added 6 commits
- a1b616e9 - vkd3d-shader/msl: Generate shader output structure declaration.
- 87b5b5b7 - vkd3d-shader/msl: Generate shader input structure declaration.
- cd7ae00a - vkd3d-shader/msl: Generate shader entrypoint.
- 2572c370 - vkd3d-shader/msl: Generate shader entrypoint epilogue.
- 846a7954 - vkd3d-shader/msl: Generate shader entrypoint prologue.
- 743fe054 - vkd3d-shader/msl: Handle signature element mask in prologue and epilogue.
Toggle commit listThere are a few design choices which are probably fine, but for which I'd like to know something more about the motivations behind:
- Prefixing some types with
vkd3d_
. Given that we control the whole namespace it doesn't seem that we risk to conflict with anything (other than builtin language identifiers). - Adding the shader type to the main and entry point functions. Given that we convert each shader in isolation, why do we need that? In particular, doesn't having an entry point whose name changes depending on the shader, even if in a very predictable way, complicate the work of whatever will have to consume that shader?
- Having an entry point function altogether. Why can't the main function do that I/O parameter copying?
- Prefixing some types with
Prefixing some types with
vkd3d_
. Given that we control the whole namespace it doesn't seem that we risk to conflict with anything (other than builtin language identifiers).No particular reason actually, probably because it looks cool?
Adding the shader type to the main and entry point functions. Given that we convert each shader in isolation, why do we need that? In particular, doesn't having an entry point whose name changes depending on the shader, even if in a very predictable way, complicate the work of whatever will have to consume that shader?
Probably not required for entrypoint but I want to keep it for main function, in case we have requirements of implementing multiple stages in one metal stage (for interesting stuffs like tessellation/geometry pipeline). I can't say we will definitely do this though.
Having an entry point function altogether. Why can't the main function do that I/O parameter copying?
Because of
ret
/retc
instruction, which is mapped to areturn
statement directly. A shader can have multiple exits butgoto
is forbidden in msl. I don't think duplicating epilogues for every exit is right.No particular reason actually, probably because it looks cool?
It's not a big deal, but honestly I'd get rid of that.
Probably not required for entrypoint but I want to keep it for main function, in case we have requirements of implementing multiple stages in one metal stage (for interesting stuffs like tessellation/geometry pipeline). I can't say we will definitely do this though.
I'd like the entry point to have a fixed name, unless there is a good reason for doing differently (and, even then, it would probably have to be configurable by the client). And ideally that name would be
main
, since that's the standard in C-like languages.For what you call the main function I don't have such a strong opinion (since that's not an external interface), though let me note that currently it looks like useless complexity, and you can always introduce it when that's actually needed.
Because of
ret
/retc
instruction, which is mapped to areturn
statement directly. A shader can have multiple exits butgoto
is forbidden in msl. I don't think duplicating epilogues for every exit is right.Ah, yes, makes sense.
- Resolved by Feifan He
added 4 commits
Toggle commit list- Resolved by Feifan He
+static void msl_generate_vertex_output_element_attribute(struct msl_generator *gen, const struct signature_element *e) +{ + switch (e->sysval_semantic) + { + case VKD3D_SHADER_SV_POSITION: + vkd3d_string_buffer_printf(gen->buffer, "[[position]]"); + break; + case VKD3D_SHADER_SV_NONE: + vkd3d_string_buffer_printf(gen->buffer, "[[user(%s%u)]]", e->semantic_name, e->semantic_index); + break;
Apologies for not noticing this earlier, but I don't think you can do that. D3D shader model 4 (and vsir by extension) interstage I/O signatures are matched by location (i.e., e->target_location), not by semantic. So unless MSL has a way to specify an explicit location (I don't think it does?), you'll have to derive the attribute name from the location.
+ if (!(gen->prefix = msl_get_prefix(type))) + { + msl_compiler_error(gen, VKD3D_SHADER_ERROR_MSL_INTERNAL, + "Internal compiler error: Unhandled shader type %#x.", type); + gen->prefix = "unknown"; + return VKD3D_ERROR_INVALID_SHADER; + }
If we're going to return an error here, there's no need to initialise "gen->prefix" either.
added 6 commits
- c4160a9b - vkd3d-shader/msl: Generate shader output structure declaration.
- fcd2c805 - vkd3d-shader/msl: Generate shader input structure declaration.
- 654602a2 - vkd3d-shader/msl: Generate shader entrypoint.
- e67ceb8e - vkd3d-shader/msl: Generate shader entrypoint epilogue.
- 793eaa5c - vkd3d-shader/msl: Generate shader entrypoint prologue.
- 0604236b - vkd3d-shader/msl: Handle signature element mask in prologue and epilogue.
Toggle commit listadded 76 commits
-
0604236b...25232f2b - 70 commits from branch
wine:master
- cd5917c6 - vkd3d-shader/msl: Generate shader output structure declarations.
- 577cc477 - vkd3d-shader/msl: Generate shader input structure declarations.
- a88f3168 - vkd3d-shader/msl: Generate the shader entry point.
- cd070f99 - vkd3d-shader/msl: Generate the shader entry point epilogue.
- 7c42da46 - vkd3d-shader/msl: Generate the shader entry point prologue.
- b7314e24 - vkd3d-shader/msl: Handle signature element masks in the prologue and epilogue.
Toggle commit list-
0604236b...25232f2b - 70 commits from branch