pan/bi: Consider all dests in helper_block_update

If an instruction has multiple destinations and *any* of them are needed by
helper invocations, we should keep helper invocations alive. This is a bug fix.
Consider the GLSL:

   first = texture(sampler, ...);
   float res = texture(sampler, vec2(first.y)).x + first.x;

Corresponding to the IR:

   first = ...
   x, y, z, w = SPLIT first
   second = TEX y, y
   x', y', z', w' = SPLIT second
   FADD res, x, x'

Here, x is not required by helper invocations (the coordinates to TEX) while y
is required. If we only look at only the first destinations, we incorrectly
decide that first is not required and fail to set the .skip bit, leading to
incorrect results.

Fixes: 5febeae58e ("pan/bi: Emit collect and split")
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17794>
(cherry picked from commit d0aaf52602)
This commit is contained in:
Alyssa Rosenzweig
2022-07-21 16:48:36 -04:00
committed by Dylan Baker
parent 7d81b290d4
commit 1959ee0183
2 changed files with 24 additions and 15 deletions

View File

@@ -5539,7 +5539,7 @@
"description": "pan/bi: Consider all dests in helper_block_update",
"nominated": true,
"nomination_type": 1,
"resolution": 0,
"resolution": 1,
"main_sha": null,
"because_sha": "5febeae58e0a133f048cb0e2d1ef45549851bea4"
},

View File

@@ -198,20 +198,24 @@ bi_helper_block_update(BITSET_WORD *deps, bi_block *block)
bool progress = false;
bi_foreach_instr_in_block_rev(block, I) {
/* If our destination is required by helper invocation... */
if (I->dest[0].type != BI_INDEX_NORMAL)
continue;
/* If a destination is required by helper invocation... */
bi_foreach_dest(I, d) {
if (bi_is_null(I->dest[d]))
continue;
if (!BITSET_TEST(deps, bi_get_node(I->dest[0])))
continue;
if (!BITSET_TEST(deps, bi_get_node(I->dest[d])))
continue;
/* ...so are our sources */
bi_foreach_src(I, s) {
if (I->src[s].type == BI_INDEX_NORMAL) {
unsigned node = bi_get_node(I->src[s]);
progress |= !BITSET_TEST(deps, node);
BITSET_SET(deps, node);
/* ...so are the sources */
bi_foreach_src(I, s) {
if (I->src[s].type == BI_INDEX_NORMAL) {
unsigned node = bi_get_node(I->src[s]);
progress |= !BITSET_TEST(deps, node);
BITSET_SET(deps, node);
}
}
break;
}
}
@@ -228,7 +232,6 @@ bi_analyze_helper_requirements(bi_context *ctx)
* derivatives */
bi_foreach_instr_global(ctx, I) {
if (I->dest[0].type != BI_INDEX_NORMAL) continue;
if (!bi_instr_uses_helpers(I)) continue;
bi_foreach_src(I, s) {
@@ -260,9 +263,15 @@ bi_analyze_helper_requirements(bi_context *ctx)
bi_foreach_instr_global(ctx, I) {
if (!bi_has_skip_bit(I->op)) continue;
if (I->dest[0].type != BI_INDEX_NORMAL) continue;
I->skip = !BITSET_TEST(deps, bi_get_node(I->dest[0]));
bool exec = false;
bi_foreach_dest(I, d) {
if (I->dest[d].type == BI_INDEX_NORMAL)
exec |= BITSET_TEST(deps, bi_get_node(I->dest[d]));
}
I->skip = !exec;
}
free(deps);