Mesa (staging/21.3): nir/loop_unroll: Always unroll loops that iterate at most once

[GitLab Mirror]

Module: Mesa
Branch: staging/21.3
Commit: 7b5d0803766ecd7cf254957bf0e18545042a6db2
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=7b5d0803766ecd7cf254957bf0e18545042a6db2

Author: Ian Romanick <ian.d.romanick at intel.com>
Date:   Tue Oct 12 13:06:04 2021 -0700

nir/loop_unroll: Always unroll loops that iterate at most once

Two carchase compute shaders (shader-db) and two Fallout 4 fragment
shaders (fossil-db) were helped.  Based on the NIR of the shaders, all
four had structures like

    for (i = 0; i < 1; i++) {
        ...

	for (...) {
            ...
	}
    }

All HSW+ platforms had similar results. (Ice Lake shown)
total loops in shared programs: 6033 -> 6031 (-0.03%)
loops in affected programs: 4 -> 2 (-50.00%)
helped: 2
HURT: 0

All Intel platforms had similar results. (Ice Lake shown)
Instructions in all programs: 143692018 -> 143692006 (-0.0%)
SENDs in all programs: 6947154 -> 6947154 (+0.0%)
Loops in all programs: 38285 -> 38283 (-0.0%)
Cycles in all programs: 8434822225 -> 8434476815 (-0.0%)
Spills in all programs: 191665 -> 191665 (+0.0%)
Fills in all programs: 298822 -> 298822 (+0.0%)

In the presense of loop unrolling like this, the change in cycles is not
accurate.

v2: Rearrange the logic in the if-condition to read a little better.
Suggested by Tim.

Closes: #5089
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
(cherry picked from commit ae99ea6f4ddb90f7120b4986709a9e9064037f94)

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13366>

---

 .pick_status.json                      |  2 +-
 src/compiler/nir/nir_opt_loop_unroll.c | 22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index 02acab772fe..43ac7726390 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -94,7 +94,7 @@
         "description": "nir/loop_unroll: Always unroll loops that iterate at most once",
         "nominated": false,
         "nomination_type": null,
-        "resolution": 4,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": null
     },
diff --git a/src/compiler/nir/nir_opt_loop_unroll.c b/src/compiler/nir/nir_opt_loop_unroll.c
index 3dec5e15e56..e81cebbb338 100644
--- a/src/compiler/nir/nir_opt_loop_unroll.c
+++ b/src/compiler/nir/nir_opt_loop_unroll.c
@@ -976,7 +976,27 @@ process_loops(nir_shader *sh, nir_cf_node *cf_node, bool *has_nested_loop_out,
          }
       }
 
-      if (has_nested_loop || !loop->info->limiting_terminator)
+      /* Intentionally don't consider exact_trip_count_known here.  When
+       * max_trip_count is non-zero, it is the upper bound on the number of
+       * times the loop will iterate, but the loop may iterate less.  For
+       * example, the following loop will iterate 0 or 1 time:
+       *
+       *    for (i = 0; i < min(x, 1); i++) { ... }
+       *
+       * Trivial single-interation loops (e.g., do { ... } while (false)) and
+       * trivial zero-iteration loops (e.g., while (false) { ... }) will have
+       * already been handled.
+       *
+       * If the loop is known to execute at most once and meets the other
+       * unrolling criteria, unroll it even if it has nested loops.
+       *
+       * It is unlikely that such loops exist in real shaders. GraphicsFuzz is
+       * known to generate spurious loops that iterate exactly once.  It is
+       * plausible that it could eventually start generating loops like the
+       * example above, so it seems logical to defend against it now.
+       */
+      if (!loop->info->limiting_terminator ||
+          (loop->info->max_trip_count != 1 && has_nested_loop))
          goto exit;
 
       if (!check_unrolling_restrictions(sh, loop))