[Mesa-dev] [PATCH 10/11] i965/vs: Simplify the scratch write handling and emit less code.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> 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.

Eep, you're right.  But I also want to avoid asking for channels that
weren't set up in the temp.  This will require more fiddly shifting bits
(with a writemask of .yz, the constructor gives us .yzzz swizzle when we
want yyzz or zyzz or something).

> BAD_FILE?  How can this work?

I made the emit check for a null reg as the indication to not set up an
implied move.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20121010/d56820bf/attachment.pgp>