Mesa (main): nir: Free the instructions in a DCE instr removal.

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 4118264643686715f16b2b611ff4b0ee5e85b4c4
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=4118264643686715f16b2b611ff4b0ee5e85b4c4

Author: Emma Anholt <emma at anholt.net>
Date:   Mon Jun 28 14:42:21 2021 -0700

nir: Free the instructions in a DCE instr removal.

No significant change in shader-db time (n=11), but should be a little win
for memory usage by the compiler.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11628>

---

 src/compiler/nir/nir.c                | 34 +++++++++++++++++++---------------
 src/compiler/nir/nir.h                |  2 +-
 src/compiler/nir/tests/core_tests.cpp |  8 ++++----
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
index d9757ec1b66..a5843ad01da 100644
--- a/src/compiler/nir/nir.c
+++ b/src/compiler/nir/nir.c
@@ -1070,7 +1070,7 @@ void nir_instr_remove_v(nir_instr *instr)
    }
 }
 
-static bool nir_instr_remove_and_dce_live_cb(nir_ssa_def *def, void *state)
+static bool nir_instr_free_and_dce_live_cb(nir_ssa_def *def, void *state)
 {
    bool *live = state;
 
@@ -1082,7 +1082,7 @@ static bool nir_instr_remove_and_dce_live_cb(nir_ssa_def *def, void *state)
    }
 }
 
-static bool nir_instr_remove_and_dce_is_live(nir_instr *instr)
+static bool nir_instr_free_and_dce_is_live(nir_instr *instr)
 {
    /* Note: don't have to worry about jumps because they don't have dests to
     * become unused.
@@ -1095,31 +1095,28 @@ static bool nir_instr_remove_and_dce_is_live(nir_instr *instr)
    }
 
    bool live = false;
-   nir_foreach_ssa_def(instr, nir_instr_remove_and_dce_live_cb, &live);
+   nir_foreach_ssa_def(instr, nir_instr_free_and_dce_live_cb, &live);
    return live;
 }
 
 /**
- * Removes an instruction and any SSA defs that it used that are now dead, returning a nir_cursor
- * where the instruction previously was.
+ * Frees an instruction and any SSA defs that it used that are now dead,
+ * returning a nir_cursor where the instruction previously was.
  */
 nir_cursor
-nir_instr_remove_and_dce(nir_instr *instr)
+nir_instr_free_and_dce(nir_instr *instr)
 {
    nir_instr_worklist *worklist = nir_instr_worklist_create();
 
    nir_instr_worklist_add_ssa_srcs(worklist, instr);
    nir_cursor c = nir_instr_remove(instr);
 
+   struct exec_list to_free;
+   exec_list_make_empty(&to_free);
+
    nir_instr *dce_instr;
    while ((dce_instr = nir_instr_worklist_pop_head(worklist))) {
-      /* Instrs can be in the worklist multiple times, so check
-       * that we haven't already removed this one.
-       */
-      if (exec_node_is_tail_sentinel(&dce_instr->node))
-         continue;
-
-      if (!nir_instr_remove_and_dce_is_live(dce_instr)) {
+      if (!nir_instr_free_and_dce_is_live(dce_instr)) {
          nir_instr_worklist_add_ssa_srcs(worklist, dce_instr);
 
          /* If we're removing the instr where our cursor is, then we have to
@@ -1131,9 +1128,16 @@ nir_instr_remove_and_dce(nir_instr *instr)
             c = nir_instr_remove(dce_instr);
          else
             nir_instr_remove(dce_instr);
+         exec_list_push_tail(&to_free, &dce_instr->node);
       }
    }
 
+   struct exec_node *node;
+   while ((node = exec_list_pop_head(&to_free))) {
+      nir_instr *removed_instr = exec_node_data(nir_instr, node, node);
+      ralloc_free(removed_instr);
+   }
+
    nir_instr_worklist_destroy(worklist);
 
    return c;
@@ -2003,7 +2007,7 @@ nir_function_impl_lower_instructions(nir_function_impl *impl,
             nir_if_rewrite_condition(use_src->parent_if, new_src);
 
          if (nir_ssa_def_is_unused(old_def)) {
-            iter = nir_instr_remove_and_dce(instr);
+            iter = nir_instr_free_and_dce(instr);
          } else {
             iter = nir_after_instr(instr);
          }
@@ -2017,7 +2021,7 @@ nir_function_impl_lower_instructions(nir_function_impl *impl,
          if (new_def == NIR_LOWER_INSTR_PROGRESS_REPLACE) {
             /* Only instructions without a return value can be removed like this */
             assert(!old_def);
-            iter = nir_instr_remove_and_dce(instr);
+            iter = nir_instr_free_and_dce(instr);
             progress = true;
          } else
             iter = nir_after_instr(instr);
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 5d123ab76f4..c4796179232 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -3951,7 +3951,7 @@ nir_instr_remove(nir_instr *instr)
    return cursor;
 }
 
-nir_cursor nir_instr_remove_and_dce(nir_instr *instr);
+nir_cursor nir_instr_free_and_dce(nir_instr *instr);
 
 /** @} */
 
diff --git a/src/compiler/nir/tests/core_tests.cpp b/src/compiler/nir/tests/core_tests.cpp
index b1c5355eda5..66bcca84d9d 100644
--- a/src/compiler/nir/tests/core_tests.cpp
+++ b/src/compiler/nir/tests/core_tests.cpp
@@ -91,14 +91,14 @@ nir_core_test::shader_contains_def(nir_ssa_def *def)
    return false;
 }
 
-TEST_F(nir_core_test, nir_instr_remove_and_dce_test)
+TEST_F(nir_core_test, nir_instr_free_and_dce_test)
 {
    nir_ssa_def *zero = nir_imm_int(b, 0);
    nir_ssa_def *one = nir_imm_int(b, 1);
    nir_ssa_def *add01 = nir_iadd(b, zero, one);
    nir_ssa_def *add11 = nir_iadd(b, one, one);
 
-   nir_cursor c = nir_instr_remove_and_dce(add01->parent_instr);
+   nir_cursor c = nir_instr_free_and_dce(add01->parent_instr);
    ASSERT_FALSE(shader_contains_def(add01));
    ASSERT_TRUE(shader_contains_def(add11));
    ASSERT_FALSE(shader_contains_def(zero));
@@ -109,12 +109,12 @@ TEST_F(nir_core_test, nir_instr_remove_and_dce_test)
    nir_validate_shader(b->shader, "after remove_and_dce");
 }
 
-TEST_F(nir_core_test, nir_instr_remove_and_dce_all_test)
+TEST_F(nir_core_test, nir_instr_free_and_dce_all_test)
 {
    nir_ssa_def *one = nir_imm_int(b, 1);
    nir_ssa_def *add = nir_iadd(b, one, one);
 
-   nir_cursor c = nir_instr_remove_and_dce(add->parent_instr);
+   nir_cursor c = nir_instr_free_and_dce(add->parent_instr);
    ASSERT_FALSE(shader_contains_def(add));
    ASSERT_FALSE(shader_contains_def(one));