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: 5febeae58e0 ("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>

diff --git a/src/panfrost/bifrost/bi_helper_invocations.c b/src/panfrost/bifrost/bi_helper_invocations.c
index 038f48b..82eda81 100644 (file)
--- a/src/panfrost/bifrost/bi_helper_invocations.c
+++ b/src/panfrost/bifrost/bi_helper_invocations.c
@@ -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 (!BITSET_TEST(deps, bi_get_node(I->dest[0])))
-                        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);
+                /* 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[d])))
+                                continue;
+
+                        /* ...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);