Bug 94295 - [swrast] piglit shader_runner fast_color_clear/all-colors regression
Summary: [swrast] piglit shader_runner fast_color_clear/all-colors regression
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: 11.2
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Plamena Manolova
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords: bisected, regression
Depends on:
Blocks:
 
Reported: 2016-02-25 20:11 UTC by Vinson Lee
Modified: 2016-03-07 01:02 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Proposed Patch (1.26 KB, patch)
2016-02-26 10:58 UTC, Plamena Manolova
Details | Splinter Review
Proposed Patch v2 (1.62 KB, patch)
2016-02-29 08:24 UTC, Plamena Manolova
Details | Splinter Review

Description Vinson Lee 2016-02-25 20:11:55 UTC
mesa: d1509a5848dee57b933139ad2610e99ae09cb5ec (master 11.3.0-devel)

$ ./bin/shader_runner tests/fast_color_clear/all-colors.shader_test -auto
Segmentation fault (core dumped)


(gdb) bt
#0  find_empty_block (prog=0xf2ae10, uniform=0xf2f030) at glsl/link_uniforms.cpp:1051
#1  link_assign_uniform_locations (prog=prog@entry=0xf2ae10, boolean_true=1065353216, num_explicit_uniform_locs=num_explicit_uniform_locs@entry=4294967295, 
    max_uniform_locs=98304) at glsl/link_uniforms.cpp:1238
#2  0x00007fd99ef73db9 in link_shaders (ctx=ctx@entry=0x7fd9a4a99010, prog=prog@entry=0xf2ae10) at glsl/linker.cpp:4566
#3  0x00007fd99eecb3fb in _mesa_glsl_link_shader (ctx=ctx@entry=0x7fd9a4a99010, prog=prog@entry=0xf2ae10) at program/ir_to_mesa.cpp:3036
#4  0x00007fd99edd1b8a in link_program (ctx=0x7fd9a4a99010, program=<optimized out>) at main/shaderapi.c:1048
#5  0x00007fd9a45dafec in stub_glLinkProgram (program=3) at piglit/tests/util/piglit-dispatch-gen.c:32599
#6  0x000000000040776a in link_and_use_shaders () at piglit/tests/shaders/shader_runner.c:1042
#7  0x000000000040e02c in piglit_init (argc=2, argv=0x7ffd6815d008) at piglit/tests/shaders/shader_runner.c:3292
#8  0x00007fd9a464b7fb in run_test (gl_fw=0xd34c20, argc=2, argv=0x7ffd6815d008)
    at piglit/tests/util/piglit-framework-gl/piglit_winsys_framework.c:73
#9  0x00007fd9a462ff6a in piglit_gl_test_run (argc=2, argv=0x7ffd6815d008, config=0x7ffd6815cec0)
    at piglit/tests/util/piglit-framework-gl.c:199
