fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-05-07 09:18:04 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

pan/bi: Fix coupling spill placement

Browse source 
In the following arrangement the old logic leads to the following:
                       |
                       v
            +----------+------------+
            |block5                 |
            |m815 = PHI m1034, m860 |<-----------+
            |343 = FMA.f32 ...      |            |
            +----------+------------+            |
                       |                         |
        +--------------+                         |
        |              |                         |
        v              v                         |
     +-----+        +-----+                      |
     |b6   |        |b7,8 |                      |
     |     |        |     |                      |
     +-----+        +--+--+                      |
        |    +---+     |    +---+                |
        +----|b9 +-----+----|b10+---+            |
        v    +---+          +---+   v            |
+-------+-------------+     +-------+---------+  |
|block12              |     |block11          |  |
|m882 = PHI m815, m860|     |m860 = MEMMOV 343+--+
+---------+-----------+     +-----------------+
          v

The spill of / into m860 (corresponding to 343) ends up in block11 when
insert_coupling_code(succ=block5, pred=block11) because of the memory
phi in block5. Later, in insert_coupling_code(block12, block9), we
reject inserting the spill after ca9c9957. As a result, m860 is
undefined along block5 -> block7,8 -> block9 -> block12.

When the spill position is chosen first, ctx->block is block5 so
choose_spill_position falsely returns the fallback position. The issue
can be fixed by explicitly passing the "current block".

Fixes: ca9c9957 ("pan: Avoid some redundant SSA spills")
Reviewed-by: Eric R. Smith <eric.smith@collabora.com>
(cherry picked from commit 09e1ba28e5)

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/40359>
This commit is contained in:
Christoph Pillmayer 2026-03-02 16:27:14 +01:00 • committed by Eric Engestrom
parent 734e53c96b
commit 955a82bb83
2 changed files with 22 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
.pick_status.json
View file

@ -3564,7 +3564,7 @@
"description": "pan/bi: Fix coupling spill placement",
"nominated": true,
"nomination_type": 2,
"resolution": 0,
"resolution": 1,
"main_sha": null,
"because_sha": "ca9c9957e20bd33f9a79ca71ef7d72d8aef0d6e8",
"notes": null

27
src/panfrost/compiler/bifrost/bi_spill_ssa.c
View file

@ -383,8 +383,21 @@ remat_to(bi_builder *b, bi_index dst, struct spill_ctx *ctx, unsigned node)
}
}
/* This tries to return a cursor in the block in which 'node' is defined.
* Returns the fallback position if the block that defines 'node' equals
* 'current_block'.
*
* 'current_block' if called from:
* - min_algorithm: ctx->block
* - insert_coupling_code: current_block should be the block in which the spill
* is needed (i.e. not ctx->block, if ctx->block == succ, but the spill is
* required from pred). The reason for this is, if a value defined in succ
* needs to be spilled first along the backedge back to succ from pred, the
* spilled version can be undefined at the place of a later use.
*/
static bi_cursor
choose_spill_position(struct spill_ctx *ctx, unsigned node, bi_cursor fallback)
choose_spill_position(struct spill_ctx *ctx, unsigned node, bi_cursor fallback,
bi_block *current_block)
{
bi_instr *producer = ctx->ssa_defs[node];
bi_block *block = ctx->ssa_def_blocks[node];
@ -392,7 +405,7 @@ choose_spill_position(struct spill_ctx *ctx, unsigned node, bi_cursor fallback)
assert(producer);
assert(block);
if (ctx->block == block)
if (current_block == block)
return fallback;
/* Don't insert spills in the middle of the PHI block. This breaks the
@ -569,8 +582,8 @@ insert_coupling_code(struct spill_ctx *ctx, bi_block *pred, bi_block *succ)
}
if (!spilled) {
bi_cursor c = choose_spill_position(ctx, I->src[s].value,
bi_after_block_logical(pred));
bi_cursor c = choose_spill_position(
ctx, I->src[s].value, bi_after_block_logical(pred), pred);
bi_builder b = bi_init_builder(ctx->shader, c);
insert_spill(&b, ctx, I->src[s].value);
@ -615,7 +628,8 @@ insert_coupling_code(struct spill_ctx *ctx, bi_block *pred, bi_block *succ)
if (spilled)
continue;
bi_cursor c = choose_spill_position(ctx, v, bi_along_edge(pred, succ));
bi_cursor c = choose_spill_position(ctx, v, bi_along_edge(pred, succ),
bi_edge_to_block(pred, succ));
bi_builder b = bi_init_builder(ctx->shader, c);
insert_spill(&b, ctx, v);
}
@ -1187,7 +1201,8 @@ limit(struct spill_ctx *ctx, bi_instr *I, unsigned m)
* another use
*/
if (!BITSET_TEST(ctx->S, v) && candidates[i].dist < DIST_INFINITY) {
bi_cursor c = choose_spill_position(ctx, v, bi_before_instr(I));
bi_cursor c =
choose_spill_position(ctx, v, bi_before_instr(I), ctx->block);
bi_builder b = bi_init_builder(ctx->shader, c);
insert_spill(&b, ctx, v);
BITSET_SET(ctx->S, v);