Bug 41980

58a7461e1672935e7d30780a4dd40c00abbc28a5 is the first bad commit
commit 58a7461e1672935e7d30780a4dd40c00abbc28a5
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Sun Sep 11 16:17:21 2011 -0500

    glsl_to_tgsi: Use _mesa_generate_parameters_list_for_uniforms
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Bryan Cain <bryancain3@gmail.com>

:040000 040000 5ca026afa96678f4f4f87fa150b0533735c7e7d0 c4bafaedbfa257fb8fe9a6f443a08f439026398c M	src
bisect run success

And with swrast and i965 I'm seeing:

Mesa: User error: GL_INVALID_OPERATION in glUniform(type mismatch)
Getting array element 0 from base location...
index 0: got 0.000000, expected 12.000000
PIGLIT: {'result': 'fail' }

The error seems to be caused from param->DataType being zero instead of a GLSL type, like GL_FLOAT_VEC4.

I'm seeing this error in other apps that call glUniform too.

I think there may be two separate bugs here.  The weird thing is getuniform-01 passed when I originally sent 58a7461 out for review.

It looks like the write to 'color' gets eliminated from the vertex shader.  When the write to color gets eliminated, the only use of 'c' is removed, so the uniform is also eliminated.

In the Gallium case, the constant {0,0,0,1} gets embedded in the instruction and the parameters list is empty.  Since the list is empty, attempts to dereference it hit a NULL pointer.

In the non-Gallium case, the constant {0,0,0,1} is added to the parameters list.  Attempts to dereference the parameters list hit an entry (correctly) filled with zeros.

The bug seems to be that the Uniforms list has 'c' in the list, but 'c' has be removed from the linked shader.  Some behavior has changed here, but I think it changed outside the bisected patch.  I'll do some more digging.

I've posted some patches to the mesa-dev and piglit mailing lists to fix this bug:

http://lists.freedesktop.org/archives/mesa-dev/2011-October/013521.html
http://lists.freedesktop.org/archives/mesa-dev/2011-October/013522.html

and

http://lists.freedesktop.org/archives/piglit/2011-October/001097.html

The first patch fixes the bug.  The second patch gives the behavior that we want (eliminating uniforms whose uses are optimized away), and the third patch fixes the test to handle that behavior.

Fixed on master by the following sequence of commits.  This bug never existed on the stable branches, so there is no need to cherry pick anything back.

ommit 6437a71d4172273db670b959dd66e3b34c866962
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Mon Oct 24 12:23:50 2011 -0700

    ir_to_mesa: Use uniform_field_visitor to add all struct fields to parameter li
    
    Previously the uniform was passed as single, whole structure to
    _mesa_add_parameter.  This was completely bogus and resulted in a
    DataType of 0 (instead of a valid GLSL type enum).
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41980
    Tested-by: Brian Paul <brianp@vmware.com>
    Cc: Bryan Cain <bryancain3@gmail.com>
    Cc: Vinson Lee <vlee@vmware.com>
    Cc: José Fonseca <jfonseca@vmware.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

commit 747e59c7590bd11a4964c6ca12c5ff0dbb6282f2
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Mon Oct 24 19:37:11 2011 -0700

    linker: Add uniform_field_visitor class to process leaf fields of a uniform
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>

commit ca95593d49a2d99a16c160c2a04acc4be007d8a4
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Mon Oct 24 19:33:16 2011 -0700

    ralloc: Add new [v]asprintf_rewrite_tail functions.
    
    This can be useful if you want to create a bunch of temporary strings
    with a common prefix.  For example, when iterating over uniform
    structure fields, one might want to create temporary strings like
    "pallete.primary", "palette.outline", and "pallette.shadow".
    
    This could be done by overwriting the '.' with a null-byte and calling
    ralloc_asprintf_append, but that incurs the cost of strlen("pallete")
    every time...when this is already known.
    
    These new functions allow you rewrite the tail of the string, given a
    starting index.  If the starting index is the length of the string, this
    is equivalent to appending.
    
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

commit 960d722bf7db636863a05ddf9258236fccb58ecd
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Fri Oct 21 11:21:02 2011 -0700

    linker: Eliminate more dead code after demoting shader inputs and outputs
    
    Consider the following vertex shader and fragment shader:
    
    // vertex shader
    varying vec4 v;
    uniform vec4 u;
    
    void main() { gl_Position = vec4(0.0); v = u; }
    
    // fragment shader
    void main() { gl_FragColor = vec4(0.0); }
    
    Since the fragment shader does not use 'v', it is demoted from a
    varying to a simple global variable.  Once that happens, the
    assignment to 'v' is useless, and it should be removed.  In addition,
    'u' is no longer active, and it should also be removed.
    
    Performing extra dead code elimination after demoting shader inputs
    and outputs takes care of this.  This elimination must occur before
    assigning uniform locations, or the declaration of 'u' cannot be
    removed.
    
    This change *breaks* the piglit test getuniform-01, but that test is
    already incorrect.  The test uses a vertex shader that assigns to a
    user-defined varying, but it has no fragment shader.  Since Mesa does
    not support ARB_separate_shader_objects (we only support the EXT
    version), the linker correctly eliminates the user-defined varying.
    The cascading effect is that the uniform queried by the C code of the
    test is also (correctly) eliminated.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41980
    Tested-by: Brian Paul <brianp@vmware.com>
    Cc: Bryan Cain <bryancain3@gmail.com>
    Cc: Vinson Lee <vlee@vmware.com>
    Cc: José Fonseca <jfonseca@vmware.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

commit 1d5d67f8adac9f94715de9804adb536d9a7ec5ee
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Fri Oct 21 11:17:39 2011 -0700

    glsl: Add uniform_locations_assigned parameter to do_dead_code opt pass
    
    Setting this flag prevents declarations of uniforms from being removed
    from the IR.  Since the IR is directly used by several API functions
    that query uniforms in shaders, uniform declarations cannot be removed
    after the locations have been set.  However, it should still be safe
    to reorder the declarations (this is not tested).
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41980
    Tested-by: Brian Paul <brianp@vmware.com>
    Reviewed-by: Bryan Cain <bryancain3@gmail.com>
    Cc: Vinson Lee <vlee@vmware.com>
    Cc: José Fonseca <jfonseca@vmware.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

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

mesa: f4fb0be605790c55abd7d66564486e5860721c21 (master)

Verified fixed.