[Mesa-dev] [PATCH 18/24] i965/fs: Don't mutate multi-component arguments in sampler payload set-up.

Francisco Jerez currojerez at riseup.net
Fri May 27 03:46:23 UTC 2016


The Gen5+ sampler message payload construction code steps through the
coordinate and derivative components by induction like 'coordinate =
offset(coordinate, bld, 1)', the problem is that while doing that it
may step one past the end of the coordinate vector causing an
assertion failure in offset() if it happens to be a (single component)
immediate.  Right now coordinates and derivatives are typically passed
as actual registers but that will no longer be the case when we start
propagating constants into logical messages.

Instead express coordinate components in closed form like
'offset(coordinate, bld, i)' -- The end result seems slightly more
readable that way and it allows passing the coordinate and derivative
registers by const reference instead of by value, so it seems like a
clean-up in its own right.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp | 62 +++++++++++++++---------------------
 1 file changed, 25 insertions(+), 37 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 31ebbe3..e05977b 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -4022,9 +4022,9 @@ lower_sampler_logical_send_gen4(const fs_builder &bld, fs_inst *inst, opcode op,
 
 static void
 lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op,
-                                fs_reg coordinate,
+                                const fs_reg &coordinate,
                                 const fs_reg &shadow_c,
-                                fs_reg lod, fs_reg lod2,
+                                const fs_reg &lod, const fs_reg &lod2,
                                 const fs_reg &sample_index,
                                 const fs_reg &surface,
                                 const fs_reg &sampler,
@@ -4044,10 +4044,10 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op,
       message.nr--;
    }
 
-   for (unsigned i = 0; i < coord_components; i++) {
-      bld.MOV(retype(offset(msg_coords, bld, i), coordinate.type), coordinate);
-      coordinate = offset(coordinate, bld, 1);
-   }
+   for (unsigned i = 0; i < coord_components; i++)
+      bld.MOV(retype(offset(msg_coords, bld, i), coordinate.type),
+              offset(coordinate, bld, i));
+
    fs_reg msg_end = offset(msg_coords, bld, coord_components);
    fs_reg msg_lod = offset(msg_coords, bld, 4);
 
@@ -4076,12 +4076,10 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op,
        */
       msg_end = msg_lod;
       for (unsigned i = 0; i < grad_components; i++) {
-         bld.MOV(msg_end, lod);
-         lod = offset(lod, bld, 1);
+         bld.MOV(msg_end, offset(lod, bld, i));
          msg_end = offset(msg_end, bld, 1);
 
-         bld.MOV(msg_end, lod2);
-         lod2 = offset(lod2, bld, 1);
+         bld.MOV(msg_end, offset(lod2, bld, i));
          msg_end = offset(msg_end, bld, 1);
       }
       break;
@@ -4131,14 +4129,14 @@ is_high_sampler(const struct brw_device_info *devinfo, const fs_reg &sampler)
 
 static void
 lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
-                                fs_reg coordinate,
+                                const fs_reg &coordinate,
                                 const fs_reg &shadow_c,
-                                fs_reg lod, fs_reg lod2,
+                                fs_reg lod, const fs_reg &lod2,
                                 const fs_reg &sample_index,
                                 const fs_reg &mcs,
                                 const fs_reg &surface,
                                 const fs_reg &sampler,
-                                fs_reg offset_value,
+                                const fs_reg &offset_value,
                                 unsigned coord_components,
                                 unsigned grad_components)
 {
@@ -4210,21 +4208,15 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
        * [hdr], [ref], x, dPdx.x, dPdy.x, y, dPdx.y, dPdy.y, z, dPdx.z, dPdy.z
        */
       for (unsigned i = 0; i < coord_components; i++) {
-         bld.MOV(sources[length], coordinate);
-         coordinate = offset(coordinate, bld, 1);
+         bld.MOV(sources[length], offset(coordinate, bld, i));
          length++;
 
          /* For cube map array, the coordinate is (u,v,r,ai) but there are
           * only derivatives for (u, v, r).
           */
          if (i < grad_components) {
-            bld.MOV(sources[length], lod);
-            lod = offset(lod, bld, 1);
-            length++;
-
-            bld.MOV(sources[length], lod2);
-            lod2 = offset(lod2, bld, 1);
-            length++;
+            bld.MOV(sources[length++], offset(lod, bld, i));
+            bld.MOV(sources[length++], offset(lod2, bld, i));
          }
       }
 
@@ -4239,13 +4231,12 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
        * On Gen9 they are u, v, lod, r
        */
       bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate);
-      coordinate = offset(coordinate, bld, 1);
       length++;
 
       if (devinfo->gen >= 9) {
          if (coord_components >= 2) {
-            bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate);
-            coordinate = offset(coordinate, bld, 1);
+            bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D),
+                    offset(coordinate, bld, 1));
          }
          length++;
       }
@@ -4254,8 +4245,8 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
       length++;
 
       for (unsigned i = devinfo->gen >= 9 ? 2 : 1; i < coord_components; i++) {
-         bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate);
-         coordinate = offset(coordinate, bld, 1);
+         bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D),
+                 offset(coordinate, bld, i));
          length++;
       }
 
@@ -4293,8 +4284,8 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
        * texture coordinates.
        */
       for (unsigned i = 0; i < coord_components; i++) {
-         bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate);
-         coordinate = offset(coordinate, bld, 1);
+         bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D),
+                 offset(coordinate, bld, i));
          length++;
       }
 
@@ -4306,20 +4297,18 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
 
       /* More crazy intermixing */
       for (unsigned i = 0; i < 2; i++) { /* u, v */
-         bld.MOV(sources[length], coordinate);
-         coordinate = offset(coordinate, bld, 1);
+         bld.MOV(sources[length], offset(coordinate, bld, i));
          length++;
       }
 
       for (unsigned i = 0; i < 2; i++) { /* offu, offv */
-         bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), offset_value);
-         offset_value = offset(offset_value, bld, 1);
+         bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D),
+                 offset(offset_value, bld, i));
          length++;
       }
 
       if (coord_components == 3) { /* r if present */
-         bld.MOV(sources[length], coordinate);
-         coordinate = offset(coordinate, bld, 1);
+         bld.MOV(sources[length], offset(coordinate, bld, 2));
          length++;
       }
 
@@ -4332,8 +4321,7 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
    /* Set up the coordinate (except for cases where it was done above) */
    if (!coordinate_done) {
       for (unsigned i = 0; i < coord_components; i++) {
-         bld.MOV(sources[length], coordinate);
-         coordinate = offset(coordinate, bld, 1);
+         bld.MOV(sources[length], offset(coordinate, bld, i));
          length++;
       }
    }
-- 
2.7.3



More information about the mesa-dev mailing list