pan/bi: Don't read nonexistant destinations

Codebase audit. In preparation to dynamically allocate destinations.

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_lower_swizzle.c b/src/panfrost/bifrost/bi_lower_swizzle.c
index 542ae35..6dc762a 100644 (file)
--- a/src/panfrost/bifrost/bi_lower_swizzle.c
+++ b/src/panfrost/bifrost/bi_lower_swizzle.c
@@ -251,7 +251,7 @@ bi_lower_swizzle(bi_context *ctx)
         BITSET_WORD *replicates_16 = calloc(sizeof(bi_index), ctx->ssa_alloc);
 
         bi_foreach_instr_global(ctx, ins) {
-                if (bi_is_ssa(ins->dest[0]) && bi_instr_replicates(ins, replicates_16))
+                if (ins->nr_dests && bi_is_ssa(ins->dest[0]) && bi_instr_replicates(ins, replicates_16))
                         BITSET_SET(replicates_16, ins->dest[0].value);
 
                 if (ins->op == BI_OPCODE_SWZ_V2I16 && bi_is_ssa(ins->src[0]) &&
@@ -264,7 +264,8 @@ bi_lower_swizzle(bi_context *ctx)
                  * Valhall, we will want to optimize this. For now, default
                  * to Bifrost compatible behaviour.
                  */
-                ins->dest[0].swizzle = BI_SWIZZLE_H01;
+                if (ins->nr_dests)
+                        ins->dest[0].swizzle = BI_SWIZZLE_H01;
         }
 
         free(replicates_16);

diff --git a/src/panfrost/bifrost/bi_opt_constant_fold.c b/src/panfrost/bifrost/bi_opt_constant_fold.c
index 0099385..bebf393 100644 (file)
--- a/src/panfrost/bifrost/bi_opt_constant_fold.c
+++ b/src/panfrost/bifrost/bi_opt_constant_fold.c
@@ -95,6 +95,7 @@ bi_opt_constant_fold(bi_context *ctx)
                 if (unsupported) continue;
 
                 /* Replace with constant move, to be copypropped */
+                assert(ins->nr_dests == 1);
                 bi_builder b = bi_init_builder(ctx, bi_after_instr(ins));
                 bi_mov_i32_to(&b, ins->dest[0], bi_imm_u32(replace));
                 bi_remove_instruction(ins);

diff --git a/src/panfrost/bifrost/bi_opt_copy_prop.c b/src/panfrost/bifrost/bi_opt_copy_prop.c
index 901b589..89798e5 100644 (file)
--- a/src/panfrost/bifrost/bi_opt_copy_prop.c
+++ b/src/panfrost/bifrost/bi_opt_copy_prop.c
@@ -77,7 +77,7 @@ bi_opt_copy_prop(bi_context *ctx)
                         /* Lower the split to moves, copyprop cleans up */
                         bi_builder b = bi_init_builder(ctx, bi_before_instr(I));
 
-                        for (unsigned d = 0; d < I->nr_dests; ++d)
+                        bi_foreach_dest(I, d)
                                 bi_mov_i32_to(&b, I->dest[d], collect->src[d]);
 
                         bi_remove_instruction(I);
@@ -101,6 +101,7 @@ bi_opt_copy_prop(bi_context *ctx)
                                         replace = chained;
                         }
 
+                        assert(ins->nr_dests == 1);
                         replacement[ins->dest[0].value] = replace;
                 }
 

