Bug 88793

Summary: [BDW/BSW Bisected]Piglit/shaders_glsl-max-varyings fails
Product: Mesa Reporter: lu hua <huax.lu>
Component: Drivers/DRI/i965Assignee: Matt Turner <mattst88>
Status: VERIFIED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: high CC: idr
Version: unspecifiedKeywords: patch
Hardware: All   
OS: Linux (All)   
URL: http://lists.freedesktop.org/archives/mesa-dev/2015-February/078228.html
Whiteboard:
i915 platform: i915 features:
Attachments: remove ifs from fragment shader
reduce to 108 varyings to avoid VS spilling

Description lu hua 2015-01-26 06:13:19 UTC 
System Environment:
--------------------------
Platform: BDW
Libdrm:		(master)libdrm-2.4.59-2-g0c5aaeef51233aec89ee1f43f03d457f278f8fa0
Mesa:		(master)0aa31bf9c37e11c041f951342f38c38907c620ff
Xserver:		(master)xorg-server-1.16.99.902
Xf86_video_intel:		(master)2.99.917-50-ged19b160089640c2e1167332b00ffe1e569a48dc
Libva:		(master)f9741725839ea144e9a6a1827f74503ee39946c3
Libva_intel_driver:		(master)3d16cf07b6b65b9bd5c2dd855d97558264a12e79
Kernel:   (drm-intel-nightly)67e9eb08a3b967b1ac6b7ec4588a93a2cb030cae

Bug detailed description:
---------------------------
It fails on BDW with mesa master branch, works well on 10.4 branch.
Following  cases also fail with the same bisect commit.
Piglit/shaders_glsl-max-varyings_>MAX_VARYING_COMPONENTS    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_int_array  
Piglit/spec_glsl-1.10_execution_varying-packing_simple_int_separate    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_ivec2_array    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_ivec2_separate    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_ivec4_array    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_ivec4_separate    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_uint_array    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_uint_separate    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_uvec2_array    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_uvec2_separate    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_uvec4_array    
Piglit/spec_glsl-1.10_execution_varying-packing_simple_uvec4_separate    
Piglit/spec_glsl-1.30_execution_varying-packing-mixed-types    

