Mesa (main): glsl: Remove EmitNoLoops and the associated lower_jumps(lower_break=true) code.

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu May 5 23:11:15 UTC 2022


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

Author: Emma Anholt <emma at anholt.net>
Date:   Mon Apr 11 22:15:46 2022 -0700

glsl: Remove EmitNoLoops and the associated lower_jumps(lower_break=true) code.

Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8044>

---

 src/compiler/glsl/glsl_parser_extras.cpp           |   5 +-
 src/compiler/glsl/ir_optimization.h                |   2 +-
 .../glsl/lower_blend_equation_advanced.cpp         |   2 +-
 src/compiler/glsl/lower_jumps.cpp                  | 105 +----------------
 src/compiler/glsl/test_optpass.cpp                 |   7 +-
 src/compiler/glsl/tests/lower_jump_cases.py        | 130 +--------------------
 src/mesa/main/consts_exts.h                        |   1 -
 src/mesa/state_tracker/st_extensions.c             |  12 +-
 8 files changed, 19 insertions(+), 245 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
index cf21902324c..4576d076582 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -2420,7 +2420,7 @@ do_common_optimization(exec_list *ir, bool linked,
    OPT(do_rebalance_tree, ir);
    OPT(do_algebraic, ir, native_integers, options);
    OPT(do_lower_jumps, ir, true, true, options->EmitNoMainReturn,
-       options->EmitNoCont, options->EmitNoLoops);
+       options->EmitNoCont);
    OPT(do_vec_index_to_swizzle, ir);
    OPT(lower_vector_insert, ir, false);
    OPT(optimize_swizzles, ir);
@@ -2463,8 +2463,7 @@ do_common_optimization(exec_list *ir, bool linked,
              */
             loop_progress |= do_lower_jumps(ir, true, true,
                                             options->EmitNoMainReturn,
-                                            options->EmitNoCont,
-                                            options->EmitNoLoops);
+                                            options->EmitNoCont);
          }
          progress |= loop_progress;
       }
diff --git a/src/compiler/glsl/ir_optimization.h b/src/compiler/glsl/ir_optimization.h
index 1d5816d4952..d40617510ba 100644
--- a/src/compiler/glsl/ir_optimization.h
+++ b/src/compiler/glsl/ir_optimization.h
@@ -114,7 +114,7 @@ bool do_dead_code_unlinked(exec_list *instructions);
 bool do_dead_functions(exec_list *instructions);
 bool opt_flip_matrices(exec_list *instructions);
 bool do_function_inlining(exec_list *instructions);
-bool do_lower_jumps(exec_list *instructions, bool pull_out_jumps = true, bool lower_sub_return = true, bool lower_main_return = false, bool lower_continue = false, bool lower_break = false);
+bool do_lower_jumps(exec_list *instructions, bool pull_out_jumps = true, bool lower_sub_return = true, bool lower_main_return = false, bool lower_continue = false);
 bool do_if_simplification(exec_list *instructions);
 bool opt_flatten_nested_if_blocks(exec_list *instructions);
 bool do_mat_op_to_vec(exec_list *instructions);
diff --git a/src/compiler/glsl/lower_blend_equation_advanced.cpp b/src/compiler/glsl/lower_blend_equation_advanced.cpp
index 4564c204e42..7eebffdea7e 100644
--- a/src/compiler/glsl/lower_blend_equation_advanced.cpp
+++ b/src/compiler/glsl/lower_blend_equation_advanced.cpp
@@ -471,7 +471,7 @@ lower_blend_equation_advanced(struct gl_linked_shader *sh, bool coherent)
    /* Lower early returns in main() so there's a single exit point
     * where we can insert our lowering code.
     */
-   do_lower_jumps(sh->ir, false, false, true, false, false);
+   do_lower_jumps(sh->ir, false, false, true, false);
 
    void *mem_ctx = ralloc_parent(sh->ir);
 
diff --git a/src/compiler/glsl/lower_jumps.cpp b/src/compiler/glsl/lower_jumps.cpp
index 389f5847b3e..fec51f7e7cc 100644
--- a/src/compiler/glsl/lower_jumps.cpp
+++ b/src/compiler/glsl/lower_jumps.cpp
@@ -169,7 +169,6 @@ struct loop_record
 
    bool may_set_return_flag;
 
