ir3: Split read-modify-write array dests in two

Instructions that operate on an array read the previous state of the
array, modify it, and write a new array, at least conceptually before
RA. Previously the same register specified the previous state and acted
as the new state, but this meant that it was both a source and
destination which meant that it was getting in the way of splitting up
sources and destinations. Break out the source into a separate register,
and use the new tied-src infrastructure to share code with a6xx atomics.
With this, there are basically no more special cases for arrays in RA.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11469>

diff --git a/src/freedreno/ir3/ir3.c b/src/freedreno/ir3/ir3.c
index 0a1df2a..fdc6184 100644 (file)
--- a/src/freedreno/ir3/ir3.c
+++ b/src/freedreno/ir3/ir3.c
@@ -387,8 +387,12 @@ unsigned ir3_block_get_pred_index(struct ir3_block *block, struct ir3_block *pre
        unreachable("ir3_block_get_pred_index() invalid predecessor");
 }
 
-static struct ir3_instruction *instr_create(struct ir3_block *block, int nreg)
+static struct ir3_instruction *instr_create(struct ir3_block *block,
+               opc_t opc, int nreg)
 {
+       /* Add an extra source for array destinations */
+       if (1 <= opc_cat(opc))
+               nreg++;
        struct ir3_instruction *instr;
        unsigned sz = sizeof(*instr) + (nreg * sizeof(instr->regs[0]));
        char *ptr = ir3_alloc(block->shader, sz);
@@ -407,7 +411,7 @@ static struct ir3_instruction *instr_create(struct ir3_block *block, int nreg)
 struct ir3_instruction * ir3_instr_create(struct ir3_block *block,
                opc_t opc, int nreg)
 {
-       struct ir3_instruction *instr = instr_create(block, nreg);
+       struct ir3_instruction *instr = instr_create(block, opc, nreg);
        instr->block = block;
        instr->opc = opc;
        insert_instr(block, instr);
@@ -416,7 +420,7 @@ struct ir3_instruction * ir3_instr_create(struct ir3_block *block,
 
 struct ir3_instruction * ir3_instr_clone(struct ir3_instruction *instr)
 {
-       struct ir3_instruction *new_instr = instr_create(instr->block,
+       struct ir3_instruction *new_instr = instr_create(instr->block, instr->opc,
                        instr->regs_count);
        struct ir3_register **regs;
        unsigned i;
@@ -470,6 +474,20 @@ struct ir3_register * ir3_reg_clone(struct ir3 *shader,
        return new_reg;
 }
 
+
+void ir3_reg_set_last_array(struct ir3_instruction *instr,
+                                                       struct ir3_register *reg,
+                                                       struct ir3_register *last_write)
+{
+       assert(reg->flags & IR3_REG_ARRAY);
+       assert(reg->flags & IR3_REG_DEST);
+       struct ir3_register *new_reg = ir3_reg_create(instr, 0, 0);
+       *new_reg = *reg;
+       new_reg->flags &= ~IR3_REG_DEST;
+       new_reg->def = last_write;
+       ir3_reg_tie(reg, new_reg);
+}
+
 void
 ir3_instr_set_address(struct ir3_instruction *instr,
                struct ir3_instruction *addr)

diff --git a/src/freedreno/ir3/ir3.h b/src/freedreno/ir3/ir3.h
index 97c36f5..487dd4b 100644 (file)
--- a/src/freedreno/ir3/ir3.h
+++ b/src/freedreno/ir3/ir3.h
@@ -611,10 +611,15 @@ struct ir3_register * ir3_reg_clone(struct ir3 *shader,
 
 static inline void ir3_reg_tie(struct ir3_register *dst, struct ir3_register *src)
 {
+       assert(!dst->tied && !src->tied);
        dst->tied = src;
        src->tied = dst;
 }
 
+void ir3_reg_set_last_array(struct ir3_instruction *instr,
+                                                       struct ir3_register *reg,
+                                                       struct ir3_register *last_write);
+
 void ir3_instr_set_address(struct ir3_instruction *instr,
                struct ir3_instruction *addr);
 

diff --git a/src/freedreno/ir3/ir3_array_to_ssa.c b/src/freedreno/ir3/ir3_array_to_ssa.c
index 53ea2d9..af0a86b 100644 (file)
--- a/src/freedreno/ir3/ir3_array_to_ssa.c
+++ b/src/freedreno/ir3/ir3_array_to_ssa.c
@@ -240,8 +240,8 @@ ir3_array_to_ssa(struct ir3 *ir)
                        for (unsigned i = 0; i < instr->regs_count; i++) {
                                struct ir3_register *reg = instr->regs[i];
                                if ((reg->flags & IR3_REG_ARRAY) &&
-                                        (!reg->def || reg->def->instr->block != block)) {
-                                       reg->def = NULL;
+                                       (((reg->flags & IR3_REG_DEST) && !reg->tied) ||
+                                        (!(reg->flags & IR3_REG_DEST) && !reg->def))) {
                                        struct ir3_array *arr = ir3_lookup_array(ir, reg->array.id);
 
                                        /* Construct any phi nodes necessary to read this value */
@@ -276,15 +276,18 @@ ir3_array_to_ssa(struct ir3 *ir)
                                for (unsigned i = 0; i < instr->regs_count; i++) {
                                        struct ir3_register *reg = instr->regs[i];
                                        if ((reg->flags & IR3_REG_ARRAY)) {
-                                               if (!reg->def) {
-                                                       /* It is assumed that before calling
-                                                        * ir3_array_to_ssa(), reg->def was set to the
-                                                        * previous writer of the array within the current
-                                                        * block (if any).  The reg->def of the first write
-                                                        * to an array within a block was cleared in the
-                                                        * loop calling read_value_beginning() above.
-                                                        */
+                                               /* It is assumed that before calling
+                                                * ir3_array_to_ssa(), reg->def was set to the
+                                                * previous writer of the array within the current
+                                                * block or NULL if none.
+                                                */
+                                               if (!(reg->flags & IR3_REG_DEST) && !reg->def) {
                                                        reg->def = lookup_live_in(&ctx, block, reg->array.id);
+                                               } else if ((reg->flags & IR3_REG_DEST) && !reg->tied) {
+                                                       struct ir3_register *def =
+                                                               lookup_live_in(&ctx, block, reg->array.id);
+                                                       if (def)
+                                                               ir3_reg_set_last_array(instr, reg, def);
                                                }
                                                reg->flags |= IR3_REG_SSA;
                                        }

diff --git a/src/freedreno/ir3/ir3_context.c b/src/freedreno/ir3/ir3_context.c
index b412936..8fdaa23 100644 (file)
--- a/src/freedreno/ir3/ir3_context.c
+++ b/src/freedreno/ir3/ir3_context.c
@@ -588,7 +588,8 @@ ir3_create_array_load(struct ir3_context *ctx, struct ir3_array *arr, int n,
        __ssa_dst(mov)->flags |= flags;
        src = ir3_reg_create(mov, 0, IR3_REG_ARRAY |
                        COND(address, IR3_REG_RELATIV) | flags);
-       src->def = arr->last_write;
+       src->def = (arr->last_write && arr->last_write->instr->block == block) ?
+               arr->last_write : NULL;
        src->size  = arr->length;
        src->array.id = arr->id;
        src->array.offset = n;
@@ -623,12 +624,14 @@ ir3_create_array_store(struct ir3_context *ctx, struct ir3_array *arr, int n,
                src->barrier_conflict |= IR3_BARRIER_ARRAY_R | IR3_BARRIER_ARRAY_W;
 
                dst->flags |= IR3_REG_ARRAY;
-               dst->def = arr->last_write;
                dst->size = arr->length;
                dst->array.id = arr->id;
                dst->array.offset = n;
                dst->array.base = INVALID_REG;
 
+               if (arr->last_write && arr->last_write->instr->block == src->block)
+                       ir3_reg_set_last_array(src, dst, arr->last_write);
+
                arr->last_write = dst;
 
                array_insert(block, block->keeps, src);
@@ -650,7 +653,6 @@ ir3_create_array_store(struct ir3_context *ctx, struct ir3_array *arr, int n,
        dst = ir3_reg_create(mov, 0, IR3_REG_DEST | IR3_REG_SSA | IR3_REG_ARRAY |
                        flags |
                        COND(address, IR3_REG_RELATIV));
-       dst->def = arr->last_write;
        dst->instr = mov;
        dst->size  = arr->length;
        dst->array.id = arr->id;
@@ -658,6 +660,9 @@ ir3_create_array_store(struct ir3_context *ctx, struct ir3_array *arr, int n,
        dst->array.base = INVALID_REG;
        ir3_reg_create(mov, 0, IR3_REG_SSA | flags)->def = src->regs[0];
 
+       if (arr->last_write && arr->last_write->instr->block == block)
+               ir3_reg_set_last_array(mov, dst, arr->last_write);
+
        if (address)
                ir3_instr_set_address(mov, address);
 

diff --git a/src/freedreno/ir3/ir3_cp.c b/src/freedreno/ir3/ir3_cp.c
index fe0cc46..bed9efe 100644 (file)
--- a/src/freedreno/ir3/ir3_cp.c
+++ b/src/freedreno/ir3/ir3_cp.c
@@ -531,12 +531,6 @@ instr_cp(struct ir3_cp_ctx *ctx, struct ir3_instruction *instr)
                }
        } while (progress);
 
-       if (instr->regs[0]->flags & IR3_REG_ARRAY) {
-               struct ir3_instruction *src = ssa(instr->regs[0]);
-               if (src)
-                       instr_cp(ctx, src);
-       }
-
        if (instr->address) {
                instr_cp(ctx, instr->address);
                ir3_instr_set_address(instr, eliminate_output_mov(ctx, instr->address));

diff --git a/src/freedreno/ir3/ir3_print.c b/src/freedreno/ir3/ir3_print.c
index 9c6cb28..2ae241b 100644 (file)
--- a/src/freedreno/ir3/ir3_print.c
+++ b/src/freedreno/ir3/ir3_print.c
@@ -222,13 +222,8 @@ static void print_reg_name(struct log_stream *stream, struct ir3_instruction *in
                        print_ssa_name(stream, reg, reg->flags & IR3_REG_DEST);
                        mesa_log_stream_printf(stream, ":");
                }
-               mesa_log_stream_printf(stream, SYN_ARRAY("arr[id=%u, offset=%d, size=%u"), reg->array.id,
+               mesa_log_stream_printf(stream, SYN_ARRAY("arr[id=%u, offset=%d, size=%u]"), reg->array.id,
                                reg->array.offset, reg->size);
-               if (reg->flags & IR3_REG_DEST) {
-                       mesa_log_stream_printf(stream, SYN_ARRAY(", "));
-                       print_ssa_name(stream, reg, false);
-               }
-               mesa_log_stream_printf(stream, SYN_ARRAY("]"));
                if (reg->array.base != INVALID_REG)
                        mesa_log_stream_printf(stream, "("SYN_REG("r%u.%c")")", reg->array.base >> 2,
                                   "xyzw"[reg->array.base & 0x3]);

diff --git a/src/freedreno/ir3/ir3_ra.c b/src/freedreno/ir3/ir3_ra.c
index 34540fe..eb53d65 100644 (file)
--- a/src/freedreno/ir3/ir3_ra.c
+++ b/src/freedreno/ir3/ir3_ra.c
@@ -1136,8 +1136,6 @@ assign_src(struct ra_ctx *ctx, struct ir3_instruction *instr, struct ir3_registe
        struct ra_interval *interval = &ctx->intervals[src->def->name];
        struct ra_file *file = ra_get_file(ctx, src);
 
-       bool array_rmw = ra_reg_is_array_rmw(src);
-
        struct ir3_register *tied = src->tied;
        physreg_t physreg;
        if (tied) {
@@ -1151,15 +1149,6 @@ assign_src(struct ra_ctx *ctx, struct ir3_instruction *instr, struct ir3_registe
 
        if (src->flags & IR3_REG_FIRST_KILL)
                ra_file_remove(file, interval);
-
-       /* This source is also a destination. */
-       if (array_rmw) {
-               struct ra_interval *dst_interval = &ctx->intervals[src->name];
-               ra_interval_init(dst_interval, src);
-               dst_interval->physreg_start = physreg;
-               dst_interval->physreg_end = physreg + src->size * reg_elem_size(src);
-               ra_file_insert(file, dst_interval);
-       }
 }
 
 /* Insert a parallel copy instruction before the instruction with the parallel
@@ -1209,8 +1198,6 @@ handle_normal_instr(struct ra_ctx *ctx, struct ir3_instruction *instr)
 
        /* Allocate the destination. */
        ra_foreach_dst(dst, instr) {
-               if (ra_reg_is_array_rmw(dst))
-                       continue;
                allocate_dst(ctx, dst);
        }
 
@@ -1224,8 +1211,6 @@ handle_normal_instr(struct ra_ctx *ctx, struct ir3_instruction *instr)
 
        /* Now finally insert the destination into the map. */
        ra_foreach_dst(dst, instr) {
-               if (ra_reg_is_array_rmw(dst))
-                       continue;
                insert_dst(ctx, dst);
        }
 

diff --git a/src/freedreno/ir3/ir3_ra.h b/src/freedreno/ir3/ir3_ra.h
index a3404df..7efb658 100644 (file)
--- a/src/freedreno/ir3/ir3_ra.h
+++ b/src/freedreno/ir3/ir3_ra.h
@@ -91,16 +91,6 @@ ra_reg_is_src(const struct ir3_register *reg)
                def_is_gpr(reg->def);
 }
 
-/* Array destinations can act as a source, reading the previous array and then
- * modifying it. Return true when the register is an array destination that
- * acts like a source.
- */
-static inline bool
-ra_reg_is_array_rmw(const struct ir3_register *reg)
-{
-       return ((reg->flags & IR3_REG_ARRAY) && (reg->flags & IR3_REG_DEST) && reg->def);
-}
-
 static inline bool
 ra_reg_is_dst(const struct ir3_register *reg)
 {

diff --git a/src/freedreno/ir3/ir3_spill.c b/src/freedreno/ir3/ir3_spill.c
index 94bd85e..1205dcc 100644 (file)
--- a/src/freedreno/ir3/ir3_spill.c
+++ b/src/freedreno/ir3/ir3_spill.c
@@ -250,11 +250,9 @@ handle_instr(struct ra_spill_ctx *ctx, struct ir3_instruction *instr)
         */
 
        ra_foreach_dst(dst, instr) {
-               if (!ra_reg_is_array_rmw(dst)) {
-                       struct ir3_register *tied_src = dst->tied;
-                       if (tied_src && !(tied_src->flags & IR3_REG_FIRST_KILL))
-                               insert_dst(ctx, dst);
-               }
+               struct ir3_register *tied_src = dst->tied;
+               if (tied_src && !(tied_src->flags & IR3_REG_FIRST_KILL))
+                       insert_dst(ctx, dst);
        }
 
        update_max_pressure(ctx);

diff --git a/src/freedreno/ir3/ir3_validate.c b/src/freedreno/ir3/ir3_validate.c
index 779edc2..68e931d 100644 (file)
--- a/src/freedreno/ir3/ir3_validate.c
+++ b/src/freedreno/ir3/ir3_validate.c
@@ -150,7 +150,11 @@ validate_instr(struct ir3_validate_ctx *ctx, struct ir3_instruction *instr)
                 * bindless, irrespective of the precision of other srcs. The
                 * tex/samp src is the first src reg when .s2en is set
                 */
-               if ((instr->flags & IR3_INSTR_S2EN) && (n < 2)) {
+               if (reg->tied) {
+                       /* must have the same size as the destination, handled in
+                        * validate_reg().
+                        */
+               } else if ((instr->flags & IR3_INSTR_S2EN) && (n < 2)) {
                        if (n == 0) {
                                if (instr->flags & IR3_INSTR_B)
                                        validate_assert(ctx, !(reg->flags & IR3_REG_HALF));