[Mesa-dev] [PATCH] i965: Respect stride and subreg_offset for ATTR registers

[Matt Turner]

On Wed, Sep 23, 2015 at 5:25 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Wed, Sep 23, 2015 at 5:11 PM, Kristian Høgsberg Kristensen
> <krh at bitplanet.net> wrote:
>> When we assign hw regs to attributes, we don't incorporate the stride
>> and subreg_offset from the fs_reg. It's rarely used, but the integer
>> multiplication lowering uses unusual stride and subreg_offset
>> combination breaks when one source is an attribute.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91970
>> Cc: "11.0" <mesa-stable at lists.freedesktop.org>
>> Signed-off-by: Kristian Høgsberg Kristensen <krh at bitplanet.net>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 225a312..618bbd2 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -1564,7 +1564,10 @@ fs_visitor::assign_vs_urb_setup()
>>
>>              inst->src[i].file = HW_REG;
>>              inst->src[i].fixed_hw_reg =
>> -               retype(brw_vec8_grf(grf, 0), inst->src[i].type);
>> +               stride(byte_offset(retype(brw_vec8_grf(grf, 0), inst->src[i].type),
>> +                                  inst->src[i].subreg_offset),
>> +                      inst->exec_size * inst->src[i].stride,
>> +                      inst->exec_size, inst->src[i].stride);
>
> I made this comment to Kristian today in the office, but I figure I'll
> also put it on the list:
>
> I'm not 100% convinced that I like having subreg offsets and strides
> respected on the ATTR.  We do respect subreg offset on the UNIFORM
> file so I guess there's some precedent for it.  If we didn't, another
> option would be to simply emit a MOV to a grf during lowering if we
> ever detect ATTR.  Given that this is only on CHV and probably not hit
> that often, I think that would be my minor preference.  That said,
> this patch does work.

IIRC you discussed uniforms in this context with Curro on-list and he
wasn't in favor of it, but I may be misremembering.

I don't understand what benefit would be gained by ignoring subreg
offset (and I don't see that it's stated anywhere).