Bug 96254

Summary: [softpipe] piglit unsized-array-not-in-last-position regression
Product: Mesa Reporter: Vinson Lee <vlee>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact: mesa-dev
Severity: normal    
Priority: medium CC: airlied, siglesias
Version: 12.0Keywords: bisected, regression
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Bug Depends on: 96228    
Bug Blocks:    

Description Vinson Lee 2016-05-27 23:35:47 UTC
mesa: fb2a5ceb3264123e94a5e3f4d92cf6ec605e76e8 (master 11.3.0-devel)

$ ./bin/glslparsertest tests/spec/arb_shader_storage_buffer_object/compiler/unsized-array-not-in-last-position.frag fail 1.20 GL_ARB_shader_storage_buffer_object
Successfully compiled fragment shader tests/spec/arb_shader_storage_buffer_object/compiler/unsized-array-not-in-last-position.frag: 
Shader source:
// [config]
// expect_result: fail
// glsl_version: 1.20
// require_extensions: GL_ARB_shader_storage_buffer_object
// [end config]

#version 120
#extension GL_ARB_shader_storage_buffer_object: require

/* From the GL_ARB_shader_storage_buffer_object spec:
 *
 *     "In a shader storage block, the last member may be declared without an
 *     explicit size."
 */

buffer a {
	vec4 b;
	int c[];
	float d;
};

vec4 foo(void) {
	return b;
}

PIGLIT: {"result": "fail" }


5b2675093e863a52b610f112884ae12d42513770 is the first bad commit
commit 5b2675093e863a52b610f112884ae12d42513770
Author: Dave Airlie <airlied@redhat.com>
Date:   Wed May 25 13:31:41 2016 +1000

    glsl: handle implicit sized arrays in ssbo
    
    The current code disallows unsized arrays except at the end of
    an SSBO but it is a bit overzealous in doing so.
    
    struct a {
    	int b[];
    	int f[4];
    };
    
    is valid as long as b is implicitly sized within the shader,
    i.e. it is accessed only by integer indices.
    
    I've submitted some piglit tests to test for this.
    
    This also has no regressions on piglit on my Haswell.
    This fixes:
    GL45-CTS.shader_storage_buffer_object.basic-syntax
    GL45-CTS.shader_storage_buffer_object.basic-syntaxSSO
    
    This patch moves a chunk of the linker code down, so
    that we don't link the uniform blocks until after we've
    merged all the variables. The logic went something like:
    
    Removing the checks for last ssbo member unsized from
    the compiler and into the linker, meant doing the check
    in the link_uniform_blocks code. However to do that the
    array sizing had to happen first, so we knew that the
    only unsized arrays were in the last block. But array
    sizing required the variable to be merged, otherwise
    you'd get two different array sizes in different
    version of two variables, and one would get lost
    when merged. So the solution was to move array sizing
    up, after variable merging, but before uniform block
    visiting.
    
    Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
    Signed-off-by: Dave Airlie <airlied@redhat.com>

:040000 040000 86192aa213e706ebd8ac097144a34364c95c7233 b0fece865f8c1a2b6a1b656f628a4cd98b58b336 M	src
bisect run success
Comment 1 Samuel Iglesias Gonsálvez 2016-05-31 06:07:14 UTC
As per Dave Airlie, piglit.spec.arb_shader_storage_buffer_object.compiler.unsized-array-not-in-last-position is a bad test and will be removed.
Comment 2 Vinson Lee 2017-03-31 23:40:41 UTC
commit 3368b4778153aa3bd042c74b25fb2f80065d9771
Author: Dave Airlie <airlied@redhat.com>
Date:   Wed May 25 13:26:42 2016 +1000

    arb_shader_storage_buffer_object: test unsized vs implicit arrays.
    This tests the difference between an unsized and an implicitly sized array
    
    This removes a compiler test as this will be a linker error now.
    
    The rules are you can have a [] array as long as the shader
    later implicitly sizes it.
    
    Reviewed-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
    Signed-off-by: Dave Airlie <airlied@redhat.com>

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.