Mesa (main): nir: refactor nir_opt_move
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Wed Jan 12 14:33:58 UTC 2022
Module: Mesa
Branch: main
Commit: 8a78706643ecad8a1f303cc9358873abc29978b4
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=8a78706643ecad8a1f303cc9358873abc29978b4
Author: Daniel Schürmann <daniel at schuermann.dev>
Date: Fri Jan 31 17:44:19 2020 +0100
nir: refactor nir_opt_move
This patch is a rewrite of nir_opt_move.
Differently from the previous version, each instruction is checked
if it can be moved downwards and then inserted before the first user
of the definition. The advantage is that less insert operations are
performed, the original order is kept if two movable instructions have
the same first user, and instructions without user in the same block
are moved towards the end.
v2: Only return true if an instruction really changed the position.
Don't care for discards, this will be handled by another MR.
v3: fix self-referring phis and update according to nir_can_move_instr().
v4: use nir_can_move_instr() and nir_instr_ssa_def()
v5: deduplicate some code
Reviewed-by: Rhys Perry <pendingchaos02 at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3657>
---
src/amd/compiler/tests/test_isel.cpp | 8 +--
src/compiler/nir/nir_opt_move.c | 134 +++++++++++++++--------------------
2 files changed, 61 insertions(+), 81 deletions(-)
diff --git a/src/amd/compiler/tests/test_isel.cpp b/src/amd/compiler/tests/test_isel.cpp
index a6aae159064..d74822810bd 100644
--- a/src/amd/compiler/tests/test_isel.cpp
+++ b/src/amd/compiler/tests/test_isel.cpp
@@ -42,12 +42,12 @@ BEGIN_TEST(isel.interp.simple)
void main() {
//>> v1: %a_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.w
//! v1: %a = v_interp_p2_f32 %by, %pm:m0, (kill)%a_tmp attr0.w
- //! v1: %b_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.z
- //! v1: %b = v_interp_p2_f32 %by, %pm:m0, (kill)%b_tmp attr0.z
+ //! v1: %r_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.x
+ //! v1: %r = v_interp_p2_f32 %by, %pm:m0, (kill)%r_tmp attr0.x
//! v1: %g_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.y
//! v1: %g = v_interp_p2_f32 %by, %pm:m0, (kill)%g_tmp attr0.y
- //! v1: %r_tmp = v_interp_p1_f32 (kill)%bx, %pm:m0 attr0.x
- //! v1: %r = v_interp_p2_f32 (kill)%by, (kill)%pm:m0, (kill)%r_tmp attr0.x
+ //! v1: %b_tmp = v_interp_p1_f32 (kill)%bx, %pm:m0 attr0.z
+ //! v1: %b = v_interp_p2_f32 (kill)%by, (kill)%pm:m0, (kill)%b_tmp attr0.z
//! exp (kill)%r, (kill)%g, (kill)%b, (kill)%a mrt0
out_color = in_color;
}
diff --git a/src/compiler/nir/nir_opt_move.c b/src/compiler/nir/nir_opt_move.c
index 8abdaf364f5..97baf4567e4 100644
--- a/src/compiler/nir/nir_opt_move.c
+++ b/src/compiler/nir/nir_opt_move.c
@@ -52,92 +52,72 @@
*/
static bool
-move_source(nir_src *src, nir_block *block, nir_instr *before, nir_move_options options)
+nir_opt_move_block(nir_block *block, nir_move_options options)
{
- if (!src->is_ssa)
- return false;
-
- nir_instr *src_instr = src->ssa->parent_instr;
-
- if (src_instr->block == block && nir_can_move_instr(src_instr, options)) {
- exec_node_remove(&src_instr->node);
-
- if (before)
- exec_node_insert_node_before(&before->node, &src_instr->node);
- else
- exec_list_push_tail(&block->instr_list, &src_instr->node);
+ bool progress = false;
+ nir_instr *last_instr = nir_block_ends_in_jump(block) ?
+ nir_block_last_instr(block) : NULL;
+ const nir_if *iff = nir_block_get_following_if(block);
+ const nir_instr *if_cond_instr = iff ? iff->condition.parent_instr : NULL;
+
+ /* Walk the instructions backwards.
+ * The instructions get indexed while iterating.
+ * For each instruction which can be moved, find the earliest user
+ * and insert the instruction before it.
+ * If multiple instructions have the same user,
+ * the original order is kept.
+ */
+ unsigned index = 1;
+ nir_foreach_instr_reverse_safe(instr, block) {
+ instr->index = index++;
- return true;
- }
- return false;
-}
+ /* Check if this instruction can be moved downwards */
+ if (!nir_can_move_instr(instr, options))
+ continue;
-struct source_cb_data {
- bool *progress;
- nir_move_options options;
-};
+ /* Check all users in this block which is the first */
+ const nir_ssa_def *def = nir_instr_ssa_def(instr);
+ nir_instr *first_user = instr == if_cond_instr ? NULL : last_instr;
+ nir_foreach_use(use, def) {
+ nir_instr *parent = use->parent_instr;
+ if (parent->type == nir_instr_type_phi || parent->block != block)
+ continue;
+ if (!first_user || parent->index > first_user->index)
+ first_user = parent;
+ }
-static bool
-move_source_cb(nir_src *src, void *data_ptr)
-{
- struct source_cb_data data = *(struct source_cb_data*)data_ptr;
+ if (first_user) {
+ /* Check predecessor instructions for the same index to keep the order */
+ while (nir_instr_prev(first_user)->index == first_user->index)
+ first_user = nir_instr_prev(first_user);
- nir_instr *instr = src->parent_instr;
- if (move_source(src, instr->block, instr, data.options))
- *data.progress = true;
+ /* check if the user is already the immediate successor */
+ if (nir_instr_prev(first_user) == instr)
+ continue;
- return true; /* nir_foreach_src should keep going */
-}
+ /* Insert the instruction before it's first user */
+ exec_node_remove(&instr->node);
+ instr->index = first_user->index;
+ exec_node_insert_node_before(&first_user->node, &instr->node);
+ progress = true;
+ continue;
+ }
-static bool
-move(nir_block *block, nir_move_options options)
-{
- bool progress = false;
+ /* No user was found in this block:
+ * This instruction will be moved to the end of the block.
+ */
+ assert(nir_block_last_instr(block)->type != nir_instr_type_jump);
+ if (instr == nir_block_last_instr(block))
+ continue;
- /* We use a simple approach: walk instructions backwards.
- *
- * If the instruction's source is a comparison from the same block,
- * simply move it here. This may break SSA if it's used earlier in
- * the block as well. However, as we walk backwards, we'll find the
- * earlier use and move it again, further up. It eventually ends up
- * dominating all uses again, restoring SSA form.
- *
- * Before walking instructions, we consider the if-condition at the
- * end of the block, if one exists. It's effectively a use at the
- * bottom of the block.
- */
- nir_if *iff = nir_block_get_following_if(block);
- if (iff) {
- progress |= move_source(&iff->condition, block, NULL, options);
- }
+ exec_node_remove(&instr->node);
+ instr->index = 0;
+ exec_list_push_tail(&block->instr_list, &instr->node);
- nir_foreach_instr_reverse(instr, block) {
- /* The sources of phi instructions happen after the predecessor block
- * but before this block. (Yes, that's between blocks). This means
- * that we don't need to move them in order for them to be correct.
- * We could move them to encourage comparisons that are used in a phi to
- * the end of the block, doing so correctly would make the pass
- * substantially more complicated and wouldn't gain us anything since
- * the phi can't use a flag value anyway.
- */
+ /* update last_instr */
+ last_instr = instr;
- if (instr->type == nir_instr_type_phi) {
- /* We're going backwards so everything else is a phi too */
- break;
- } else if (instr->type == nir_instr_type_alu) {
- /* Walk ALU instruction sources backwards so that bcsel's boolean
- * condition is processed last for when comparisons are being moved.
- */
- nir_alu_instr *alu = nir_instr_as_alu(instr);
- for (int i = nir_op_infos[alu->op].num_inputs - 1; i >= 0; i--) {
- progress |= move_source(&alu->src[i].src, block, instr, options);
- }
- } else {
- struct source_cb_data data;
- data.progress = &progress;
- data.options = options;
- nir_foreach_src(instr, move_source_cb, &data);
- }
+ progress = true;
}
return progress;
@@ -154,7 +134,7 @@ nir_opt_move(nir_shader *shader, nir_move_options options)
bool impl_progress = false;
nir_foreach_block(block, func->impl) {
- if (move(block, options))
+ if (nir_opt_move_block(block, options))
impl_progress = true;
}
More information about the mesa-commit
mailing list