Bisect shows: c638ea3d19d46cfe65739af2c78373146ed50737 is the first bad commit
commit c638ea3d19d46cfe65739af2c78373146ed50737
Author:     Matt Turner <mattst88@gmail.com>
AuthorDate: Wed Apr 9 13:38:14 2014 -0700
Commit:     Matt Turner <mattst88@gmail.com>
CommitDate: Fri Jan 23 17:57:39 2015 -0800

    i965: Don't make instructions with a null dest a barrier to scheduling.

    Now that we properly track accumulator dependencies, the scheduler is
    able to schedule instructions between the mach and mov in the common
    the integer multiplication pattern:

       mul  acc0, x, y
       mach null, x, y
       mov  dest, acc0

    Since a null destination implies no dependency on the destination, we
    can also safely schedule instructions (that don't write the accumulator)
    between the mul and mach.

    GAINED:                                103
    LOST:                                  43

    Causes one program to spill (643 -> 1076 instructions).

    I committed this patch last year (commit 42a26cb5) but reverted it
    (commit 0d3f83f4) after inexplicable artifacts in Kerbal Space Program
    (bug 78648). Tapani reapplied this patch and could not reproduce the bug
    with current Mesa.

    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>

output:
libGL: OpenDriver: trying /opt/X11R7/lib/dri/tls/i965_dri.so
libGL: OpenDriver: trying /opt/X11R7/lib/dri/i965_dri.so
Vertical axis: Increasing numbers of varyings.
Horizontal axis: Which of the varyings contains the color.
GL_MAX_VARYING_FLOATS = 128
Probe color at (90,94)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.501961 0.501961 0.501961
  Failure with 24 vec4 varyings used in varying index 22
Probe color at (90,98)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.501961 0.501961 0.501961
  Failure with 25 vec4 varyings used in varying index 22
Probe color at (2,102)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 26 vec4 varyings used in varying index 0
Probe color at (6,106)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 27 vec4 varyings used in varying index 1
Probe color at (2,110)
  Expected: 0.000000 1.000000 0.000000
  Observed: 1.000000 1.000000 0.000000
  Failure with 28 vec4 varyings used in varying index 0
Probe color at (6,114)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 29 vec4 varyings used in varying index 1
Probe color at (2,118)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.501961 0.501961 0.501961
  Failure with 30 vec4 varyings used in varying index 0
Probe color at (6,122)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 31 vec4 varyings used in varying index 1
Probe color at (2,126)
  Expected: 0.000000 1.000000 0.000000
  Observed: 1.000000 0.000000 0.000000
  Failure with 32 vec4 varyings used in varying index 0
PIGLIT: {"result": "fail" }

Reproduce steps:
-------------------------
1. xinit
2. ./bin/glsl-max-varyings -auto -fbo
Comment 1 lu hua 2015-02-12 04:54:43 UTC 
*** Bug 88906 has been marked as a duplicate of this bug. ***
Comment 2 Matt Turner 2015-02-12 19:23:27 UTC 
Created attachment 113419 [details] [review]
remove ifs from fragment shader

I've reproduced the bug by running

> bin/varying-packing-simple int separate -auto -fbo

and can confirm that the test passes after reverting the bisected patch.

I suspect that this patch simply uncovered an existing bug with spilling. Both the fragment and vertex shader spill heavily.

I've modified simple.c to remove if statements from the fragment shader. After that the fragment shader doesn't spill, but the test still fails.

Reducing the number of varyings to 108 allows the VS to now spill and then the test passes. More investigation needed.
Comment 3 Matt Turner 2015-02-12 19:23:51 UTC 
Created attachment 113420 [details] [review]
reduce to 108 varyings to avoid VS spilling
Comment 4 lu hua 2015-02-13 07:32:07 UTC 
(In reply to Matt Turner from comment #3)
> Created attachment 113420 [details] [review] [review]
> reduce to 108 varyings to avoid VS spilling

Apply this patch, It still fails:
libGL: OpenDriver: trying /opt/X11R7/lib/dri/tls/i965_dri.so
libGL: OpenDriver: trying /opt/X11R7/lib/dri/i965_dri.so
libGL: Error in /etc/drirc line 1, column 0: no element found.
libGL: Error in /root/.drirc line 1, column 0: no element found.
libGL: Error in /etc/drirc line 1, column 0: no element found.
libGL: Error in /root/.drirc line 1, column 0: no element found.
Vertical axis: Increasing numbers of varyings.
Horizontal axis: Which of the varyings contains the color.
GL_MAX_VARYING_FLOATS = 128
Probe color at (90,94)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.501961 0.501961 0.501961
  Failure with 24 vec4 varyings used in varying index 22
Probe color at (90,98)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.501961 0.501961 0.501961
  Failure with 25 vec4 varyings used in varying index 22
Probe color at (2,102)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 26 vec4 varyings used in varying index 0
Probe color at (6,106)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 27 vec4 varyings used in varying index 1
Probe color at (2,110)
  Expected: 0.000000 1.000000 0.000000
  Observed: 1.000000 1.000000 0.000000
  Failure with 28 vec4 varyings used in varying index 0
Probe color at (6,114)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 29 vec4 varyings used in varying index 1
Probe color at (2,118)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.501961 0.501961 0.501961
  Failure with 30 vec4 varyings used in varying index 0
Probe color at (6,122)
  Expected: 0.000000 1.000000 0.000000
  Observed: 0.000000 0.000000 0.000000
  Failure with 31 vec4 varyings used in varying index 1
Probe color at (2,126)
  Expected: 0.000000 1.000000 0.000000
  Observed: 1.000000 0.000000 0.000000
  Failure with 32 vec4 varyings used in varying index 0
PIGLIT: {"result": "fail" }
Comment 5 Mark Janes 2015-02-20 18:00:28 UTC 
This test also began failing intermittently on haswell.
Comment 6 Mark Janes 2015-02-20 18:02:11 UTC 
oops, disregard my last comment.  I confused this test with EXT_transform_feedback.max-varyings
Comment 7 Matt Turner 2015-02-28 21:43:06 UTC 
Patch sent to the mailing list.
Comment 8 Matt Turner 2015-03-02 18:29:10 UTC 
Should be fixed by

commit da20bf068ef0f816968d9bc4dfea81facf0fd680
Author: Matt Turner <mattst88@gmail.com>
Date:   Sat Feb 28 13:36:21 2015 -0800

    i965: Consider scratch writes to have side effects.
Comment 9 lu hua 2015-03-09 06:55:14 UTC 
Verified.Fixed.
libGL: OpenDriver: trying /opt/X11R7/lib/dri/tls/i965_dri.so
libGL: OpenDriver: trying /opt/X11R7/lib/dri/i965_dri.so
Vertical axis: Increasing numbers of varyings.
Horizontal axis: Which of the varyings contains the color.
GL_MAX_VARYING_FLOATS = 128
PIGLIT: {"result": "pass" }

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.