[PATCH v3 0/2] MR340: vkd3d-shader/hlsl: Document some possibly obscure HLSL opcodes.
-- v3: vkd3d-shader/hlsl: Document some possibly obscure HLSL opcodes. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/340
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> It is unused, and it's not clear whether it would be of any help to have it. --- libs/vkd3d-shader/hlsl.c | 1 - libs/vkd3d-shader/hlsl.h | 1 - 2 files changed, 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 09580970..5fe9047b 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2486,7 +2486,6 @@ const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op) [HLSL_OP2_RSHIFT] = ">>", [HLSL_OP3_DP2ADD] = "dp2add", - [HLSL_OP3_LERP] = "lerp", [HLSL_OP3_MOVC] = "movc", [HLSL_OP3_TERNARY] = "ternary", }; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 04e681a9..bece9a4a 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -551,7 +551,6 @@ enum hlsl_ir_expr_op HLSL_OP2_RSHIFT, HLSL_OP3_DP2ADD, - HLSL_OP3_LERP, /* TERNARY is used specifically for ternary operator, and later lowered according to profile to e.g. MOVC. */ HLSL_OP3_MOVC, HLSL_OP3_TERNARY, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/340
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index bece9a4a..44cebaaf 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -550,8 +550,13 @@ enum hlsl_ir_expr_op HLSL_OP2_NEQUAL, HLSL_OP2_RSHIFT, + /* DP2ADD(a, b, c) computes the scalar product of a.xy and b.xy, + * then adds c. */ HLSL_OP3_DP2ADD, - /* TERNARY is used specifically for ternary operator, and later lowered according to profile to e.g. MOVC. */ + /* MOVC(a, b, c) returns c if a is bitwise zero and b otherwise. + * TERNARY(a, b, c) returns c if a == 0 and b otherwise. + * They differ for floating point numbers, because + * -0.0 == 0.0, but it is not bitwise zero. */ HLSL_OP3_MOVC, HLSL_OP3_TERNARY, }; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/340
Alternatively:
"MOVC(x, y, z) evaluates to \`z' when \`x' is bitwise zero and to \`y' otherwise."
I deliberately avoided using `x`, `y` and `z` because they easily confuse with swizzle letters. Of course `a` and `b` are used for swizzles too, but the fact that `c` and `d` are not arguably makes it harder to fall for it.
Fair enough, although "a" in particular has the annoying property that it's also a word in English. (As well as in a number of other languages, for that matter.)
At the risk of rehashing a discussion we've been over before, this does suggest that "MOVC" is perhaps not the most appropriate name for operation; this sounds like what some other languages might call "if". (I.e., "MOVC(x, y, z)" is essentially "(if x y z)".)
Both `MOVC` and `TERNARY` are a form of `if`, so `if` looks precisely like the thing you'd want to avoid to name either of them. They just differ by a technicality in how they evaluate trueness. The names still make some sense, given the context: `MOVC` is designed to reproduce the raw behavior of, well, the `MOVC` operator in TPF, while `TERNARY` is designed to reproduce the behavior of the ternary operator in C.
Are both "y" and "z" always evaluated, or is that conditional on "x" as well?
From my point of view the HLSL IR doesn't have a concept of evaluating nodes. Nodes purely forward a value and have no side effects.
HLSL_IR_STORE and HLSL_IR_RESOURCE_STORE exist though. I don't think you can use those as HLSL_IR_EXPR sources (right?), but that might not necessarily be obvious. It does touch a bit on what I think the issue with "MOVC" is as well though; "MOVC" sounds suspiciously like something that might be called a conditional store operation.
So "TERNARY(a, b, c)" is equivalent to "MOVC(EQUAL(a, 0), b, c)", right?
Rather `MOVC(NEQUAL(a, 0), b, c)`, unless I am missing something, where `0` is assumed to be of the same type as `a`. As the comment notices, they only differ for `a == -0.0`: `MOVC(-0.0, b, c) == b`, while `TERNARY(-0.0, b, c) == c`.
Right, NEQUAL(), and that's indeed what lower_ternary() does. Anyway, I think this MR is fine like this. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/340#note_45348
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/340
HLSL_IR_STORE and HLSL_IR_RESOURCE_STORE exist though. I don't think you can use those as HLSL_IR_EXPR sources (right?), but that might not necessarily be obvious. It does touch a bit on what I think the issue with "MOVC" is as well though; "MOVC" sounds suspiciously like something that might be called a conditional store operation.
Yes, those have a side effect, but they don't yield a value. Naming is hard and I don't think that we're going to have a perfect name here, which is why we need this documentation. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/340#note_45359
participants (4)
-
Giovanni Mascellani -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura (＠zfigura)