On 10/04/2012 04:07 PM, Eric Anholt wrote:
We're always doing it for an immediately preceding instruction, and we can
make that instruction write directly into the MRF instead of using a
temporary.

Along with reducing instructions, it avoids confusing the upcoming live
variables code (which saw our use of all channels of the temporary that had
only one channel populated, and assumes the register was live at the start of
the basic block, i.e. main()).
---
  src/mesa/drivers/dri/i965/brw_vec4.h               |    2 --
  src/mesa/drivers/dri/i965/brw_vec4_emit.cpp        |    8 ++++---
  .../drivers/dri/i965/brw_vec4_reg_allocate.cpp     |   11 +---------
  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp     |   23 +++++++++-----------
  4 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h 
b/src/mesa/drivers/dri/i965/brw_vec4.h
index cd05462..fe70300 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -419,8 +419,6 @@ public:
                          src_reg orig_src,
                          int base_offset);
     void emit_scratch_write(vec4_instruction *inst,
-                          src_reg temp,
-                          dst_reg orig_dst,
                           int base_offset);
     void emit_pull_constant_load(vec4_instruction *inst,
                                dst_reg dst,
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
index 479b0a6..340841f 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
@@ -588,9 +588,11 @@ vec4_visitor::generate_scratch_write(vec4_instruction 
*inst,
     generate_oword_dual_block_offsets(brw_message_reg(inst->base_mrf + 1),
                                     index);

-   brw_MOV(p,
-          retype(brw_message_reg(inst->base_mrf + 2), BRW_REGISTER_TYPE_D),
-          retype(src, BRW_REGISTER_TYPE_D));
+   if (src.file != BRW_ARCHITECTURE_REGISTER_FILE || src.nr != BRW_ARF_NULL) {
+      brw_MOV(p,
+              retype(brw_message_reg(inst->base_mrf + 2), BRW_REGISTER_TYPE_D),
+              retype(src, BRW_REGISTER_TYPE_D));
+   }

     uint32_t msg_type;

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
index 5529794..79966e8 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
@@ -346,16 +346,7 @@ vec4_visitor::spill_reg(int spill_reg_nr)
        }

        if (inst->dst.file == GRF && inst->dst.reg == spill_reg_nr) {
-         dst_reg spill_reg = inst->dst;
-         inst->dst.reg = virtual_grf_alloc(1, 4);
-
-         /* We don't want a swizzle when reading from the source; read the
-          * whole register and use spill_reg's writemask to select which
-          * channels to write.
-          */
-         src_reg temp = src_reg(inst->dst);
-         temp.swizzle = BRW_SWIZZLE_XYZW;

I don't understand how you can remove this. You need to write out inst->dst, but non-swizzled. (Otherwise you re-swizzle your swizzles and get the wrong channels...)

Enable the "Go spill everything" debugging on brw_vec4_emit.cpp:817 and then run:

./bin/shader_runner tests/shaders/glsl-vs-masked-cos.shader_test -auto

Your patch causes an assertion failure, which is a regression:
brw_eu_emit.c:1672: brw_math: Assertion `dest.file == 1' failed.

-         emit_scratch_write(inst, temp, spill_reg, spill_offset);
+         emit_scratch_write(inst, spill_offset);
        }
     }

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 762f4c1..1dfdcce 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -2468,19 +2468,23 @@ vec4_visitor::emit_scratch_read(vec4_instruction *inst,
   */
  void
  vec4_visitor::emit_scratch_write(vec4_instruction *inst,
-                                src_reg temp, dst_reg orig_dst,
                                 int base_offset)
  {
-   int reg_offset = base_offset + orig_dst.reg_offset;
-   src_reg index = get_scratch_offset(inst, orig_dst.reladdr, reg_offset);
+   int reg_offset = base_offset + inst->dst.reg_offset;
+   src_reg index = get_scratch_offset(inst, inst->dst.reladdr, reg_offset);

     dst_reg dst = dst_reg(brw_writemask(brw_vec8_grf(0, 0),
-                                      orig_dst.writemask));
-   vec4_instruction *write = SCRATCH_WRITE(dst, temp, index);
+                                      inst->dst.writemask));
+   vec4_instruction *write = SCRATCH_WRITE(dst, src_reg(), index);

BAD_FILE?  How can this work?

     write->predicate = inst->predicate;
     write->ir = inst->ir;
     write->annotation = inst->annotation;
     inst->insert_after(write);
+
+   inst->dst.file = MRF;
+   inst->dst.reg = write->base_mrf + 2;
+   inst->dst.reg_offset = 0;
+   inst->dst.reladdr = NULL;
  }

  /**
@@ -2535,14 +2539,7 @@ vec4_visitor::move_grf_array_access_to_scratch()
        current_annotation = inst->annotation;

        if (inst->dst.file == GRF && scratch_loc[inst->dst.reg] != -1) {
-        src_reg temp = src_reg(this, glsl_type::vec4_type);
-
-        emit_scratch_write(inst, temp, inst->dst, scratch_loc[inst->dst.reg]);
-
-        inst->dst.file = temp.file;
-        inst->dst.reg = temp.reg;
-        inst->dst.reg_offset = temp.reg_offset;
-        inst->dst.reladdr = NULL;
+        emit_scratch_write(inst, scratch_loc[inst->dst.reg]);
        }

        for (int i = 0 ; i < 3; i++) {


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to