Paul Berry <stereotype...@gmail.com> writes:
> On 2 December 2013 11:31, Francisco Jerez <curroje...@riseup.net> wrote:
>
>> +/**
>> + * Get either of the 8-component halves of a 16-component register.
>> + */
>> +static inline fs_reg
>> +half(const fs_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 <stereotype...@gmail.com> [1] http://cgit.freedesktop.org/~currojerez/mesa/commit/?h=image-load-store&id=14ec9dfd42fec1ee5e5e7f48f98a36fe620cf4e6
pgpx2YHF727V9.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
