Mesa (staging/21.1): nir/lower_tex: Rework invalid implicit LOD lowering

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Sat Jul 24 20:32:48 UTC 2021


Module: Mesa
Branch: staging/21.1
Commit: 16b21db99e40e2a259b252a2e120ace2722849bb
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=16b21db99e40e2a259b252a2e120ace2722849bb

Author: Jason Ekstrand <jason at jlekstrand.net>
Date:   Fri Jul  9 11:34:23 2021 -0500

nir/lower_tex: Rework invalid implicit LOD lowering

Only fragment and some compute shaders support implicit derivatives.
They're totally meaningless without helper invocations and some
understanding of the dispatch pattern.  We've got code to lower
nir_texop_tex in these shader stages to use an explicit derivative of 0
but it was pretty badly broken:

 1. It only handled nir_texop_tex, not nir_texop_txb or nir_texop_lod.

 2. It didn't take min_lod into account

 3. It was conflated with adding a missing LOD parameter to opcodes
    which expect one such as nir_texop_txf.  While not really a bug,
    this does make it way harder to reason about the code.

 4. Unless you set a flag (which most drivers don't), it left the
    opcode nir_texop_tex instead of nir_texop_txl which it should have
    been.

This reworks it to go through roughly the same path as other LOD
lowering only with a constant lod of 0 instead of calling out to
nir_texop_lod.  We also get rid of the lower_tex_without_implicit_lod
flag because most drivers set it and those that don't are probably
subtly broken.  If someone really wants to get nir_texop_tex in their
vertex shaders, they can write a new patch to add the flag back in.

Fixes: e382890e25c0 "nir: set default lod to texture opcodes that..."
Fixes: d5ac5d6e836f "nir: Add option to lower tex to txl when..."
Reviewed-by: Connor Abbott <cwabbott0 at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11775>
(cherry picked from commit 74ec2b12be17a7796186b3100a5a6b208be45b23)

---

 .pick_status.json                          |  2 +-
 src/compiler/nir/nir.h                     |  6 ----
 src/compiler/nir/nir_lower_tex.c           | 52 ++++++++++++++++++++++--------
 src/gallium/auxiliary/gallivm/lp_bld_nir.c |  2 +-
 src/intel/compiler/brw_nir.c               |  1 -
 src/panfrost/bifrost/bifrost_compile.c     |  1 -
 6 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index b8d82cd940b..edc97caa7b6 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -238,7 +238,7 @@
         "description": "nir/lower_tex: Rework invalid implicit LOD lowering",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "e382890e25c0d95b0765ec00126f27dacc0e1da9"
     },
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index a0a646610db..64e22a456c5 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -4758,12 +4758,6 @@ typedef struct nir_lower_tex_options {
     */
    unsigned lower_srgb;
 
-   /**
-    * If true, lower nir_texop_tex on shaders that doesn't support implicit
-    * LODs to nir_texop_txl.
-    */
-   bool lower_tex_without_implicit_lod;
-
    /**
     * If true, lower nir_texop_txd on cube maps with nir_texop_txl.
     */
diff --git a/src/compiler/nir/nir_lower_tex.c b/src/compiler/nir/nir_lower_tex.c
index 51e6e000ddc..05b4df2dce4 100644
--- a/src/compiler/nir/nir_lower_tex.c
+++ b/src/compiler/nir/nir_lower_tex.c
@@ -230,17 +230,13 @@ lower_rect_tex_scale(nir_builder *b, nir_tex_instr *tex)
 }
 
 static void
