lima/gpir/sched: Don't try to spill when something else has succeeded

In try_node(), we assume that the node we pick can still be scheduled
successfully after speculatively trying all the other nodes. Normally we
always undo every node after speculating it, so that when we finally
schedule best_node the scheduler state is exactly the same and it
succeeds. However, we also try to spill nodes, which can change the
state and in a corner case that can make scheduling best_node fail. In
particular, the following sequence of events happened with piglit
shaders@glsl-vs-if-nested: a partially-ready node N was spilled and a
register store node S, which is a use of N, was created and then later
the other uses of N were scheduled, so that S is now ready and N is
partially ready. First we try to schedule S and succeed, then we try to
schedule another node M, which fails, so we try to spill the remaining
uses of N. This succeeds, but scheduling M still fails so that best_node
is still S. However since one of the uses of N is one cycle ago, and
therefore we inserted a read dependent on S one cycle ago when spilling
N, S can no longer be scheduled as read-after-write latency is three
cycles.

While we could ad-hoc try to catch cases like this, or (the best option
but very complicated) treat the spill as speculative and roll it back if
we decide not to schedule the node, a simpler solution is to just
give up on spilling if we've already successfully speculatively
scheduled another node. We'd give up a few cases where we discover that
by spilling even harder we could schedule a more desirable node, but
that seems like it would be pretty rare in practice. With this we
guarantee that nothing has been touched after best_node was successfully
scheduled. We also cut down on pointless spilling, since if we already
scheduled a node it's unlikely that spilling harder will let us schedule
an even better node, and hence any spilling at this point is probably
useless.

While we're here, clean up the code around spilling by flattening the
two if's and getting rid of the second unnecessary check for INT_MIN.

Fixes: 54434fe6706 ("lima/gpir: Rework the scheduler")
Acked-by: Qiang Yu <[email protected]>
Signed-off-by: Connor Abbott <[email protected]>

1 files changed, 4 insertions, 7 deletions

diff --git a/src/gallium/drivers/lima/ir/gp/scheduler.c b/src/gallium/drivers/lima/ir/gp/scheduler.c
index 7149e1dd213..69606f22d51 100644
--- a/src/gallium/drivers/lima/ir/gp/scheduler.c
+++ b/src/gallium/drivers/lima/ir/gp/scheduler.c
@@ -1212,13 +1212,10 @@ static bool try_node(sched_ctx *ctx)
          ctx->total_spill_needed = 0;
          ctx->max_node_spill_needed = 0;
          int score = schedule_try_node(ctx, node, true);
-         if (score == INT_MIN) {
-            if (ctx->total_spill_needed > 0 &&
-                try_spill_nodes(ctx, node)) {
-               score = schedule_try_node(ctx, node, true);
-               if (score == INT_MIN)
-                  continue;
-            }
+         if (score == INT_MIN && !best_node &&
+             ctx->total_spill_needed > 0 &&
+             try_spill_nodes(ctx, node)) {
+            score = schedule_try_node(ctx, node, true);
          }
 
          if (score > best_score) {