[Mesa-dev] [PATCH] nouveau: codegen: combineLd/St do not combine indirect loads

[Hans de Goede]

Hi,

On 21-04-16 16:28, Ilia Mirkin wrote:
> On Thu, Apr 21, 2016 at 9:55 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>> combineLd/St would combine, i.e. :
>>
>> st  u32 # g[$r2+0x0] $r2
>> st  u32 # g[$r2+0x4] $r3
>>
>> into:
>>
>> st  u64 # g[$r2+0x0] $r2d
>>
>> But this is only valid if r2 contains an 8 byte aligned address,
>> which is unknown.
>>
>> This commit checks for src0 dim 0 not being indirect when combining
>> loads / stores as combining indirect loads / stores may break alignment
>> rules.
>
> I believe the assumption is that all indirect addresses are 16-byte
> aligned. This works out for GL, I think. Although hm... I wonder what
> happens if you have a
>
> layout (std430) buffer foo {
>    int x[16];
> }
>
> And you access x[i], x[i+1], and i == 1. I think we end up doing a ton
> of size-based validation which might avoid the problem.
>
> My concern is that now constbufs will get the same treatment, and for
> constbufs the alignment is always 16 :(
>
> What do you think? Just drop those, or add extra conditionals to allow
> it for constbufs?

I'm not sure we've the alignment guarantee for constbufs, IIRC we lower
const buf accesses to be indirect because we want to provide more then 8 UBO-s,
right ? So we read the offset from NVC0_CB_AUX_BUF_INFO and then end up with e.g.:

ld u64  r2d c7[r1+0x0]

Where r1 contains the offset of the user-buf. But what if the user is somehow
indirectly accessing the userbuf, then we will have added that indirect offset
to r1, and we can no longer assume that we can safely merge the loads without
breaking alignment rules.

I hope I'm making sense here, I'm still a bit unsure about the details how
this all works.

Regards,

Hans