-lower_implicit_lod(nir_builder *b, nir_tex_instr *tex)
+lower_lod(nir_builder *b, nir_tex_instr *tex, nir_ssa_def *lod)
 {
    assert(tex->op == nir_texop_tex || tex->op == nir_texop_txb);
    assert(nir_tex_instr_src_index(tex, nir_tex_src_lod) < 0);
    assert(nir_tex_instr_src_index(tex, nir_tex_src_ddx) < 0);
    assert(nir_tex_instr_src_index(tex, nir_tex_src_ddy) < 0);
 
-   b->cursor = nir_before_instr(&tex->instr);
-
-   nir_ssa_def *lod = nir_get_texture_lod(b, tex);
-
    int bias_idx = nir_tex_instr_src_index(tex, nir_tex_src_bias);
    if (bias_idx >= 0) {
       /* If we have a bias, add it in */
@@ -259,6 +255,27 @@ lower_implicit_lod(nir_builder *b, nir_tex_instr *tex)
    tex->op = nir_texop_txl;
 }
 
+static void
+lower_implicit_lod(nir_builder *b, nir_tex_instr *tex)
+{
+   b->cursor = nir_before_instr(&tex->instr);
+   lower_lod(b, tex, nir_get_texture_lod(b, tex));
+}
+
+static void
+lower_zero_lod(nir_builder *b, nir_tex_instr *tex)
+{
+   b->cursor = nir_before_instr(&tex->instr);
+
+   if (tex->op == nir_texop_lod) {
+      nir_ssa_def_rewrite_uses(&tex->dest.ssa, nir_imm_int(b, 0));
+      nir_instr_remove(&tex->instr);
+      return;
+   }
+
+   lower_lod(b, tex, nir_imm_int(b, 0));
+}
+
 static nir_ssa_def *
 sample_plane(nir_builder *b, nir_tex_instr *tex, int plane,
              const nir_lower_tex_options *options)
@@ -1275,26 +1292,33 @@ nir_lower_tex_block(nir_block *block, nir_builder *b,
          continue;
       }
 
-      bool shader_supports_implicit_lod =
-         b->shader->info.stage == MESA_SHADER_FRAGMENT ||
-         (b->shader->info.stage == MESA_SHADER_COMPUTE &&
-          b->shader->info.cs.derivative_group != DERIVATIVE_GROUP_NONE);
-
       /* TXF, TXS and TXL require a LOD but not everything we implement using those
        * three opcodes provides one.  Provide a default LOD of 0.
        */
       if ((nir_tex_instr_src_index(tex, nir_tex_src_lod) == -1) &&
           (tex->op == nir_texop_txf || tex->op == nir_texop_txs ||
-           tex->op == nir_texop_txl || tex->op == nir_texop_query_levels ||
-           (tex->op == nir_texop_tex && !shader_supports_implicit_lod))) {
+           tex->op == nir_texop_txl || tex->op == nir_texop_query_levels)) {
          b->cursor = nir_before_instr(&tex->instr);
          nir_tex_instr_add_src(tex, nir_tex_src_lod, nir_src_for_ssa(nir_imm_int(b, 0)));
-         if (tex->op == nir_texop_tex && options->lower_tex_without_implicit_lod)
-            tex->op = nir_texop_txl;
          progress = true;
          continue;
       }
 
+      /* Only fragment and compute (in some cases) support implicit
+       * derivatives.  Lower those opcodes which use implicit derivatives to
+       * use an explicit LOD of 0.
+       */
+      bool shader_supports_implicit_lod =
+         b->shader->info.stage == MESA_SHADER_FRAGMENT ||
+         (b->shader->info.stage == MESA_SHADER_COMPUTE &&
+          b->shader->info.cs.derivative_group != DERIVATIVE_GROUP_NONE);
+
+      if (nir_tex_instr_has_implicit_derivative(tex) &&
+          !shader_supports_implicit_lod) {
+         lower_zero_lod(b, tex);
+         progress = true;
+      }
+
       if (options->lower_txs_lod && tex->op == nir_texop_txs) {
          progress |= nir_lower_txs_lod(b, tex);
          continue;
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_nir.c b/src/gallium/auxiliary/gallivm/lp_bld_nir.c
index 38afac47de6..33e26b50162 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_nir.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_nir.c
@@ -2317,7 +2317,7 @@ void lp_build_opt_nir(struct nir_shader *nir)
       NIR_PASS_V(nir, nir_opt_algebraic);
       NIR_PASS_V(nir, nir_lower_pack);
 
-      nir_lower_tex_options options = { .lower_tex_without_implicit_lod = true };
+      nir_lower_tex_options options = { 0, };
       NIR_PASS_V(nir, nir_lower_tex, &options);
 
       const nir_lower_subgroups_options subgroups_options = {
diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index 8a1a6779c0e..988badc5f33 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -821,7 +821,6 @@ brw_preprocess_nir(const struct brw_compiler *compiler, nir_shader *nir,
       .lower_txp = ~0,
       .lower_txf_offset = true,
       .lower_rect_offset = true,
-      .lower_tex_without_implicit_lod = true,
       .lower_txd_cube_map = true,
       .lower_txb_shadow_clamp = true,
       .lower_txd_shadow_clamp = true,
diff --git a/src/panfrost/bifrost/bifrost_compile.c b/src/panfrost/bifrost/bifrost_compile.c
index a0c21ceef57..68c65557c65 100644
--- a/src/panfrost/bifrost/bifrost_compile.c
+++ b/src/panfrost/bifrost/bifrost_compile.c
@@ -2824,7 +2824,6 @@ bi_optimize_nir(nir_shader *nir, bool is_blend)
         nir_lower_tex_options lower_tex_options = {
                 .lower_txs_lod = true,
                 .lower_txp = ~0,
-                .lower_tex_without_implicit_lod = true,
                 .lower_tg4_broadcom_swizzle = true,
                 .lower_txd = true,
         };



More information about the mesa-commit mailing list