[Mesa-dev] [PATCH 1/2] gallivm: don't use control flow when doing indirect constant buffer lookups

[Roland Scheidegger]

I guess one day we might actually make use of specifying the size in
bytes despite currently requiring that size to be a multiple of vec4
size. It should not really be difficult to allow 4-byte offsets / sizes
since the constants are only fetched one element at a time anyway.
Essentially just the valid index test would need fixing (which right now
we don't even do for direct accesses anyway).

Roland


Am 25.05.2015 um 20:04 schrieb Ilia Mirkin:
> This is the code for setting the vs constbuf (in nv30_draw.c):
> 
>          void *map = nv04_resource(nv30->vertprog.constbuf)->data;
>          debug_printf("map: %p, nr: %d\n", map, nv30->vertprog.constbuf_nr);
>          draw_set_mapped_constant_buffer(draw, PIPE_SHADER_VERTEX, 0,
>                                          map, nv30->vertprog.constbuf_nr);
> 
> This prints:
> 
> map: 0x2559d00, nr: 12
> 
> Does that seem wrong in any way? Hm... the size should probably be *4*4... oops.
> 
> Yeah that works much better :) Thanks for the hint!
> 
> On Mon, May 25, 2015 at 1:42 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> The only thing I can think of is an illegal setup of the constant
>> buffer, because the index was not verified before for direct accesses,
>> which made this work before even for "illegal" sizes.
>> This is still not verified but buffers which get rounded down to size
>> zero are now definitely no longer working.
>> See https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D90081&d=BQIFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=So3De-3GAtjCtBtZOpF8oYR4n1lIkXcHKCbKAEJCROs&s=ftOq5P_zbWHOny52p98bnshRg9tQHFBiX_OTj1Wm2DY&e= 
>>
>> I'm still not actually sure (tried to get some feedback without much
>> success on the list) if a bind buffer range with offset properly aligned
>> but size not should actually work correctly, hence still don't know if
>> there's actually something wrong at all (though technically it would
>> probably be safer if the index is also verified for direct accesses).
>> You can however trivially test that theory by removing the code which
>> replaces zero-sized buffers with dummy ones (which is just there so we
>> don't crash when trying to access them).
>>
>> Roland
>>
>>
>>
>> Am 24.05.2015 um 06:42 schrieb Ilia Mirkin:
>>> Hi Roland,
>>>
>>> I've just bisected a nv30 swtnl regression to this commit. When
>>> running the nv30 driver (on a NV44, if it matters) and forcing swtnl
>>> (NV30_SWTNL=1), glxgears from 60% broken to 100% broken. Now, I'm not
>>> sure what the initial breakage is, but at least it shows a
>>> gears-looking thing some of the time. After this change, I just see a
>>> few random lines every 30 frames or so. Setting DRAW_USE_LLVM=0 makes
>>> the whole thing fail horribly before and after this change (nothing
>>> drawn at all).
>>>
>>> Do you think that it's likely coincidence that the LLVM path worked at
>>> all before this and that I should figure out what's not working with
>>> DRAW_USE_LLVM=0? Or can you think of some reason why this change may
>>> have broken the llvm path in a swtnl use-case with a driver like nv30
>>> (no gs, no integers)?
>>>
>>> Thanks,
>>>
>>>   -ilia
>>>