vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.
Merge request reports
Activity
+ if (!sm1->disassembling) + element->used_mask &= element->mask;
Couldn't we just check for something like "program->normalisation_level >= VSIR_NORMALISED_SM4"?
vkd3d_shader_parser_init(&sm1->p, program, message_context, compile_info->source_name); sm1->ptr = sm1->start; + sm1->disassembling = compile_info->target_type == VKD3D_SHADER_TARGET_D3D_ASM; + + if (sm1->disassembling) + program->normalisation_level = VSIR_NORMALISED_SM1;
Instead of modifying the normalisation level shortly after calling vsir_program_init(), it would make more sense to me to pass the appropriate level to vsir_program_init() in the first place.
- /* Here we'd likely want to validate that the usage mask is a subset of the - * signature mask. Unfortunately the D3DBC parser sometimes violates this. - * For example I've seen a shader like this: - * ps_3_0 - * [...] - * dcl_texcoord0 v0 - * [...] - * texld r2.xyzw, v0.xyzw, s1.xyzw - * [...] - * - * The dcl_textcoord0 instruction secretly has a .xy mask, which is used to - * compute the signature mask, but the texld instruction apparently uses all - * the components. Of course the last two components are ignored, but - * formally they seem to be used. So we end up with a signature element with - * mask .xy and usage mask .xyzw. - * - * The correct fix would probably be to make the D3DBC parser aware of which - * components are really used for each instruction, but that would take some - * time. */
It may not hurt to retain that comment in some form; either here, in the parser when adjusting the mask, or both.
It may not hurt to retain that comment in some form; either here, in the parser when adjusting the mask, or both.
Let me know whether the amended version sounds better.
BTW, I think I'll eventually have to fix the register mask/swizzle too; actually, that's the part I really care about, because I need to ensure that each register resolves unambiguously to a single signature element, and since for later shader models more than a signature element might use the same register index the write mask has to be taken into consideration too.
added 1 commit
- 5fcafe39 - vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.
This is just an opinion, and I don't understand well the different vsir normalization levels yet, but I don't think it is good to start adding target_type checks in the sm1 parser.
Have you considered leaving the parser to always retrieve SM1-normalized vsir and add these signature modifications in an independent ir.c pass that turns a SM1-normalized program into a SM4-normalized program? This pass to be skipped when disassembling.
Edited by Francisco CasasHave you considered leaving the parser to always retrieve SM1-normalized vsir and add these signature modifications in an independent ir.c pass that turns a SM1-normalized program into a SM4-normalized program? This pass to be skipped when disassembling.
I considered that, and while on one hand I appreciate how completely decoupling the frontend from the backend is a cleaner decision, on the other hand that would require some more code. Not that much, but still more than the two-line change here. Also, to me disassembling is a somewhat more special target than all the others, since it needs us to preserve the exact program form, not just its semantics. So it's less of a sin to treat it differently even in the parser.
On top of that, this decision can be reverted at any time without too much hassle. So I would bother with moving the change to a separate pass only once there is some evidence that it would make something easier. Currently we don't even apply passes when disassembling, do we?
added 17 commits
-
5fcafe39...09095403 - 15 commits from branch
wine:master
- 64126a00 - vkd3d-shader/ir: Give more meaningful names to I/O normalisation levels.
- dd0ed989 - vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.
-
5fcafe39...09095403 - 15 commits from branch