From patch 1/5:
I prefer avoiding the vkd3d_* prefix on all internal functions, for these reasons. However, I'm open to restoring it.
Note that this can make it harder to find the affected code from backtraces and logs, and in some cases bug tracker searches. I.e., suppose you're getting an error message from an application running in wine mentioning "spirv_compiler_some_function()". Does that come from Mesa, vkd3d, wined3d, or perhaps SPIRV-Tools?
From patch 2/5:
vkd3d-shader/dxbc: Use the VKD3D_DXBC_MAX_SOURCE_COUNT macro where possible.
The macro is unfortunately named, but this touches the shader model 4 (TPF) parser, for which we use the "vkd3d-shader/sm4" prefix. Similarly, if we're going to rename this in 3/5, ideally that would be to something like VKD3D_SM4_MAX_SOURCE_COUNT.
If we're cleaning things up, I'd argue for moving the SM4 parser out of dxbc.c, into its own file, like sm4.c or perhaps tpf.c. I'd also argue for merging hlsl_sm4.c and sm4.h into the same file.
From patch 4/5:
- ins->src_count = strlen(opcode_info->src_info); + ins->src_count = strnlen(opcode_info->src_info, DXBC_MAX_SOURCE_COUNT);
As it happens, I have a bit of a dislike for the strn*() functions. You'll find very few of them in vkd3d or wined3d...
From patch 5/5:
This cuts about 7 kB off of the 64-bit build.
Is that a debug build, or stripped? Avoiding pointers in struct vkd3d_sm4_opcode_info is generally desirable, but I'd expect the "read_opcode_func" field to be the bigger problem with that. Ideally we'd get rid of the linear search as well...
- struct vkd3d_shader_dst_param dst_param[2]; + struct vkd3d_shader_dst_param dst_param[DXBC_MAX_DEST_COUNT];
DXBC_MAX_DEST_COUNT is unfortunate. Like above, I'd prefer VKD3D_SM4_ over DXBC_. However, we also use either "src/dst" or "source/destination"; never "dest".