[Mesa-dev] [PATCH] i965/fs: Don't use the pixel interpolater for centroid interpolation

Neil Roberts neil at linux.intel.com
Tue Jun 30 07:02:05 PDT 2015


For centroid interpolation we can just directly use the values set up
in the shader payload instead of querying the pixel interpolator. To
do this we need to modify brw_compute_barycentric_interp_modes to
detect when interpolateAtCentroid is called.

This fixes the interpolateAtCentroid tests on SKL in Piglit but it is
probably cheating a bit because there still seems to be some
underlying problem with using the pixel interpolater and this patch
just avoids it. The other interpolateAt* tests are still failing.
---

I'm not really sure if we want this patch because although it does
make using interpolateAtCentroid more efficient I don't think we know
of anything that is actually using that so it's not really a priority
to optimise. It adds a bit of complexity to the compiler so maybe we
are better off without it? I'm mostly just posting it to gauge
opinion.

 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 42 ++++++++++++++++--------
 src/mesa/drivers/dri/i965/brw_wm.c       | 55 ++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 14 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 59081ea..fcde545 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1217,6 +1217,21 @@ fs_visitor::emit_percomp(const fs_builder &bld, const fs_inst &inst,
    }
 }
 
+/* For most messages, we need one reg of ignored data; the hardware requires
+ * mlen==1 even when there is no payload. in the per-slot offset case, we'll
+ * replace this with the proper source data.
+ */
+static void
+setup_pixel_interpolater_instruction(nir_intrinsic_instr *instr,
+                                     fs_inst *inst,
+                                     int mlen = 1)
+{
+      inst->mlen = mlen;
+      inst->regs_written = 2; /* 2 floats per slot returned */
+      inst->pi_noperspective = instr->variables[0]->var->data.interpolation ==
+                               INTERP_QUALIFIER_NOPERSPECTIVE;
+}
+
 void
 fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr)
 {
@@ -1469,19 +1484,21 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
 
       fs_reg dst_xy = bld.vgrf(BRW_REGISTER_TYPE_F, 2);
 
-      /* For most messages, we need one reg of ignored data; the hardware
-       * requires mlen==1 even when there is no payload. in the per-slot
-       * offset case, we'll replace this with the proper source data.
-       */
       fs_reg src = vgrf(glsl_type::float_type);
-      int mlen = 1;     /* one reg unless overriden */
       fs_inst *inst;
 
       switch (instr->intrinsic) {
-      case nir_intrinsic_interp_var_at_centroid:
-         inst = bld.emit(FS_OPCODE_INTERPOLATE_AT_CENTROID,
-                         dst_xy, src, fs_reg(0u));
+      case nir_intrinsic_interp_var_at_centroid: {
+         enum brw_wm_barycentric_interp_mode interp_mode;
+         if (instr->variables[0]->var->data.interpolation ==
+             INTERP_QUALIFIER_NOPERSPECTIVE)
+            interp_mode = BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
+         else
+            interp_mode = BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
+         uint8_t reg = payload.barycentric_coord_reg[interp_mode];
+         dst_xy = fs_reg(brw_vec16_grf(reg, 0));
          break;
+      }
 
       case nir_intrinsic_interp_var_at_sample: {
          /* XXX: We should probably handle non-constant sample id's */
@@ -1490,6 +1507,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
          unsigned msg_data = const_sample ? const_sample->i[0] << 4 : 0;
          inst = bld.emit(FS_OPCODE_INTERPOLATE_AT_SAMPLE, dst_xy, src,
                          fs_reg(msg_data));
+         setup_pixel_interpolater_instruction(instr, inst);
          break;
       }
 
@@ -1502,6 +1520,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
 
             inst = bld.emit(FS_OPCODE_INTERPOLATE_AT_SHARED_OFFSET, dst_xy, src,
                             fs_reg(off_x | (off_y << 4)));
+            setup_pixel_interpolater_instruction(instr, inst);
          } else {
             src = vgrf(glsl_type::ivec2_type);
             fs_reg offset_src = retype(get_nir_src(instr->src[0]),
@@ -1531,9 +1550,9 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
                            bld.SEL(offset(src, i), itemp, fs_reg(7)));
             }
 
-            mlen = 2;
             inst = bld.emit(FS_OPCODE_INTERPOLATE_AT_PER_SLOT_OFFSET, dst_xy, src,
                             fs_reg(0u));
+            setup_pixel_interpolater_instruction(instr, inst, 2);
          }
          break;
       }
@@ -1542,11 +1561,6 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
          unreachable("Invalid intrinsic");
       }
 
-      inst->mlen = mlen;
-      inst->regs_written = 2; /* 2 floats per slot returned */
-      inst->pi_noperspective = instr->variables[0]->var->data.interpolation ==
-                               INTERP_QUALIFIER_NOPERSPECTIVE;
-
       for (unsigned j = 0; j < instr->num_components; j++) {
          fs_reg src = interp_reg(instr->variables[0]->var->data.location, j);
          src.type = dest.type;
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
index 592a729..f7fe1e0 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -40,9 +40,62 @@
 #include "program/prog_parameter.h"
 #include "program/program.h"
 #include "intel_mipmap_tree.h"
+#include "brw_nir.h"
 
 #include "util/ralloc.h"
 
+static bool
+compute_modes_in_block(nir_block *block,
+                       void *state)
+{
+   unsigned *interp_modes = state;
+   nir_intrinsic_instr *intrin;
+   enum brw_wm_barycentric_interp_mode interp_mode;
+
+   nir_foreach_instr(block, instr) {
+      if (instr->type != nir_instr_type_intrinsic)
+         continue;
+
+      intrin = nir_instr_as_intrinsic(instr);
+
+      if (intrin->intrinsic != nir_intrinsic_interp_var_at_centroid)
+         continue;
+
+      if (intrin->variables[0]->var->data.interpolation ==
+          INTERP_QUALIFIER_NOPERSPECTIVE)
+         interp_mode = BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
+      else
+         interp_mode = BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
+
+      *interp_modes |= 1 << interp_mode;
+   }
+
+   return true;
+}
+
+/**
+ * Looks for calls to interpolateAtCentroid within the program and returns a
+ * mask of the additional interpolation modes that they require.
+ */
+static unsigned
+compute_interpolate_at_centroid_modes(const struct gl_fragment_program *fprog)
+{
+   unsigned interp_modes = 0;
+   struct nir_shader *shader = fprog->Base.nir;
+
+   if (shader == NULL)
+      return 0;
+
+   nir_foreach_overload(shader, overload) {
+      if (overload->impl == NULL)
+         continue;
+
+      nir_foreach_block(overload->impl, compute_modes_in_block, &interp_modes);
+   }
+
+   return interp_modes;
+}
+
 /**
  * Return a bitfield where bit n is set if barycentric interpolation mode n
  * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
@@ -114,6 +167,8 @@ brw_compute_barycentric_interp_modes(struct brw_context *brw,
       }
    }
 
+   barycentric_interp_modes |= compute_interpolate_at_centroid_modes(fprog);
+
    return barycentric_interp_modes;
 }
 
-- 
1.9.3



More information about the mesa-dev mailing list