On 4/20/21 3:16 AM, Matteo Bruni wrote:
On Fri, Apr 16, 2021 at 2:04 AM Zebediah Figura zfigura@codeweavers.com wrote:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
Makefile.am | 2 + include/.gitignore | 1 + include/vkd3d_d3d9types.h | 6 + include/vkd3d_d3dx9shader.idl | 76 +++++++++ libs/vkd3d-shader/hlsl.h | 7 +- libs/vkd3d-shader/hlsl.y | 1 + libs/vkd3d-shader/hlsl_codegen.c | 254 ++++++++++++++++++++++++++++++- 7 files changed, 344 insertions(+), 3 deletions(-) create mode 100644 include/vkd3d_d3dx9shader.idl
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 3a0bf38d..3dc7a26b 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -115,7 +115,6 @@ struct hlsl_type unsigned int modifiers; unsigned int dimx; unsigned int dimy;
- unsigned int reg_size; union { struct list *elements;
@@ -125,6 +124,9 @@ struct hlsl_type unsigned int elements_count; } array; } e;
- unsigned int reg_size;
- unsigned int bytecode_offset;
};
struct hlsl_struct_field @@ -134,7 +136,9 @@ struct hlsl_struct_field struct hlsl_type *type; const char *name; const char *semantic;
- unsigned int reg_offset;
- unsigned int name_offset;
Although these are indeed two offsets, they offset into very different things. I don't know that I have any good suggestions (name_bc_offset?) and I guess I'm not too disturbed by the current naming anyway, but I feel there is room for improvement.
Yes, you're right; honestly the mistake here is grouping them together via whitespace I think, but of course the naming is what led me to do that. "name_bytecode_offset" strikes me as at least a better name than "name_offset".
I've vacillated for a while, also; it may make more sense not to store reg_offset + reg_size but instead to calculate them as needed. But I've tried that approach a couple times and the code doesn't look prettier :-(
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 25227057..73f8bdb0 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c
- LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
- {
if (!var->semantic && var->reg.allocated){put_dword(buffer, 0); /* name */put_dword(buffer, D3DXRS_FLOAT4 | (var->reg.id << 16));This is fine for now, but maybe a comment mentioning that we might not always want the float register set here would be useful. Relatedly, we might have the same variable allocated to multiple register sets (e.g. if it's used in a MOV and in a FOR) so in theory there needs to be a loop (and we need to support multiple register allocations per var, or something to the same effect). Again, no need to change the code right away, just something that might be worth a comment.
Fair enough. Actually I'd be inclined to handle this by creating multiple hlsl_ir_var objects.