vkd3d-shader/hlsl: Support non-constant offset dereferences, part 1
Indexing with non-constants offsets requires relative addressing in SM4. In assembly, this is written like in the following example:
x1[r1.x + 3]
The first part of this patch series only includes support for indexing vectors with non-constant indexes.
Following patches in https://gitlab.winehq.org/fcasas/vkd3d/-/commits/nonconst-offsets-3.
Non-constant indexing of vectors cannot be implemented with relative addressing in SM4 because this indexation cannot be performed at the level of register-components, only whole registers.
Mathematical operations must be used instead.
For floats, the native compiler seems to index an identity matrix, to get the i-th column, and then proceedes to compute the dot product between that column and the vector. For ints, bit operations seem to be performed.
While probably less efficient, this implementation complies with the type-checking at the IR level and when writing bytecode.
Merge request reports
Activity
- tests/array-index-expr.shader_test 0 → 100644
15 draw quad 16 probe all rgba (12.0, 12.0, 12.0, 12.0) 17 uniform 0 float 2 18 draw quad 19 probe all rgba (13.0, 13.0, 13.0, 13.0) 20 uniform 0 float 3 21 draw quad 22 probe all rgba (14.0, 14.0, 14.0, 14.0) 23 24 25 [pixel shader todo] 26 uniform float2 i; 27 28 float4 main() : sv_target 29 { 30 float4 f[3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; Does it matter if this is a const array or a uniform? I suppose either way we can obsolete !189 (merged) with this MR, whether we need a separate test or not.
Currently it doesn't matter only because our strategy is copying everything into temps at the beginning of the shader and (if they are output semantics or uniforms) copying from the temps back to the variables at the end of the shader.
However, I think it is good to retain both tests. Merging depending on which MR goes first.
2051 for (i = 0; i < type->dimx; ++i) 2052 { 2053 unsigned int s = hlsl_swizzle_from_writemask(1 << i); 2054 2055 if (!(comps[i] = hlsl_new_swizzle(ctx, s, 1, mult, &instr->loc))) 2056 return false; 2057 list_add_before(&instr->entry, &comps[i]->entry); 2058 } 2059 2060 res = comps[0]; 2061 for (i = 1; i < type->dimx; ++i) 2062 { 2063 if (!(res = hlsl_new_binary_expr(ctx, HLSL_OP2_ADD, res, comps[i]))) 2064 return false; 2065 list_add_before(&instr->entry, &res->entry); 2066 } - Comment on lines +2047 to +2066
I suppose that you're spelling out the dot product so that it works with any numeric type? I'd rather emit an actual DOT here and then implement DOT lowering for integral types: you make the code cleaner and implement another feature for free.
Also, notice that the current code triggers an assertion when indexing
bool4
. I think that's due to the introduction of abool
cast once they're assumed to have already been lowered. I guess this pass should happen a bit earlier (though, to be fair, it's always difficult to track the pass dependencies and assumptions). I suppose that you're spelling out the dot product so that it works with any numeric type? I'd rather emit an actual DOT here and then implement DOT lowering for integral types: you make the code cleaner and implement another feature for free.
That is a good idea! I did just that. Note that HLSL_OP2_DOT is not defined for bools, so bool vectors are a special case I had to handle with HLSL_OP2_LOGIC_AND and HLSL_OP2_LOGIC_OR.
Also, notice that the current code triggers an assertion when indexing
bool4
. I think that's due to the introduction of abool
cast once they're assumed to have already been lowered. I guess this pass should happen a bit earlier (though, to be fair, it's always difficult to track the pass dependencies and assumptions).I moved lower_nonconstant_vector_derefs() up. Also, I realized that this shouldn't be a transform_derefs(·) pass, but just a hlsl_transform_ir(·) pass on hlsl_ir_loads. transform_derefs(·) will still be used in following patches though, so I will keep its introduction patch here.
That is a good idea! I did just that. Note that HLSL_OP2_DOT is not defined for bools, so bool vectors are a special case I had to handle with HLSL_OP2_LOGIC_AND and HLSL_OP2_LOGIC_OR.
Maybe my mathematician side is prevailing too much for Zeb's tastes, but I would just decide that
DOT
is defined forbool
too, simply usingAND
instead ofMUL
andOR
instead ofADD
(unless I am missing something, you can basically use the samelower_int_dot()
implementation, just using the appropriateHLSL_OP2
constants). Notice that this is different of how thedot()
intrinsic work, but that's not necessarily a problem because thebool
dot product is immediately casted toint
while parsing (insideexpr_common_base_type()
).Edited by Giovanni MascellaniThat would save some code, but it seemed odd to me to make the space between
hlsl_transform_ir(ctx, lower_nonconstant_vector_derefs, body, NULL);
and
hlsl_transform_ir(ctx, lower_int_dot, body, NULL);
the only place in the whole codebase where HLSL_OP2_DOT for bools can exist.
I will change it if nobody disagrees though.
We don't have tests for this, but dot() of bools is uniquely supposed to return float.
I don't necessarily object to having HLSL_OP2_DOT mean something other than dot(), although I don't know that it's great. The idea that dot() of bools returns a bool is weird, too. I don't hate either one but I'm not sure it's better than the existing code either.
We don't have tests for this, but dot() of bools is uniquely supposed to return float.
Hmm, I think we are not doing that then. As Giovanni mentioned, we are making them return int.
I don't necessarily object to having HLSL_OP2_DOT mean something other than dot(), although I don't know that it's great. The idea that dot() of bools returns a bool is weird, too. I don't hate either one but I'm not sure it's better than the existing code either.
Since it allows to remove a chunk of code, I am generating the DOT for bools now. It is lowered by the following lower_int_dot() pass. I also added a comment:
/* Note: We may be creating a DOT for bool vectors here, which we need to lower to * LOGIC_OR + LOGIC_AND. */
changed this line in version 3 of the diff
the only place in the whole codebase where HLSL_OP2_DOT for bools can exist.
Though notice that it's not by design. If we find something else that sensibly generates a
DOT
ofbool
s, then we can use it there too.At any rate, I fully understand that my way of reasoning might sound a bit peculiar here, and definitely the SLOC difference isn't that large, so no hard feelings if you and Zeb decide in the end that I can keep my algebraic mutterings somewhere else.
- Resolved by Giovanni Mascellani
added 5 commits
Toggle commit list2390 2390 } 2391 2391 2392 static bool lower_int_dot(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) 2393 { 2394 struct hlsl_ir_node *arg1, *arg2, *mult, *comps[4] = {0}, *res; 2395 struct hlsl_type *type = instr->data_type; 2396 struct hlsl_ir_expr *expr; 2397 unsigned int i, dimx; 2398 2399 if (instr->type != HLSL_IR_EXPR) 2400 return false; 2401 expr = hlsl_ir_expr(instr); 2402 2403 if (expr->op != HLSL_OP2_DOT) 2404 return false; 2405 if (type->base_type == HLSL_TYPE_FLOAT) changed this line in version 3 of the diff
Would it be better to avoid that deref path manipulation by doing this lowering in lower_index_loads()?
I remember to have considered this, but:
- Intertwining the logic for indexing column-major matrices with the logic for non-constant vector dereferences may get messy.
- I just remembered that we probably want to run this pass after folding constant expressions, to avoid adding instructions that we can't fold, e.g. just because of an unfolded cast in the index.
This, unless we make the passes smart enough so they can fold
x + 0 = x
andx * 0 = 0
.
So, I moved the pass after the constant folding. I am afraid that that also requires to go through the lower_casts_to_bool() and lower_int_dot() passes a second time.
If not, I'd rather have a dedicated interface for "get the immediate parent of this deref". I really want to make as little code touch the "path" fields as possible.
I changed
hlsl_new_load_partial_path()
tohlsl_new_load_path_parent()
. I wonder if it will be possible to keep a low level of path manipulation for the resources support though.I just remembered that we probably want to run this pass after folding constant expressions, to avoid adding instructions that we can't fold, e.g. just because of an unfolded cast in the index. This, unless we make the passes smart enough so they can fold
x + 0 = x
andx * 0 = 0
.I came to thinking the same while reviewing. Even more, I might be not computationally optimal, but in the end the compiler structure might be simpler if we just lower all vector indexing operations to a dot product and them let constant folding and other passes recover the indexing operations that can be made constant.
Just saying, though, I'd first have this MR accepted and then this can be revisited later.
2038 list_add_before(&instr->entry, &c->node.entry); 2039 2040 operands[0] = swizzle; 2041 operands[1] = &c->node; 2042 if (!(eq = hlsl_new_expr(ctx, HLSL_OP2_EQUAL, operands, 2043 hlsl_get_vector_type(ctx, HLSL_TYPE_BOOL, type->dimx), &instr->loc))) 2044 return false; 2045 list_add_before(&instr->entry, &eq->entry); 2046 2047 if (!(eq = hlsl_new_cast(ctx, eq, type, &instr->loc))) 2048 return false; 2049 list_add_before(&instr->entry, &eq->entry); 2050 2051 op = HLSL_OP2_DOT; 2052 if (type->dimx == 1) 2053 op = type->base_type == HLSL_TYPE_BOOL ? HLSL_OP2_LOGIC_AND : HLSL_OP2_MUL; 2410 2486 arg2 = expr->operands[1].node; 2411 2487 assert(arg1->data_type->dimx == arg2->data_type->dimx); 2412 2488 dimx = arg1->data_type->dimx; 2489 is_bool = type->base_type == HLSL_TYPE_BOOL; 2413 2490 2414 if (!(mult = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, arg1, arg2))) 2491 if (!(mult = hlsl_new_binary_expr(ctx, is_bool ? HLSL_OP2_LOGIC_AND : HLSL_OP2_MUL, arg1, arg2))) Did my own rebase of this series as well as the second part, feel free to reuse for this MR: https://gitlab.winehq.org/flibitijibibo/vkd3d/-/tree/nonconst-offsets
In the meantime I have realized some things that needed changes in the following patches, which I now keep in https://gitlab.winehq.org/fcasas/vkd3d/-/commits/nonconst-offsets-4
added 60 commits
-
74888ad5...7c360330 - 55 commits from branch
wine:master
- 5b22cd9d - tests: Test non-constant vector indexing.
- d2db1d30 - tests: Test indexing temps.
- 48c2ae4e - vkd3d-shader/hlsl: Introduce transform_derefs().
- b8b538b3 - vkd3d-shader/hlsl: Lower dot for non-float types.
- 09b3a0ec - vkd3d-shader/hlsl: Support non-constant vector indexing.
Toggle commit list-
74888ad5...7c360330 - 55 commits from branch
mentioned in merge request !223 (merged)