From d26d8c5617491bb9ade6db77dd9cb3a1c594ec2e Mon Sep 17 00:00:00 2001 From: Connor Abbott Date: Sat, 27 Jul 2019 18:31:09 +0200 Subject: [PATCH] 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 Signed-off-by: Connor Abbott --- src/gallium/drivers/lima/ir/gp/scheduler.c | 11 ++++------- 1 file 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 7149e1d..69606f2 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) { -- 2.7.4