#10 0x0000000000405b50 in main (argc=2, argv=0x7ffd6815d008) at piglit/tests/shaders/shader_runner.c:54
(gdb) l
1046	find_empty_block(struct gl_shader_program *prog,
1047	                 struct gl_uniform_storage *uniform)
1048	{
1049	   const unsigned entries = MAX2(1, uniform->array_elements);
1050	
1051	   foreach_list_typed(struct empty_uniform_block, block, link,
1052	                      &prog->EmptyUniformLocations) {
1053	      /* Found a block with enough slots to fit the uniform */
1054	      if (block->slots == entries) {
1055	         unsigned start = block->start;
(gdb) print block
$1 = (empty_uniform_block *) 0x0
(gdb) print prog->EmptyUniformLocations
$2 = {head = 0x0, tail = 0x0, tail_pred = 0x0}


65dfb3048e8291675ca33581aeff8921f7ea509d is the first bad commit
commit 65dfb3048e8291675ca33581aeff8921f7ea509d
Author: Plamena Manolova <plamena.manolova@intel.com>
Date:   Thu Feb 11 15:00:02 2016 +0200

    compiler/glsl: Fix uniform location counting.
    
    This patch moves the calculation of current uniforms to
    link_uniforms, which makes use of UniformRemapTable which
    stores all the reserved uniform locations.
    
    Location assignment for implicit uniforms now tries to use
    any gaps left in the table after the location assignment
    for explicit uniforms. This gives us more space to store more
    uniforms.
    
    Patch is based on earlier patch with following changes/additions:
    
       1: Move the counting of explicit locations to
          check_explicit_uniform_locations and then pass
          the number to link_assign_uniform_locations.
       2: Count the number of empty slots in UniformRemapTable
          and store them in a list_head.
       3: Try to find an empty slot for implicit locations from
          the list, if that fails resize UniformRemapTable.
    
    Fixes following CTS tests:
       ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max
       ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max-array
    
    Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
    Signed-off-by: Plamena Manolova <plamena.manolova@intel.com>
    Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93696

:040000 040000 5848c556c369c2c798c1c1e036c70c740b56a97a 25915fac71a54954aafd0139a55045ba394969e6 M	src
bisect run success
Comment 1 Plamena Manolova 2016-02-26 10:58:32 UTC
Created attachment 121982 [details] [review]
Proposed Patch
Comment 2 Plamena Manolova 2016-02-26 11:08:31 UTC
(In reply to Vinson Lee from comment #0)
> mesa: d1509a5848dee57b933139ad2610e99ae09cb5ec (master 11.3.0-devel)
> 
> $ ./bin/shader_runner tests/fast_color_clear/all-colors.shader_test -auto
> Segmentation fault (core dumped)
> 
> 
> (gdb) bt
> #0  find_empty_block (prog=0xf2ae10, uniform=0xf2f030) at
> glsl/link_uniforms.cpp:1051
> #1  link_assign_uniform_locations (prog=prog@entry=0xf2ae10,
> boolean_true=1065353216,
> num_explicit_uniform_locs=num_explicit_uniform_locs@entry=4294967295, 
>     max_uniform_locs=98304) at glsl/link_uniforms.cpp:1238
> #2  0x00007fd99ef73db9 in link_shaders (ctx=ctx@entry=0x7fd9a4a99010,
> prog=prog@entry=0xf2ae10) at glsl/linker.cpp:4566
> #3  0x00007fd99eecb3fb in _mesa_glsl_link_shader
> (ctx=ctx@entry=0x7fd9a4a99010, prog=prog@entry=0xf2ae10) at
> program/ir_to_mesa.cpp:3036
> #4  0x00007fd99edd1b8a in link_program (ctx=0x7fd9a4a99010,
> program=<optimized out>) at main/shaderapi.c:1048
> #5  0x00007fd9a45dafec in stub_glLinkProgram (program=3) at
> piglit/tests/util/piglit-dispatch-gen.c:32599
> #6  0x000000000040776a in link_and_use_shaders () at
> piglit/tests/shaders/shader_runner.c:1042
> #7  0x000000000040e02c in piglit_init (argc=2, argv=0x7ffd6815d008) at
> piglit/tests/shaders/shader_runner.c:3292
> #8  0x00007fd9a464b7fb in run_test (gl_fw=0xd34c20, argc=2,
> argv=0x7ffd6815d008)
>     at piglit/tests/util/piglit-framework-gl/piglit_winsys_framework.c:73
> #9  0x00007fd9a462ff6a in piglit_gl_test_run (argc=2, argv=0x7ffd6815d008,
> config=0x7ffd6815cec0)
>     at piglit/tests/util/piglit-framework-gl.c:199
> #10 0x0000000000405b50 in main (argc=2, argv=0x7ffd6815d008) at
> piglit/tests/shaders/shader_runner.c:54
> (gdb) l
> 1046	find_empty_block(struct gl_shader_program *prog,
> 1047	                 struct gl_uniform_storage *uniform)
> 1048	{
> 1049	   const unsigned entries = MAX2(1, uniform->array_elements);
> 1050	
> 1051	   foreach_list_typed(struct empty_uniform_block, block, link,
> 1052	                      &prog->EmptyUniformLocations) {
> 1053	      /* Found a block with enough slots to fit the uniform */
> 1054	      if (block->slots == entries) {
> 1055	         unsigned start = block->start;
> (gdb) print block
> $1 = (empty_uniform_block *) 0x0
> (gdb) print prog->EmptyUniformLocations
> $2 = {head = 0x0, tail = 0x0, tail_pred = 0x0}
> 
> 
> 65dfb3048e8291675ca33581aeff8921f7ea509d is the first bad commit
> commit 65dfb3048e8291675ca33581aeff8921f7ea509d
> Author: Plamena Manolova <plamena.manolova@intel.com>
> Date:   Thu Feb 11 15:00:02 2016 +0200
> 
>     compiler/glsl: Fix uniform location counting.
>     
>     This patch moves the calculation of current uniforms to
>     link_uniforms, which makes use of UniformRemapTable which
>     stores all the reserved uniform locations.
>     
>     Location assignment for implicit uniforms now tries to use
>     any gaps left in the table after the location assignment
>     for explicit uniforms. This gives us more space to store more
>     uniforms.
>     
>     Patch is based on earlier patch with following changes/additions:
>     
>        1: Move the counting of explicit locations to
>           check_explicit_uniform_locations and then pass
>           the number to link_assign_uniform_locations.
>        2: Count the number of empty slots in UniformRemapTable
>           and store them in a list_head.
>        3: Try to find an empty slot for implicit locations from
>           the list, if that fails resize UniformRemapTable.
>     
>     Fixes following CTS tests:
>        ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max
>       
> ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max-array
>     
>     Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
>     Signed-off-by: Plamena Manolova <plamena.manolova@intel.com>
>     Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93696
> 
> :040000 040000 5848c556c369c2c798c1c1e036c70c740b56a97a
> 25915fac71a54954aafd0139a55045ba394969e6 M	src
> bisect run success

Hi Vinson,
Could you verify whether this patch fixes this issue for you?
Comment 3 Vinson Lee 2016-02-26 20:11:27 UTC
(In reply to Plamena Manolova from comment #2)

> Could you verify whether this patch fixes this issue for you?

Test still segfaults with patch id=121982.

#0  find_empty_block (uniform=<optimized out>, prog=<optimized out>) at glsl/link_uniforms.cpp:1054
1054	   foreach_list_typed(struct empty_uniform_block, block, link,
(gdb) l
1049	   const unsigned entries = MAX2(1, uniform->array_elements);
1050	
1051	   if (exec_list_is_empty(&prog->EmptyUniformLocations))
1052	      return -1;
1053	
1054	   foreach_list_typed(struct empty_uniform_block, block, link,
1055	                      &prog->EmptyUniformLocations) {
1056	      /* Found a block with enough slots to fit the uniform */
1057	      if (block->slots == entries) {
1058	         unsigned start = block->start;
(gdb) print block
$1 = (empty_uniform_block *) 0x0
Comment 4 Plamena Manolova 2016-02-29 08:24:18 UTC
Created attachment 122023 [details] [review]
Proposed Patch v2
Comment 5 Plamena Manolova 2016-02-29 08:25:22 UTC
(In reply to Vinson Lee from comment #3)
> (In reply to Plamena Manolova from comment #2)
> 
> > Could you verify whether this patch fixes this issue for you?
> 
> Test still segfaults with patch id=121982.
> 
> #0  find_empty_block (uniform=<optimized out>, prog=<optimized out>) at
> glsl/link_uniforms.cpp:1054
> 1054	   foreach_list_typed(struct empty_uniform_block, block, link,
> (gdb) l
> 1049	   const unsigned entries = MAX2(1, uniform->array_elements);
> 1050	
> 1051	   if (exec_list_is_empty(&prog->EmptyUniformLocations))
> 1052	      return -1;
> 1053	
> 1054	   foreach_list_typed(struct empty_uniform_block, block, link,
> 1055	                      &prog->EmptyUniformLocations) {
> 1056	      /* Found a block with enough slots to fit the uniform */
> 1057	      if (block->slots == entries) {
> 1058	         unsigned start = block->start;
> (gdb) print block
> $1 = (empty_uniform_block *) 0x0

Could you please try out the new patch?
Comment 6 Vinson Lee 2016-02-29 23:35:17 UTC
(In reply to Plamena Manolova from comment #5)
> Could you please try out the new patch?

#0  find_empty_block (uniform=<optimized out>, prog=<optimized out>) at glsl/link_uniforms.cpp:1054
1054	   foreach_list_typed_safe(struct empty_uniform_block, block, link,
(gdb) l
1049	   const unsigned entries = MAX2(1, uniform->array_elements);
1050	
1051	   if (exec_list_is_empty(&prog->EmptyUniformLocations))
1052	      return -1;
1053	
1054	   foreach_list_typed_safe(struct empty_uniform_block, block, link,
1055	                           &prog->EmptyUniformLocations) {
1056	      /* Found a block with enough slots to fit the uniform */
1057	      if (block->slots == entries) {
1058	         unsigned start = block->start;
(gdb) print block
$1 = (empty_uniform_block *) 0x0
Comment 7 Plamena Manolova 2016-03-02 08:54:09 UTC
(In reply to Vinson Lee from comment #6)
> (In reply to Plamena Manolova from comment #5)
> > Could you please try out the new patch?
> 
> #0  find_empty_block (uniform=<optimized out>, prog=<optimized out>) at
> glsl/link_uniforms.cpp:1054
> 1054	   foreach_list_typed_safe(struct empty_uniform_block, block, link,
> (gdb) l
> 1049	   const unsigned entries = MAX2(1, uniform->array_elements);
> 1050	
> 1051	   if (exec_list_is_empty(&prog->EmptyUniformLocations))
> 1052	      return -1;
> 1053	
> 1054	   foreach_list_typed_safe(struct empty_uniform_block, block, link,
> 1055	                           &prog->EmptyUniformLocations) {
> 1056	      /* Found a block with enough slots to fit the uniform */
> 1057	      if (block->slots == entries) {
> 1058	         unsigned start = block->start;
> (gdb) print block
> $1 = (empty_uniform_block *) 0x0

I think commit f3b68fc5fc806cbfd5e7d79b8679fd2bcbae71f4 on master could fix this problem. Do you mind trying it out?
Comment 8 Vinson Lee 2016-03-03 19:38:18 UTC
(In reply to Plamena Manolova from comment #7)
> 
> I think commit f3b68fc5fc806cbfd5e7d79b8679fd2bcbae71f4 on master could fix
> this problem. Do you mind trying it out?

Tested master 4f028bfcc048d7cbd7a7239e9f61b4d7b708aebb which includes f3b68fc5fc806cbfd5e7d79b8679fd2bcbae71f4.

The regression is fixed.


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.