From 31f6aebb2ca4b99cbb05a81cd752e6d5a0dd7865 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= <daniel@schuermann.dev>
Date: Thu, 22 Feb 2024 17:21:05 +0100
Subject: [PATCH] aco/spill: keep loop-carried variables spilled at merge
 blocks

If loop-carried variables are partially spilled at some predecessor,
keep them spilled without extra instruction.

Also slightly refactors the code for better readability.

Totals from 13 (0.02% of 79395) affected shaders: (GFX11)

Instrs: 259129 -> 258194 (-0.36%)
CodeSize: 1337604 -> 1331768 (-0.44%)
SpillSGPRs: 561 -> 560 (-0.18%)
SpillVGPRs: 146 -> 140 (-4.11%)
Latency: 2410886 -> 2408560 (-0.10%)
InvThroughput: 759229 -> 758416 (-0.11%)
VClause: 4178 -> 4154 (-0.57%)
SClause: 7217 -> 7219 (+0.03%)
Copies: 32411 -> 32174 (-0.73%)
Branches: 9426 -> 9296 (-1.38%)
VALU: 144602 -> 144098 (-0.35%)
SALU: 30582 -> 30405 (-0.58%)
VMEM: 6961 -> 6908 (-0.76%)
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27774>
---
 src/amd/compiler/aco_spill.cpp | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/amd/compiler/aco_spill.cpp b/src/amd/compiler/aco_spill.cpp
index 71a677f8fab..9bfc702c399 100644
--- a/src/amd/compiler/aco_spill.cpp
+++ b/src/amd/compiler/aco_spill.cpp
@@ -661,10 +661,13 @@ init_live_in_vars(spill_ctx& ctx, Block* block, unsigned block_idx)
        * it. Otherwise, if any predecessor reloads it, ensure it's reloaded on all other
        * predecessors. The idea is that it's better in practice to rematerialize redundantly than to
        * create lots of phis. */
-      /* TODO: test this idea with more than Dawn of War III shaders (the current pipeline-db
-       * doesn't seem to exercise this path much) */
-      bool remat = ctx.remat.count(pair.first);
-      bool spill = !remat;
+      const bool remat = ctx.remat.count(pair.first);
+      /* If the variable is spilled at the current loop-header, spilling is essentially for free
+       * while reloading is not. Thus, keep them spilled if they are at least partially spilled.
+       */
+      const bool avoid_respill = block->loop_nest_depth && ctx.loop.back().spills.count(pair.first);
+      bool spill = true;
+      bool partial_spill = false;
       uint32_t spill_id = 0;
       for (unsigned pred_idx : preds) {
          /* variable is not even live at the predecessor: probably from a phi */
@@ -673,22 +676,22 @@ init_live_in_vars(spill_ctx& ctx, Block* block, unsigned block_idx)
             break;
          }
          if (!ctx.spills_exit[pred_idx].count(pair.first)) {
-            partial_spills.emplace(pair.first, false);
-            if (!remat)
-               spill = false;
+            spill = false;
          } else {
-            partial_spills[pair.first] = true;
+            partial_spill = true;
             /* it might be that on one incoming path, the variable has a different spill_id, but
              * add_couple_code() will take care of that. */
             spill_id = ctx.spills_exit[pred_idx][pair.first];
-            if (remat)
-               spill = true;
          }
       }
+      spill |= (remat && partial_spill);
+      spill |= (avoid_respill && partial_spill);
       if (spill) {
          ctx.spills_entry[block_idx][pair.first] = spill_id;
          partial_spills.erase(pair.first);
          spilled_registers += pair.first;
+      } else {
+         partial_spills[pair.first] = partial_spill;
       }
    }
 
@@ -716,6 +719,7 @@ init_live_in_vars(spill_ctx& ctx, Block* block, unsigned block_idx)
          /* The phi is spilled at all predecessors. Keep it spilled. */
          ctx.add_to_spills(phi->definitions[0].getTemp(), ctx.spills_entry[block_idx]);
          spilled_registers += phi->definitions[0].getTemp();
+         partial_spills.erase(phi->definitions[0].getTemp());
       } else {
          /* Phis might increase the register pressure. */
          partial_spills[phi->definitions[0].getTemp()] = is_partial_spill;