[Mesa-dev] [PATCH 10/23] i965/fs: Remove fs_reg::sechalf.

[Francisco Jerez]

Paul Berry <stereotype441 at gmail.com> writes:

> On 2 December 2013 11:31, Francisco Jerez <currojerez at riseup.net> wrote:
>
>> +/**
>> + * Get either of the 8-component halves of a 16-component register.
>> + */
>> +static inline fs_reg
>> +half(const fs_reg &reg, unsigned idx)
>> +{
>> +   assert(idx == 0 || (reg.file != HW_REG && reg.file != IMM));
>> +   return byte_offset(reg, 8 * idx * reg.stride * type_sz(reg.type));
>> +}
>> +
>>
>
> I'd like to see a comment explaining that it's safe to call this function
> on a register with 32 bits per component, since brw_reg.h's byte_offset()
> handles byte offsets greater than 32 by incrementing the register number.
>
I've added a comment on the allowed range of fs_reg::subreg_offset here
[1].  Do you think that's clear enough?

Thanks.

> Also, for sanity's sake, it would be nice for this function to include the
> assertion:
>
>    assert(idx < 2);
>
> With those changes, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com>

[1] http://cgit.freedesktop.org/~currojerez/mesa/commit/?h=image-load-store&id=14ec9dfd42fec1ee5e5e7f48f98a36fe620cf4e6
-------------- 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/34bffd27/attachment-0001.pgp>