From d28b22374cb01ebb22c66bb940891d4b1996bfaa Mon Sep 17 00:00:00 2001 From: Connor Abbott Date: Mon, 22 Mar 2021 11:30:53 +0100 Subject: [PATCH] ir3/cp_postsched: Fixup SSA use pointer for direct reads There's an optimization here to sink direct (i.e. not relative) reads of an array past unrelated direct writes. However, since each write actually reads, modifies, and then writes again to the array, this means that we need to read the latest updated array. The old RA used the array id instead of the SSA information, so it didn't care, but the new RA uses ->instr instead and ignores the array id because arrays are now SSA so it needs to be correct. Part-of: --- src/freedreno/ir3/ir3_cp_postsched.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/freedreno/ir3/ir3_cp_postsched.c b/src/freedreno/ir3/ir3_cp_postsched.c index ba5ff6b97b4..65b1ea0612c 100644 --- a/src/freedreno/ir3/ir3_cp_postsched.c +++ b/src/freedreno/ir3/ir3_cp_postsched.c @@ -43,14 +43,18 @@ * access and we care about all writes to the array (as we don't know * which array element is read). Otherwise in the case of non-relative * access, we only have to care about the write to the specified (>= 0) - * offset. + * offset. In this case, we update `def` to point to the last write in + * between `use` and `src` to the same array, so that `use` points to + * the correct array write. */ static bool has_conflicting_write(struct ir3_instruction *src, struct ir3_instruction *use, + struct ir3_instruction **def, unsigned id, int offset) { assert(src->block == use->block); + bool last_write = true; /* NOTE that since src and use are in the same block, src by * definition appears in the block's instr_list before use: @@ -93,6 +97,11 @@ has_conflicting_write(struct ir3_instruction *src, /* is write to same array element? */ if (dst->array.offset == offset) return true; + + if (last_write) + *def = instr; + + last_write = false; } return false; @@ -143,7 +152,8 @@ instr_cp_postsched(struct ir3_instruction *mov) if (is_meta(use)) continue; - if (has_conflicting_write(mov, use, src->array.id, offset)) + struct ir3_instruction *def = src->instr; + if (has_conflicting_write(mov, use, &def, src->array.id, offset)) continue; if (conflicts(mov->address, use->address)) @@ -164,6 +174,11 @@ instr_cp_postsched(struct ir3_instruction *mov) /* preserve (abs)/etc modifiers: */ use->regs[n + 1]-> flags |= reg->flags; + /* If we're sinking the array read past any writes, make + * sure to update it to point to the new previous write: + */ + use->regs[n + 1]->instr = def; + removed = true; }