From f583d6f7bac53ac4f72e1be9f9a60bc51fb884c0 Mon Sep 17 00:00:00 2001 From: Connor Abbott Date: Mon, 25 Mar 2024 11:18:40 -0400 Subject: [PATCH] ir3: Fix detection of nontrivial continues We may still need to insert a continue block even if there is only one backedge, in a situation like: for (...) { if (...) continue; foo(); break; } We want foo() to be executed before reconverging. This is important for the BVH encoding kernel, which launches an invocation for each node in the tree and does a preorder traversal: while (true) { if (!ready[node]) continue; encode(); for (child node) ready[child] = true; break; } For the first few nodes, which will be in the same wave, we need encode() for the root node to be called first, then its children spin until ready, then the children call encode(), and so on. This can only work if the children that aren't ready yet are parked while the parent executes encode(), which requires the continue block. This is also required because divergence analysis will assume that uniform values written before the continue are still uniform after it, which isn't the case now and causes an RA validation failure with Godot. Fixes: 0fa93fb6626 ("ir3: Fix convergence behavior for loops with continues") Part-of: (cherry picked from commit d3533716f95d77e35103562b8edee864ffc16a18) --- .pick_status.json | 2 +- src/freedreno/ir3/ir3_compiler_nir.c | 35 ++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 48a3a519206..831144a8498 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -564,7 +564,7 @@ "description": "ir3: Fix detection of nontrivial continues", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "0fa93fb6626b4270ec0086d34a336046addd4462", "notes": null diff --git a/src/freedreno/ir3/ir3_compiler_nir.c b/src/freedreno/ir3/ir3_compiler_nir.c index 19b6d6784f4..2fae5ac5345 100644 --- a/src/freedreno/ir3/ir3_compiler_nir.c +++ b/src/freedreno/ir3/ir3_compiler_nir.c @@ -4251,6 +4251,32 @@ emit_if(struct ir3_context *ctx, nir_if *nif) emit_cf_list(ctx, &nif->else_list); } +static bool +has_nontrivial_continue(nir_loop *nloop) +{ + struct nir_block *nstart = nir_loop_first_block(nloop); + + /* There's always one incoming edge from outside the loop, and if there + * is more than one backedge from inside the loop (so more than 2 total + * edges) then one must be a nontrivial continue. + */ + if (nstart->predecessors->entries > 2) + return true; + + /* Check whether the one backedge is a nontrivial continue. This can happen + * if the loop ends with a break. + */ + set_foreach (nstart->predecessors, entry) { + nir_block *pred = (nir_block*)entry->key; + if (pred == nir_loop_last_block(nloop) || + pred == nir_cf_node_as_block(nir_cf_node_prev(&nloop->cf_node))) + continue; + return true; + } + + return false; +} + static void emit_loop(struct ir3_context *ctx, nir_loop *nloop) { @@ -4260,12 +4286,11 @@ emit_loop(struct ir3_context *ctx, nir_loop *nloop) struct nir_block *nstart = nir_loop_first_block(nloop); struct ir3_block *continue_blk = NULL; - /* There's always one incoming edge from outside the loop, and if there - * is more than one backedge from inside the loop (so more than 2 total - * edges) then we need to create a continue block after the loop to ensure - * that control reconverges at the end of each loop iteration. + /* If the loop has a continue statement that isn't at the end, then we need to + * create a continue block in order to let control flow reconverge before + * entering the next iteration of the loop. */ - if (nstart->predecessors->entries > 2) { + if (has_nontrivial_continue(nloop)) { continue_blk = create_continue_block(ctx, nstart); }