-   ir_variable* break_flag;
    ir_variable* execute_flag; /* cleared to emulate continue */
 
    loop_record(ir_function_signature* p_signature = 0, ir_loop* p_loop = 0)
@@ -179,7 +178,6 @@ struct loop_record
       this->nesting_depth = 0;
       this->in_if_at_the_end_of_the_loop = false;
       this->may_set_return_flag = false;
-      this->break_flag = 0;
       this->execute_flag = 0;
    }
 
@@ -194,17 +192,6 @@ struct loop_record
       }
       return this->execute_flag;
    }
-
-   ir_variable* get_break_flag()
-   {
-      assert(this->loop);
-      if(!this->break_flag) {
-         this->break_flag = new(this->signature) ir_variable(glsl_type::bool_type, "break_flag", ir_var_temporary);
-         this->loop->insert_before(this->break_flag);
-         this->loop->insert_before(new(this->signature) ir_assignment(new(this->signature) ir_dereference_variable(break_flag), new(this->signature) ir_constant(false)));
-      }
-      return this->break_flag;
-   }
 };
 
 struct function_record
@@ -278,7 +265,6 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor {
 
    bool pull_out_jumps;
    bool lower_continue;
-   bool lower_break;
    bool lower_sub_return;
    bool lower_main_return;
 
@@ -286,7 +272,6 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor {
       : progress(false),
         pull_out_jumps(false),
         lower_continue(false),
-        lower_break(false),
         lower_sub_return(false),
         lower_main_return(false)
    {
@@ -350,49 +335,6 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor {
       ir->replace_with(new(ir) ir_loop_jump(ir_loop_jump::jump_break));
    }
 
-   /**
-    * Create the necessary instruction to replace a break instruction.
-    */
-   ir_instruction *create_lowered_break()
-   {
-      void *ctx = this->function.signature;
-      return new(ctx) ir_assignment(
-          new(ctx) ir_dereference_variable(this->loop.get_break_flag()),
-          new(ctx) ir_constant(true));
-   }
-
-   /**
-    * If the given instruction is a break, lower it to an instruction
-    * that sets the break flag, without consulting
-    * should_lower_jump().
-    *
-    * It is safe to pass NULL to this function.
-    */
-   void lower_break_unconditionally(ir_instruction *ir)
-   {
-      if (get_jump_strength(ir) != strength_break) {
-         return;
-      }
-      ir->replace_with(create_lowered_break());
-   }
-
-   /**
-    * If the block ends in a conditional or unconditional break, lower
-    * it, even though should_lower_jump() says it needn't be lowered.
-    */
-   void lower_final_breaks(exec_list *block)
-   {
-      ir_instruction *ir = (ir_instruction *) block->get_tail();
-      lower_break_unconditionally(ir);
-      ir_if *ir_if = ir->as_if();
-      if (ir_if) {
-          lower_break_unconditionally(
-              (ir_instruction *) ir_if->then_instructions.get_tail());
-          lower_break_unconditionally(
-              (ir_instruction *) ir_if->else_instructions.get_tail());
-      }
-   }
-
    virtual void visit(class ir_loop_jump * ir)
    {
       /* Eliminate all instructions after each one, since they are
@@ -477,13 +419,7 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor {
          lower = lower_continue;
          break;
       case strength_break:
-         assert(this->loop.loop);
-         /* never lower "canonical break" */
-         if(ir->get_next()->is_tail_sentinel() && (this->loop.nesting_depth == 0
-               || (this->loop.nesting_depth == 1 && this->loop.in_if_at_the_end_of_the_loop)))
-            lower = false;
-         else
-            lower = lower_break;
+         lower = false;
          break;
       case strength_return:
          /* never lower return at the end of a this->function */
@@ -658,19 +594,7 @@ retry: /* we get here if we put code after the if inside a branch */
             }
             this->progress = true;
          } else if(jump_strengths[lower] == strength_break) {
-            /* To lower a break, we create a break flag (if the loop
-             * doesn't have one already) and add an instruction that
-             * sets it.
-             *
-             * Then we proceed as we would for a continue statement
-             * (set the execute flag to false to prevent the rest of
-             * the loop body from executing).
-             *
-             * The visit() function for the loop will ensure that the
-             * break flag is checked after executing the loop body.
-             */
-            jumps[lower]->insert_before(create_lowered_break());
-            goto lower_continue;
+            unreachable("no lowering of breaks any more");
          } else if(jump_strengths[lower] == strength_continue) {
 lower_continue:
             /* To lower a continue, we create an execute flag (if the
@@ -888,28 +812,6 @@ lower_continue:
           */
       }
 
-      if(this->loop.break_flag) {
-         /* We only get here if we are lowering breaks */
-         assert (lower_break);
-
-         /* If a break flag was generated while visiting the body of
-          * the loop, then at least one break was lowered, so we need
-          * to generate an if statement at the end of the loop that
-          * does a "break" if the break flag is set.  The break we
-          * generate won't violate the CONTAINED_JUMPS_LOWERED
-          * postcondition, because should_lower_jump() always returns
-          * false for a break that happens at the end of a loop.
-          *
-          * However, if the loop already ends in a conditional or
-          * unconditional break, then we need to lower that break,
-          * because it won't be at the end of the loop anymore.
-          */
-         lower_final_breaks(&ir->body_instructions);
-
-         ir_if* break_if = new(ir) ir_if(new(ir) ir_dereference_variable(this->loop.break_flag));
-         break_if->then_instructions.push_tail(new(ir) ir_loop_jump(ir_loop_jump::jump_break));
-         ir->body_instructions.push_tail(break_if);
-      }
 
       /* If the body of the loop may set the return flag, then at
        * least one return was lowered to a break, so we need to ensure
@@ -1016,12 +918,11 @@ lower_continue:
 } /* anonymous namespace */
 
 bool
-do_lower_jumps(exec_list *instructions, bool pull_out_jumps, bool lower_sub_return, bool lower_main_return, bool lower_continue, bool lower_break)
+do_lower_jumps(exec_list *instructions, bool pull_out_jumps, bool lower_sub_return, bool lower_main_return, bool lower_continue)
 {
    ir_lower_jumps_visitor v;
    v.pull_out_jumps = pull_out_jumps;
    v.lower_continue = lower_continue;
-   v.lower_break = lower_break;
    v.lower_sub_return = lower_sub_return;
    v.lower_main_return = lower_main_return;
 
diff --git a/src/compiler/glsl/test_optpass.cpp b/src/compiler/glsl/test_optpass.cpp
index 72294cd4935..6fad23efa8e 100644
--- a/src/compiler/glsl/test_optpass.cpp
+++ b/src/compiler/glsl/test_optpass.cpp
@@ -61,7 +61,6 @@ do_optimization(struct exec_list *ir, const char *optimization,
    int int_1;
    int int_2;
    int int_3;
-   int int_4;
 
    if (sscanf(optimization, "do_common_optimization ( %d ) ", &int_0) == 1) {
       return do_common_optimization(ir, int_0 != 0, false, options, true);
@@ -88,10 +87,10 @@ do_optimization(struct exec_list *ir, const char *optimization,
    } else if (strcmp(optimization, "do_function_inlining") == 0) {
       return do_function_inlining(ir);
    } else if (sscanf(optimization,
-                     "do_lower_jumps ( %d , %d , %d , %d , %d ) ",
-                     &int_0, &int_1, &int_2, &int_3, &int_4) == 5) {
+                     "do_lower_jumps ( %d , %d , %d , %d ) ",
+                     &int_0, &int_1, &int_2, &int_3) == 4) {
       return do_lower_jumps(ir, int_0 != 0, int_1 != 0, int_2 != 0,
-                            int_3 != 0, int_4 != 0);
+                            int_3 != 0);
    } else if (strcmp(optimization, "do_if_simplification") == 0) {
       return do_if_simplification(ir);
    } else if (strcmp(optimization, "do_mat_op_to_vec") == 0) {
diff --git a/src/compiler/glsl/tests/lower_jump_cases.py b/src/compiler/glsl/tests/lower_jump_cases.py
index 1977f3a9b4f..1236ed69b2f 100644
--- a/src/compiler/glsl/tests/lower_jump_cases.py
+++ b/src/compiler/glsl/tests/lower_jump_cases.py
@@ -272,8 +272,7 @@ def bash_quote(*args):
 
 def create_test_case(input_sexp, expected_sexp, test_name,
                      pull_out_jumps=False, lower_sub_return=False,
-                     lower_main_return=False, lower_continue=False,
-                     lower_break=False):
+                     lower_main_return=False, lower_continue=False):
     """Create a test case that verifies that do_lower_jumps transforms
     the given code in the expected way.
     """
@@ -282,9 +281,9 @@ def create_test_case(input_sexp, expected_sexp, test_name,
     input_str = sexp_to_string(sort_decls(input_sexp))
     expected_output = sexp_to_string(sort_decls(expected_sexp)) # XXX: don't stringify this
     optimization = (
-        'do_lower_jumps({0:d}, {1:d}, {2:d}, {3:d}, {4:d})'.format(
+        'do_lower_jumps({0:d}, {1:d}, {2:d}, {3:d})'.format(
             pull_out_jumps, lower_sub_return, lower_main_return,
-            lower_continue, lower_break))
+            lower_continue))
 
     return (test_name, optimization, input_str, expected_output)
 
@@ -449,115 +448,6 @@ def test_lower_pulled_out_jump():
         input_sexp, expected_sexp, 'lower_pulled_out_jump',
         lower_main_return=True, pull_out_jumps=True)
 
-def test_lower_breaks_1():
-    """If a loop contains an unconditional break at the bottom of it, it should
-    not be lowered.
-    """
-    input_sexp = make_test_case('main', 'void', (
-            loop(assign_x('a', const_float(1)) +
-                 break_())
-            ))
-    expected_sexp = input_sexp
-    yield create_test_case(
-        input_sexp, expected_sexp, 'lower_breaks_1', lower_break=True)
-
-def test_lower_breaks_2():
-    """If a loop contains a conditional break at the bottom of it, it should
-    not be lowered if it is in the then-clause.
-    """
-    input_sexp = make_test_case('main', 'void', (
-            loop(assign_x('a', const_float(1)) +
-                 simple_if('b', break_()))
-            ))
-    expected_sexp = input_sexp
-    yield create_test_case(
-        input_sexp, expected_sexp, 'lower_breaks_2', lower_break=True)
-
-def test_lower_breaks_3():
-    """If a loop contains a conditional break at the bottom of it, it should
-    not be lowered if it is in the then-clause, even if there are statements
-    preceding the break.
-    """
-    input_sexp = make_test_case('main', 'void', (
-            loop(assign_x('a', const_float(1)) +
-                 simple_if('b', (assign_x('c', const_float(1)) +
-                                 break_())))
-            ))
-    expected_sexp = input_sexp
-    yield create_test_case(
-        input_sexp, expected_sexp, 'lower_breaks_3', lower_break=True)
-
-def test_lower_breaks_4():
-    """If a loop contains a conditional break at the bottom of it, it should
-    not be lowered if it is in the else-clause.
-    """
-    input_sexp = make_test_case('main', 'void', (
-            loop(assign_x('a', const_float(1)) +
-                 simple_if('b', [], break_()))
-            ))
-    expected_sexp = input_sexp
-    yield create_test_case(
-        input_sexp, expected_sexp, 'lower_breaks_4', lower_break=True)
-
-def test_lower_breaks_5():
-    """If a loop contains a conditional break at the bottom of it, it should
-    not be lowered if it is in the else-clause, even if there are statements
-    preceding the break.
-    """
-    input_sexp = make_test_case('main', 'void', (
-            loop(assign_x('a', const_float(1)) +
-                 simple_if('b', [], (assign_x('c', const_float(1)) +
-                                     break_())))
-            ))
-    expected_sexp = input_sexp
-    yield create_test_case(
-        input_sexp, expected_sexp, 'lower_breaks_5', lower_break=True)
-
-def test_lower_breaks_6():
-    """If a loop contains conditional breaks and continues, and ends in an
-    unconditional break, then the unconditional break needs to be lowered,
-    because it will no longer be at the end of the loop after the final break
-    is added.
-    """
-    input_sexp = make_test_case('main', 'void', (
-            loop(simple_if('a', (complex_if('b', continue_()) +
-                                 complex_if('c', break_()))) +
-                 break_())
-            ))
-    expected_sexp = make_test_case('main', 'void', (
-            declare_break_flag() +
-            loop(declare_execute_flag() +
-                 simple_if(
-                    'a',
-                    (complex_if('b', lowered_continue()) +
-                     if_execute_flag(
-                            complex_if('c', lowered_break())))) +
-                 if_execute_flag(lowered_break_simple()) +
-                 final_break())
-            ))
-    yield create_test_case(
-        input_sexp, expected_sexp, 'lower_breaks_6', lower_break=True,
-        lower_continue=True)
-
-def test_lower_guarded_conditional_break():
-    """Normally a conditional break at the end of a loop isn't lowered, however
-    if the conditional break gets placed inside an if(execute_flag) because of
-    earlier lowering of continues, then the break needs to be lowered.
-    """
-    input_sexp = make_test_case('main', 'void', (
-            loop(complex_if('a', continue_()) +
-                 simple_if('b', break_()))
-            ))
-    expected_sexp = make_test_case('main', 'void', (
-            declare_break_flag() +
-            loop(declare_execute_flag() +
-                 complex_if('a', lowered_continue()) +
-                 if_execute_flag(simple_if('b', lowered_break())) +
-                 final_break())
-            ))
-    yield create_test_case(
-        input_sexp, expected_sexp, 'lower_guarded_conditional_break',
-        lower_break=True, lower_continue=True)
 
 def test_remove_continue_at_end_of_loop():
     """Test that a redundant continue-statement at the end of a loop is
@@ -594,10 +484,7 @@ def test_lower_return_void_at_end_of_loop():
     yield create_test_case(
         input_sexp, expected_sexp, 'return_void_at_end_of_loop_lower_return',
         lower_main_return=True)
-    yield create_test_case(
-        input_sexp, expected_sexp,
-        'return_void_at_end_of_loop_lower_return_and_break',
-        lower_main_return=True, lower_break=True)
+
 
 def test_lower_return_non_void_at_end_of_loop():
     """Test that a non-void return at the end of a loop is properly lowered."""
@@ -626,15 +513,10 @@ def test_lower_return_non_void_at_end_of_loop():
     yield create_test_case(
         input_sexp, expected_sexp,
         'return_non_void_at_end_of_loop_lower_return', lower_sub_return=True)
-    yield create_test_case(
-        input_sexp, expected_sexp,
-        'return_non_void_at_end_of_loop_lower_return_and_break',
-        lower_sub_return=True, lower_break=True)
+
 
 CASES = [
-    test_lower_breaks_1, test_lower_breaks_2, test_lower_breaks_3,
-    test_lower_breaks_4, test_lower_breaks_5, test_lower_breaks_6,
-    test_lower_guarded_conditional_break, test_lower_pulled_out_jump,
+    test_lower_pulled_out_jump,
     test_lower_return_non_void_at_end_of_loop,
     test_lower_return_void_at_end_of_loop,
     test_lower_returns_1, test_lower_returns_2, test_lower_returns_3,
diff --git a/src/mesa/main/consts_exts.h b/src/mesa/main/consts_exts.h
index 20fc47af5ab..1de115b162e 100644
--- a/src/mesa/main/consts_exts.h
+++ b/src/mesa/main/consts_exts.h
@@ -309,7 +309,6 @@ struct gl_extensions
 struct gl_shader_compiler_options
 {
    /** Driver-selectable options: */
-   GLboolean EmitNoLoops;
    GLboolean EmitNoCont;                  /**< Emit CONT opcode? */
    GLboolean EmitNoMainReturn;            /**< Emit CONT/RET opcodes? */
    GLboolean EmitNoSat;                   /**< Emit SAT opcodes? */
diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
index 3409c5ce429..61707d7a5dc 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -349,15 +349,9 @@ void st_init_limits(struct pipe_screen *screen,
          can_ubo = false;
       }
 
-      if (options->EmitNoLoops)
-         options->MaxUnrollIterations =
-            MIN2(screen->get_shader_param(screen, sh,
-                                          PIPE_SHADER_CAP_MAX_INSTRUCTIONS),
-                 65536);
-      else
-         options->MaxUnrollIterations =
-            screen->get_shader_param(screen, sh,
-                                  PIPE_SHADER_CAP_MAX_UNROLL_ITERATIONS_HINT);
+      options->MaxUnrollIterations =
+         screen->get_shader_param(screen, sh,
+                              PIPE_SHADER_CAP_MAX_UNROLL_ITERATIONS_HINT);
 
       if (!screen->get_param(screen, PIPE_CAP_NIR_COMPACT_ARRAYS))
          options->LowerCombinedClipCullDistance = true;



More information about the mesa-commit mailing list