diff --git a/src/panfrost/bifrost/bi_opt_message_preload.c b/src/panfrost/bifrost/bi_opt_message_preload.c
index 3a26101..87d7008 100644 (file)
--- a/src/panfrost/bifrost/bi_opt_message_preload.c
+++ b/src/panfrost/bifrost/bi_opt_message_preload.c
@@ -96,7 +96,7 @@ bi_opt_message_preload(bi_context *ctx)
         bi_builder b = bi_init_builder(ctx, bi_before_nonempty_block(block));
 
         bi_foreach_instr_in_block_safe(block, I) {
-                if (!bi_is_ssa(I->dest[0])) continue;
+                if (I->nr_dests != 1 || !bi_is_ssa(I->dest[0])) continue;
 
                 struct bifrost_message_preload msg;
 

diff --git a/src/panfrost/bifrost/bi_opt_mod_props.c b/src/panfrost/bifrost/bi_opt_mod_props.c
index f4fc470..fff9332 100644 (file)
--- a/src/panfrost/bifrost/bi_opt_mod_props.c
+++ b/src/panfrost/bifrost/bi_opt_mod_props.c
@@ -192,8 +192,10 @@ bi_opt_mod_prop_forward(bi_context *ctx)
         bi_instr **lut = calloc(sizeof(bi_instr *), ctx->ssa_alloc);
 
         bi_foreach_instr_global_safe(ctx, I) {
-                if (bi_is_ssa(I->dest[0]))
-                        lut[I->dest[0].value] = I;
+                bi_foreach_dest(I, d) {
+                        if (bi_is_ssa(I->dest[d]))
+                                lut[I->dest[d].value] = I;
+                }
 
                 bi_foreach_src(I, s) {
                         if (!bi_is_ssa(I->src[s]))
@@ -391,7 +393,7 @@ bi_opt_mod_prop_backward(bi_context *ctx)
                         }
                 }
 
-                if (!bi_is_ssa(I->dest[0]))
+                if (!I->nr_dests || !bi_is_ssa(I->dest[0]))
                         continue;
 
                 bi_instr *use = uses[I->dest[0].value];

diff --git a/src/panfrost/bifrost/bi_pack.c b/src/panfrost/bifrost/bi_pack.c
index 4aa2012..ff42a08 100644 (file)
--- a/src/panfrost/bifrost/bi_pack.c
+++ b/src/panfrost/bifrost/bi_pack.c
@@ -137,7 +137,7 @@ bi_assign_slots(bi_tuple *now, bi_tuple *prev)
          * +ATEST wants its destination written to both a staging register
          * _and_ a regular write, because it may not generate a message */
 
-        if (prev->add && (!write_dreg || prev->add->op == BI_OPCODE_ATEST)) {
+        if (prev->add && prev->add->nr_dests && (!write_dreg || prev->add->op == BI_OPCODE_ATEST)) {
                 bi_index idx = prev->add->dest[0];
 
                 if (idx.type == BI_INDEX_REGISTER) {
@@ -146,8 +146,8 @@ bi_assign_slots(bi_tuple *now, bi_tuple *prev)
                 }
         }
 
-        if (prev->fma) {
-                bi_index idx = (prev->fma)->dest[0];
+        if (prev->fma && prev->fma->nr_dests) {
+                bi_index idx = prev->fma->dest[0];
 
                 if (idx.type == BI_INDEX_REGISTER) {
                         if (now->regs.slot23.slot3) {

diff --git a/src/panfrost/bifrost/bi_schedule.c b/src/panfrost/bifrost/bi_schedule.c
index c0e46be..3f02860 100644 (file)
--- a/src/panfrost/bifrost/bi_schedule.c
+++ b/src/panfrost/bifrost/bi_schedule.c
@@ -585,8 +585,7 @@ bi_can_add(bi_instr *ins)
 static bool
 bi_must_not_last(bi_instr *ins)
 {
-        return !bi_is_null(ins->dest[0]) && !bi_is_null(ins->dest[1]) &&
-               (ins->op != BI_OPCODE_TEXC_DUAL);
+        return (ins->nr_dests >= 2) && (ins->op != BI_OPCODE_TEXC_DUAL);
 }
 
 /* Check for a message-passing instruction. +DISCARD.f32 is special-cased; we
@@ -970,6 +969,9 @@ bi_has_staging_passthrough_hazard(bi_index fma, bi_instr *add)
 static bool
 bi_has_cross_passthrough_hazard(bi_tuple *succ, bi_instr *ins)
 {
+        if (ins->nr_dests == 0)
+                return false;
+
         bi_foreach_instr_in_tuple(succ, pins) {
                 bi_foreach_src(pins, s) {
                         if (bi_is_word_equiv(ins->dest[0], pins->src[s]) &&
@@ -1103,7 +1105,7 @@ bi_instr_schedulable(bi_instr *instr,
 
         /* If this choice of FMA would force a staging passthrough, the ADD
          * instruction must support such a passthrough */
-        if (tuple->add && bi_has_staging_passthrough_hazard(instr->dest[0], tuple->add))
+        if (tuple->add && instr->nr_dests && bi_has_staging_passthrough_hazard(instr->dest[0], tuple->add))
                 return false;
 
         /* If this choice of destination would force a cross-tuple passthrough, the next tuple must support that */
@@ -1138,9 +1140,15 @@ bi_instr_schedulable(bi_instr *instr,
                 return false;
 
         /* Count effective reads for the successor */
-        unsigned succ_reads = bi_count_succ_reads(instr->dest[0],
-                        tuple->add ? tuple->add->dest[0] : bi_null(),
-                        tuple->prev_reads, tuple->nr_prev_reads);
+        unsigned succ_reads = 0;
+
+        if (instr->nr_dests) {
+                bool has_t1 = tuple->add && tuple->add->nr_dests;
+                succ_reads = bi_count_succ_reads(instr->dest[0],
+                                                 has_t1 ? tuple->add->dest[0] : bi_null(),
+                                                 tuple->prev_reads,
+                                                 tuple->nr_prev_reads);
+        }
 
         /* Successor must satisfy R+W <= 4, so we require W <= 4-R */
         if ((signed) total_writes > (4 - (signed) succ_reads))
@@ -1310,9 +1318,11 @@ bi_use_passthrough(bi_instr *ins, bi_index old,
                 bool except_sr)
 {
         /* Optional for convenience */
-        if (!ins || bi_is_null(old))
+        if (!ins)
                 return;
 
+        assert(!bi_is_null(old));
+
         bi_foreach_src(ins, i) {
                 if ((i == 0 || i == 4) && except_sr)
                         continue;
@@ -1339,12 +1349,12 @@ bi_rewrite_passthrough(bi_tuple prec, bi_tuple succ)
 {
         bool sr_read = succ.add ? bi_opcode_props[succ.add->op].sr_read : false;
 
-        if (prec.add) {
+        if (prec.add && prec.add->nr_dests) {
                 bi_use_passthrough(succ.fma, prec.add->dest[0], BIFROST_SRC_PASS_ADD, false);
                 bi_use_passthrough(succ.add, prec.add->dest[0], BIFROST_SRC_PASS_ADD, sr_read);
         }
 
-        if (prec.fma) {
+        if (prec.fma && prec.fma->nr_dests) {
                 bi_use_passthrough(succ.fma, prec.fma->dest[0], BIFROST_SRC_PASS_FMA, false);
                 bi_use_passthrough(succ.add, prec.fma->dest[0], BIFROST_SRC_PASS_FMA, sr_read);
         }
@@ -1776,7 +1786,7 @@ bi_schedule_clause(bi_context *ctx, bi_block *block, struct bi_worklist st, uint
                  * passthrough the sources of the ADD that read from the
                  * destination of the FMA */
 
-                if (tuple->fma) {
+                if (tuple->fma && tuple->fma->nr_dests) {
                         bi_use_passthrough(tuple->add, tuple->fma->dest[0],
                                         BIFROST_SRC_STAGE, false);
                 }

diff --git a/src/panfrost/bifrost/bi_scoreboard.c b/src/panfrost/bifrost/bi_scoreboard.c
index f54e413..b103040 100644 (file)
--- a/src/panfrost/bifrost/bi_scoreboard.c
+++ b/src/panfrost/bifrost/bi_scoreboard.c
@@ -153,8 +153,8 @@ bi_write_mask(bi_instr *I)
          * Obscurely, ATOM_CX is sr_write but can ignore the staging register in
          * certain circumstances; this does not require consideration.
          */
-        if (bi_opcode_props[I->op].sr_write && bi_is_null(I->dest[0]) &&
-            !bi_is_null(I->src[0])) {
+        if (bi_opcode_props[I->op].sr_write && I->nr_dests &&
+            bi_is_null(I->dest[0]) && !bi_is_null(I->src[0])) {
 
                 unsigned reg = I->src[0].value;
                 unsigned count = bi_count_write_registers(I, 0);

diff --git a/src/panfrost/bifrost/valhall/va_pack.c b/src/panfrost/bifrost/valhall/va_pack.c
index 2b419a7..7b45845 100644 (file)
--- a/src/panfrost/bifrost/valhall/va_pack.c
+++ b/src/panfrost/bifrost/valhall/va_pack.c
@@ -191,6 +191,7 @@ va_pack_atom_opc_1(const bi_instr *I)
 static unsigned
 va_pack_dest(const bi_instr *I)
 {
+   assert(I->nr_dests);
    return va_pack_reg(I, I->dest[0]) | (va_pack_wrmask(I) << 6);
 }
 
@@ -442,7 +443,7 @@ va_pack_alu(const bi_instr *I)
    if (info.has_dest && info.nr_staging_dests == 0) {
       hex |= (uint64_t) va_pack_dest(I) << 40;
    } else if (info.nr_staging_dests == 0 && info.nr_staging_srcs == 0) {
-      pack_assert(I, bi_is_null(I->dest[0]));
+      pack_assert(I, I->nr_dests == 0);
       hex |= 0xC0ull << 40; /* Placeholder */
    }