Bug 92822

Tim has reported that the following (sub)tests are new, not regressions:

piglit.spec.glsl-1_10.execution.varying-packing.simple uvec3 arrays_of_arrays
piglit.spec.glsl-1_10.execution.varying-packing.simple ivec3 arrays_of_arrays
piglit.spec.arb_program_interface_query.arb_program_interface_query-getprogramresourceindex

However, subsequent testing has shown that the following tests are *intermittent* on BDW only:

arb_arrays_of_arrays.execution.image_store.basic-imagestore-non-const-uniform-index
arb_arrays_of_arrays.execution.image_store.basic-imagestore-const-uniform-index


These tests are disabled for that platform until this bug is resolved.

1/3 problems reported in this bug report are now fixed.

Fixed:

piglit.spec.glsl-1_10.execution.varying-packing.simple uvec3 arrays_of_arrays
piglit.spec.glsl-1_10.execution.varying-packing.simple ivec3 arrays_of_arrays

This was a problem with the piglit test and has been fixed.

<tarceri> [13:30:26] janesma, I still have 92822 on my list but as I said to
          Ian here when he marked it as a blocker for 11.1 its not a
          regression
<tarceri> [13:30:55] I don't see why it should be a blocker for the 11.1
          release
<tarceri> [13:34:14] There are two issues 1. a bug with image store on bdw
          which also happens on the non arrays of arrays tests 2. A bug with
          arb_program_interface_query for arrays of arrays which again is not
          a regression just a bug
<tarceri> [13:34:27] with new functionality 
<tarceri> [13:36:37] Unfortunately the fix for 2. is not as simple as it
          seems, issue 1. I have been unable to reproduce but is clearly a bug
          with image store rather than AoA

The following intermittent failures are very likely to be bugs in the piglit tests rather than in Mesa:

 arb_arrays_of_arrays.execution.image_store.basic-imagestore-non-const-uniform-index
 arb_arrays_of_arrays.execution.image_store.basic-imagestore-const-uniform-index

Neither of them runs glMemoryBarrier() before reading from the image they render to, so their behaviour is unspecified.

I haven't looked into the ARB_program_interface_query bug in detail, which is likely unrelated, but the above failures shouldn't be considered a blocker for the Mesa 11.1 release.

