[Mesa-dev] [PATCH 13/23] i965/fs: Take into account reg_offset consistently for MRF regs.

[Francisco Jerez]

Paul Berry <stereotype441 at gmail.com> writes:

> On 2 December 2013 11:31, Francisco Jerez <currojerez at riseup.net> wrote:
>
>> Until now it was only being taken into account in the VEC4 back-end
>> but not in the FS back-end.  Do it in both cases.
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.h             |  2 +-
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 10 ++++++----
>>  src/mesa/drivers/dri/i965/brw_shader.h         |  7 ++++---
>>  3 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
>> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 2c36d9f..f918f7e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -615,4 +615,4 @@ bool brw_do_channel_expressions(struct exec_list
>> *instructions);
>>  bool brw_do_vector_splitting(struct exec_list *instructions);
>>  bool brw_fs_precompile(struct gl_context *ctx, struct gl_shader_program
>> *prog);
>>
>> -struct brw_reg brw_reg_from_fs_reg(fs_reg *reg);
>> +struct brw_reg brw_reg_from_fs_reg(fs_reg *reg, unsigned dispatch_width);
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index 8d310a1..1de59eb 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -981,8 +981,9 @@ static uint32_t brw_file_from_reg(fs_reg *reg)
>>  }
>>
>>  struct brw_reg
>> -brw_reg_from_fs_reg(fs_reg *reg)
>> +brw_reg_from_fs_reg(fs_reg *reg, unsigned dispatch_width)
>>  {
>> +   const int reg_size = 4 * dispatch_width;
>>
>
> What happens when reg.type is UW and dispatch_width is 16?  In that case,
> we would compute reg_size == 64, but the correct value seems like it's
> actually 32 in this case.
>
> Are we perhaps relying on reg.type being a 32-bit type?  If so, maybe we
> should add an assertion:
>
>    assert(type_sz(reg.type) == 4);
>

Nope, "reg_size" is supposed to be the size in bytes of a ::reg_offset
unit, i.e. one hardware register in SIMD8 mode and two hardware
registers in SIMD16 mode as the comment at the definition of
::reg_offset explains.  The fixed factor of four is intentional and
correct no matter what the register type is.

Thanks.

> With that added, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140115/34fb55f8/attachment.pgp>