[Mesa-dev] [PATCH] i965/fs: Handle non-const sample number in interpolateAtSample

Neil Roberts neil at linux.intel.com
Tue Jul 21 09:44:23 PDT 2015


If a non-const sample number is given to interpolateAtSample it will
now generate an indirect send message with the sample ID similar to
how non-const sampler array indexing works. Previously non-const
values were ignored and instead it ended up using a constant 0 value.

Note that unlike sampler array indexing I can't find any mention in
the GLSL spec about the sample ID having to be dynamically uniform.
That wouldn't be handled by this patch and I'm not sure how that could
be implemented unless we do something like sending a separate message
for each fragment (ie, 16 messages in SIMD16).

---
I've added a Piglit test for this here:

http://patchwork.freedesktop.org/patch/55028/

 src/mesa/drivers/dri/i965/brw_eu.h             |  2 +-
 src/mesa/drivers/dri/i965/brw_eu_emit.c        | 34 +++++++++++++++++---------
 src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  5 ++--
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp       | 23 ++++++++++++++---
 4 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h
index 761aa0e..0ac1ad9 100644
--- a/src/mesa/drivers/dri/i965/brw_eu.h
+++ b/src/mesa/drivers/dri/i965/brw_eu.h
@@ -461,7 +461,7 @@ brw_pixel_interpolator_query(struct brw_codegen *p,
                              struct brw_reg mrf,
                              bool noperspective,
                              unsigned mode,
-                             unsigned data,
+                             struct brw_reg data,
                              unsigned msg_length,
                              unsigned response_length);
 
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 4d39762..25524d4 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -3192,26 +3192,38 @@ brw_pixel_interpolator_query(struct brw_codegen *p,
                              struct brw_reg mrf,
                              bool noperspective,
                              unsigned mode,
-                             unsigned data,
+                             struct brw_reg data,
                              unsigned msg_length,
                              unsigned response_length)
 {
    const struct brw_device_info *devinfo = p->devinfo;
-   struct brw_inst *insn = next_insn(p, BRW_OPCODE_SEND);
+   struct brw_inst *insn;
+   uint16_t exec_size;
 
-   brw_set_dest(p, insn, dest);
-   brw_set_src0(p, insn, mrf);
-   brw_set_message_descriptor(p, insn, GEN7_SFID_PIXEL_INTERPOLATOR,
-                              msg_length, response_length,
-                              false /* header is never present for PI */,
-                              false);
+   if (data.file == BRW_IMMEDIATE_VALUE) {
+      insn = next_insn(p, BRW_OPCODE_SEND);
+      brw_set_dest(p, insn, dest);
+      brw_set_src0(p, insn, mrf);
+      brw_set_message_descriptor(p, insn, GEN7_SFID_PIXEL_INTERPOLATOR,
+                                 msg_length, response_length,
+                                 false /* header is never present for PI */,
+                                 false);
+      brw_inst_set_pi_message_data(devinfo, insn, data.dw1.ud);
+   } else {
+      insn = brw_send_indirect_message(p,
+                                       GEN7_SFID_PIXEL_INTERPOLATOR,
+                                       dest,
+                                       mrf,
+                                       vec1(data));
+      brw_inst_set_mlen(devinfo, insn, msg_length);
+      brw_inst_set_rlen(devinfo, insn, response_length);
+   }
 
-   brw_inst_set_pi_simd_mode(
-         devinfo, insn, brw_inst_exec_size(devinfo, insn) == BRW_EXECUTE_16);
+   exec_size = brw_inst_exec_size(devinfo, p->current);
+   brw_inst_set_pi_simd_mode(devinfo, insn, exec_size == BRW_EXECUTE_16);
    brw_inst_set_pi_slot_group(devinfo, insn, 0); /* zero unless 32/64px dispatch */
    brw_inst_set_pi_nopersp(devinfo, insn, noperspective);
    brw_inst_set_pi_message_type(devinfo, insn, mode);
-   brw_inst_set_pi_message_data(devinfo, insn, data);
 }
 
 void
diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index bae7216..3c92341 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -1324,15 +1324,14 @@ fs_generator::generate_pixel_interpolator_query(fs_inst *inst,
                                                 struct brw_reg msg_data,
                                                 unsigned msg_type)
 {
-   assert(msg_data.file == BRW_IMMEDIATE_VALUE &&
-          msg_data.type == BRW_REGISTER_TYPE_UD);
+   assert(msg_data.type == BRW_REGISTER_TYPE_UD);
 
    brw_pixel_interpolator_query(p,
          retype(dst, BRW_REGISTER_TYPE_UW),
          src,
          inst->pi_noperspective,
          msg_type,
-         msg_data.dw1.ud,
+         msg_data,
          inst->mlen,
          inst->regs_written);
 }
diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 12cd453..6e979d5 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1482,12 +1482,27 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
          break;
 
       case nir_intrinsic_interp_var_at_sample: {
-         /* XXX: We should probably handle non-constant sample id's */
+         fs_reg sample_id_reg;
          nir_const_value *const_sample = nir_src_as_const_value(instr->src[0]);
-         assert(const_sample);
-         unsigned msg_data = const_sample ? const_sample->i[0] << 4 : 0;
+
+         if (const_sample) {
+            unsigned msg_data = const_sample->i[0] << 4;
+            sample_id_reg = fs_reg(msg_data);
+         } else {
+            /* XXX: This doesn't handle calls where the sample number isn't
+             * dynamically uniform. As far as I can tell that would be allowed
+             * by the GLSL spec but it would be quite tricky to implement.
+             */
+            fs_reg sample_src = retype(get_nir_src(instr->src[0]),
+                                       BRW_REGISTER_TYPE_UD);
+            sample_id_reg = vgrf(glsl_type::uint_type);
+            bld.SHL(sample_id_reg, sample_src, fs_reg(4u));
+            bld.emit_uniformize(sample_id_reg, sample_id_reg);
+         }
+
          inst = bld.emit(FS_OPCODE_INTERPOLATE_AT_SAMPLE, dst_xy, src,
-                         fs_reg(msg_data));
+                         sample_id_reg);
+
          break;
       }
 
-- 
1.9.3



More information about the mesa-dev mailing list