(In reply to Francisco Jerez from comment #4)
> The following intermittent failures are very likely to be bugs in the piglit
> tests rather than in Mesa:
> 
>  arb_arrays_of_arrays.execution.image_store.basic-imagestore-non-const-
> uniform-index
>  arb_arrays_of_arrays.execution.image_store.basic-imagestore-const-uniform-
> index
> 
> Neither of them runs glMemoryBarrier() before reading from the image they
> render to, so their behaviour is unspecified.
> 

Thanks for taking a look at this. I've sent a piglit series to try fix this here:

http://patchwork.freedesktop.org/series/1663/

If this looks correct then I will need to push and get Mark to check that this resolves the issue as I can't reproduce it on my BDW.

The patch seems to have resolved the issue with spec.arb_arrays_of_arrays.execution.image_store.basic-imagestore-non-const-uniform-index

(In reply to Mark Janes from comment #6)
> The patch seems to have resolved the issue with
> spec.arb_arrays_of_arrays.execution.image_store.basic-imagestore-non-const-
> uniform-index

Thanks for testing Mark.

I have pushed these patches to the piglit repo so the issue with these tests should be fixed as of feeefb1b47b758.

That makes 2/3 issues being tracked in this bug fixed.

Only piglit.spec.arb_program_interface_query.arb_program_interface_query-getprogramresourceindex remains to be fixed.

This one requires a Mesa fix, there are two options:

1.
Create a lowering pass the turns all arrays of arrays into seperate single dimensions arrays.

For example an AoA:  
in my_aoa[2][2][2];

Would become:
in my_aoa[0][0][2];
in my_aoa[0][1][2];
in my_aoa[1][0][2];
in my_aoa[1][1][2];

Where the outer two subscripts are now just part of the variable name rather than being functional. This will also allow inactive arrays to be easily found.

However it will be a bit of work to get this correct as indirects will need to be handled somehow as well as things such as assignments of whole AoA.
Also a bunch of clean-up would be needed as its a major change in the way AoA are currently handled after the initial compile/linking validation is complete.

2.
The other simpler option would be to process the existing IR and store the values we need for arb_program_interface_query in a newly allocated piece of memory used just for interface querys.

removing release tracker references.

Based on the commentary in this bug, the remaining piglit failure is a new test, not a regression.  This bug should not block releases.

*** Bug 107639 has been marked as a duplicate of this bug. ***

Hi all,

The solution for the last issue is suggested:
https://patchwork.freedesktop.org/patch/246791/

and test extensions which helped me to find some issues in my patch:
https://patchwork.freedesktop.org/patch/246789/

Regards,
Andrii.

Just to update this bug report. I ended up suggesting against the solution proposed by Andrii because it was too much code for something that didn't actually remove the unused components, but just hid them from the resource list.

I think if we actually want to fix this properly then we could do it by finally hooking up a NIR linker for GLSL.

For example after the linking (i.e. when we produce the resource list) the GLSL IR is converted to this NIR form:

shader: MESA_SHADER_VERTEX
name: GLSL3
inputs: 0
outputs: 0
uniforms: 0
shared: 0
decl_var shader_in INTERP_MODE_NONE vec4[2][2] vs_input2 (VERT_ATTRIB_GENERIC8, 0, 0)
decl_var shader_in INTERP_MODE_NONE vec4[2][2][2] vs_input3 (VERT_ATTRIB_GENERIC0, 0, 0)
decl_var shader_out INTERP_MODE_NONE vec4 gl_Position (VARYING_SLOT_POS, 0, 0)
decl_function main (0 params)

impl main {
	block block_0:
	/* preds: */
	vec1 32 ssa_0 = deref_var &vs_input2 (shader_in vec4[2][2]) 
	vec1 32 ssa_1 = load_const (0x00000000 /* 0.000000 */)
	vec1 32 ssa_2 = deref_array &(*ssa_0)[0] (shader_in vec4[2]) /* &vs_input2[0] */
	vec1 32 ssa_3 = deref_array &(*ssa_2)[0] (shader_in vec4) /* &vs_input2[0][0] */
	vec4 32 ssa_4 = intrinsic load_deref (ssa_3) (0) /* access=0 */
	vec1 32 ssa_5 = deref_var &vs_input3 (shader_in vec4[2][2][2]) 
	vec1 32 ssa_6 = deref_array &(*ssa_5)[0] (shader_in vec4[2][2]) /* &vs_input3[0] */
	vec1 32 ssa_7 = deref_array &(*ssa_6)[0] (shader_in vec4[2]) /* &vs_input3[0][0] */
	vec1 32 ssa_8 = deref_array &(*ssa_7)[0] (shader_in vec4) /* &vs_input3[0][0][0] */
	vec4 32 ssa_9 = intrinsic load_deref (ssa_8) (0) /* access=0 */
	vec1 32 ssa_10 = fadd ssa_4.x, ssa_9.x
	vec1 32 ssa_11 = fadd ssa_4.y, ssa_9.y
	vec1 32 ssa_12 = fadd ssa_4.z, ssa_9.z
	vec1 32 ssa_13 = fadd ssa_4.w, ssa_9.w
	vec1 32 ssa_14 = deref_var &gl_Position (shader_out vec4) 
	vec4 32 ssa_15 = vec4 ssa_10, ssa_11, ssa_12, ssa_13
	intrinsic store_deref (ssa_14, ssa_15) (15, 0) /* wrmask=xyzw */ /* access=0 */
	/* succs: block_1 */
	block block_1:
}


But due to better NIR having better optimisations this ends up as:

shader: MESA_SHADER_VERTEX
name: GLSL3
inputs: 2
outputs: 1
uniforms: 0
shared: 0
decl_var shader_in INTERP_MODE_NONE vec4 vs_input2 (VERT_ATTRIB_GENERIC8, 4, 0)
decl_var shader_in INTERP_MODE_NONE vec4 vs_input3 (VERT_ATTRIB_GENERIC0, 0, 0)
decl_var shader_out INTERP_MODE_NONE vec4 gl_Position (VARYING_SLOT_POS, 0, 0)
decl_function main (0 params)

impl main {
	block block_0:
	/* preds: */
	vec1 32 ssa_0 = deref_var &vs_input2 (shader_in vec4) 
	vec4 32 ssa_1 = intrinsic load_deref (ssa_0) (0) /* access=0 */
	vec1 32 ssa_2 = deref_var &vs_input3 (shader_in vec4) 
	vec4 32 ssa_3 = intrinsic load_deref (ssa_2) (0) /* access=0 */
	vec1 32 ssa_4 = fadd ssa_1.x, ssa_3.x
	vec1 32 ssa_5 = fadd ssa_1.y, ssa_3.y
	vec1 32 ssa_6 = fadd ssa_1.z, ssa_3.z
	vec1 32 ssa_7 = fadd ssa_1.w, ssa_3.w
	vec1 32 ssa_8 = deref_var &gl_Position (shader_out vec4) 
	vec4 32 ssa_9 = vec4 ssa_4, ssa_5, ssa_6, ssa_7
	intrinsic store_deref (ssa_8, ssa_9) (15, 0) /* wrmask=xyzw */ /* access=0 */
	/* succs: block_1 */
	block block_1:
}


If we were to build the resource list from a NIR linker this piglit test would pass this test just like the Nvidia blob.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/807.