[Mesa-dev] [PATCH v2 16/19] i965/fs: Use the builder dispatch_width for computing register offsets

[Jason Ekstrand]

On Fri, Jun 26, 2015 at 11:51 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
>> On Fri, Jun 26, 2015 at 8:52 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> Jason Ekstrand <jason at jlekstrand.net> writes:
>>>
>>>> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>>>> ---
>>>>  src/mesa/drivers/dri/i965/brw_fs.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
>>>> index d4cc43d..d94a842 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>>>> @@ -72,7 +72,7 @@ offset(fs_reg reg, const brw::fs_builder& bld, unsigned delta)
>>>>     case MRF:
>>>>     case ATTR:
>>>>        return byte_offset(reg,
>>>> -                         delta * MAX2(reg.width * reg.stride, 1) *
>>>> +                         delta * bld.dispatch_width() * reg.stride *
>>>
>>> Er...  This doesn't look right for stride == 0.  If you keep the
>>> MAX2(.., 1) expression this patch is:
>>
>> I don't think offset() even makes sense for something with stride ==
>> 0.  I added "assert(stride != 0)" right above the byte_offset() call
>> and it passed Jenkins.  Would that be an acceptable alternative?
>
> stride == 0 implies that each logical component of your vector takes 1
> scalar component (because all the N channels of your SIMDN value are one
> and the same scalar in your register file), that means that logically
> independent components of a vector or array are stored 1 scalar apart,
> and the previous code was doing the right thing.

I still think offset() is bogus for stride == 0.  However, I don't
really feel like arguing the point, so I added the MAX2().
--Jason

>> --Jason
>>
>>> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>>>
>>>>                           type_sz(reg.type));
>>>>     case UNIFORM:
>>>>        reg.reg_offset += delta;
>>>> --
>>>> 2.4.3
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev