nir/lower_tex: Rework invalid implicit LOD lowering
authorJason Ekstrand <jason@jlekstrand.net>
Fri, 9 Jul 2021 16:34:23 +0000 (11:34 -0500)
committerMarge Bot <eric+marge@anholt.net>
Fri, 23 Jul 2021 15:53:57 +0000 (15:53 +0000)
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@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11775>

src/compiler/nir/nir.h
src/compiler/nir/nir_lower_tex.c
src/gallium/auxiliary/gallivm/lp_bld_nir.c
src/intel/compiler/brw_nir.c
src/panfrost/bifrost/bifrost_compile.c

index e1bd3e8..75a4a5f 100644 (file)
@@ -5020,12 +5020,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.
     */
    bool lower_txd_cube_map;
index 6a47851..d9e6f7e 100644 (file)
@@ -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)
@@ -1340,26 +1357,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;
index 042f103..8db4066 100644 (file)
@@ -2349,7 +2349,7 @@ void lp_build_opt_nir(struct nir_shader *nir)
       NIR_PASS(progress, nir, nir_opt_algebraic);
       NIR_PASS(progress, 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 = {
index 76bfa7d..dc09dcb 100644 (file)
@@ -831,7 +831,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_txd_3d = devinfo->verx10 >= 125,
       .lower_txb_shadow_clamp = true,
index 858808e..63b7a28 100644 (file)
@@ -3190,7 +3190,6 @@ bi_optimize_nir(nir_shader *nir, unsigned gpu_id, 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,
         };