[Mesa-dev] Mesa (master): st/mesa: fix incorrect size of UBO declarations

[Brian Paul]

On 07/01/2014 08:43 PM, Michel Dänzer wrote:
> On 02.07.2014 00:43, Brian Paul wrote:
>> Module: Mesa
>> Branch: master
>> Commit: f4b0ab7afd83c811329211eae8167c9bf238870c
>> URL:    https://urldefense.proofpoint.com/v1/url?u=http://cgit.freedesktop.org/mesa/mesa/commit/?id%3Df4b0ab7afd83c811329211eae8167c9bf238870c&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=X5dPXUSLXb%2B60RW5%2FHCgcIkPXz083MgfZAbiRJFVOxc%3D%0A&s=c9766b9e3797ccad30888c534fae5891f9a7313418a228c81c0528230412cf1c
>>
>> Author: Brian Paul <brianp at vmware.com>
>> Date:   Tue Jul  1 08:17:09 2014 -0600
>>
>> st/mesa: fix incorrect size of UBO declarations
>>
>> UniformBufferSize is in bytes so we need to divide by 16 to get the
>> number of constant buffer slots.  Also, the ureg_DECL_constant2D()
>> function takes first..last parameters so we need to subtract one
>> for the last value.
>
> This change broke the GLSL uniform_buffer fs/vs/gs-struct-pad piglit
> tests with radeonsi:
>
> ../../../../src/gallium/auxiliary/gallivm/lp_bld_tgsi.c:306:lp_build_emit_fetch: Assertion `reg->Register.Index <= bld_base->info->file_max[reg->Register.File]' failed.
>
> AFAICT reg->Register.Index is 2, and the new code calls
> ureg_DECL_constant2D() with last=1. This is the TGSI code:
>
> FRAG
> PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
> DCL OUT[0], COLOR
> DCL CONST[1][0..1]
> DCL TEMP[0], LOCAL
> IMM[0] UINT32 {0, 16, 32, 0}
>    0: MOV TEMP[0].x, CONST[1][0].xxxx
>    1: MOV TEMP[0].y, CONST[1][1].xxxx
>    2: MOV TEMP[0].z, CONST[1][2].xxxx
>    3: MOV TEMP[0].w, CONST[1][2].xxxx
>    4: MOV OUT[0], TEMP[0]
>    5: END
>
> which results from this GLSL code:
>
> #version 140
>
> struct S1 {
>          float r;
> };
>
> struct S2 {
>          float g;
>          float b;
>          float a;
> };
>
> struct S {
>         S1 s1;
>         S2 s2;
> };
>
> uniform ubo1 {
>          S s;
> };
>
> void main()
> {
>          gl_FragColor = vec4(s.s1.r, s.s2.g, s.s2.b, s.s2.a);
> }
>
>
> Something doesn't seem to add up, but I'm not sure where exactly the
> problem is or how to fix it.


I think there's at least a one GLSL compiler bug exposed here.

 From the code fragments above, the z and w components of TEMP[0] are 
getting the same value when they should not.

I reverted Mesa to before my changes and modified the piglit test to use 
different values for the blue/alpha channels and it fails (nvidia 
passes).  We were just getting lucky before my changes.

I suspect the code which is laying out the floats in the UBO is doing 
something wrong, and the size of the UBO is miscalculated as a result.

I'll dig a bit more but I think someone more familiar with the compiler 
will need to help.

-Brian