Bug 68086 - [SNB Bisected]Piglit shaders/glsl-fs-atan-3 fail
Summary: [SNB Bisected]Piglit shaders/glsl-fs-atan-3 fail
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: high major
Assignee: Kenneth Graunke
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-14 01:44 UTC by lu hua
Modified: 2013-08-19 02:49 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description lu hua 2013-08-14 01:44:40 UTC
System Environment:
--------------------------
Arch:             i386
Platform:         Sandybridge
Libdrm:		(master)libdrm-2.4.46-26-g3c967e715528ee52195c178c4d09d03b643f0c06
Mesa:		(master)8b5b5fe3944d75c5d3667516139e366d2207c22a
Xserver:	(master)xorg-server-1.14.99.1-176-gf67d022523c59a27f3bf8791aa9ca6624318b1fd
Xf86_video_intel:(master)2.21.14-35-g5840bfe285e3fd9dc550cbe5fa87437870c92038
Cairo:		(master)e438071e9debeca81f97c6fcdc1c2a91a969761d
Libva:		(staging)d540f278465929f3a31030e3f18fdc95ceecffa5
Libva_intel_driver:(staging)42bb613e72d235bcbe141c906dec9431e4c29661
Kernel:	(drm-intel-nightly) b382289157092f9c7643acd1cd8640b26bbca0ab

Bug detailed description:
-------------------------
It fails on sandybridge with mesa master branch, and works well on 9.2 branch.
Following cases also fail with same bisect commit:
spec_ARB_shading_language_packing_execution_built-in-functions_fs-unpackHalf2x16
spec_glsl-1.10_execution_built-in-functions_fs-faceforward-float-float-float
spec_glsl-1.30_execution_isinf-and-isnan_fs_basic
GL_control_flow_for_continue_frag.test
GL_equal_equal_bvec2_frag.test
GL_equal_equal_ivec2_frag.test
GL_equal_equal_vec2_frag.test
GL_faceforward_faceforward_float_frag_nvaryiconst.test
GL_greaterThanEqual_greaterThanEqual_ivec2_frag.test
GL_greaterThanEqual_greaterThanEqual_vec2_frag.test
GL_greaterThan_greaterThan_ivec2_frag.test
GL_greaterThan_greaterThan_vec2_frag.test
GL_lessThanEqual_lessThanEqual_ivec2_frag.test
GL_lessThanEqual_lessThanEqual_vec2_frag.test
GL_lessThan_lessThan_ivec2_frag.test
GL_lessThan_lessThan_vec2_frag.test
GL_notEqual_notEqual_bvec2_frag.test
GL_notEqual_notEqual_ivec2_frag.test
GL_notEqual_notEqual_vec2_frag.test
GL_not_not_bvec2_frag.test
GL_step_step_float_frag_xvary_edgeconsthalf.test
GL3Tests_uniform_buffer_object_uniform_buffer_object_all_valid_basic_types.test

Bisect shows:80e1c2f35f76562a6a62cfa630edaf97e6d6f227 is the first bad commit.
commit 80e1c2f35f76562a6a62cfa630edaf97e6d6f227
Author:     Kenneth Graunke <kenneth@whitecape.org>
AuthorDate: Sun Aug 4 02:05:43 2013 -0700
Commit:     Kenneth Graunke <kenneth@whitecape.org>
CommitDate: Mon Aug 12 13:13:01 2013 -0700

    i965/fs: Optimize IF/MOV/ELSE/MOV/ENDIF to SEL when possible.

    Many GLSL shaders contain code of the form:

       x = condition ? foo : bar

    The compiler emits an ir_if tree for this, since each subexpression
    might be a complex tree that could have side-effects and short-circuit
    logic operations.

    However, the common case is to simply pick one of two constants or
    variable's values---which is exactly what SEL is for.  Replacing IF/ELSE
    with SEL also simplifies the control flow graph, making optimization
    passes which work on basic blocks more effective.

    The shader-db statistics:

       total instructions in shared programs: 1655247 -> 1503234 (-9.18%)
       instructions in affected programs:     949188 -> 797175 (-16.02%)

       2,970 shaders were helped, none hurt.  Gained 181 SIMD16 programs.

    This helps Valve's Source Engine games (max -41.33%), The Cave
    (max -33.33%), Serious Sam 3 (max -18.64%), Yo Frankie! (max -30.19%),
    Zen Bound (max -22.22%), GStreamer (max -6.12%), and GLBenchmark 2.7
    (max -1.94%).

    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Matt Turner <mattst88@gmail.com>


output:
Probe color at (0,100)
  Expected: 0.000000 0.000000 0.000000
  Observed: 0.501961 0.501961 0.501961
Probe color at (25,100)
  Expected: 0.100000 0.100000 0.100000
  Observed: 0.600000 0.600000 0.600000
Probe color at (50,100)
  Expected: 0.200000 0.200000 0.200000
  Observed: 0.701961 0.701961 0.701961
Probe color at (200,100)
  Expected: 0.800000 0.800000 0.800000
  Observed: 0.301961 0.301961 0.301961
Probe color at (224,100)
  Expected: 0.900000 0.900000 0.900000
  Observed: 0.396078 0.396078 0.396078
PIGLIT: {'result': 'fail' }


Reproduce steps:
----------------
1. xinit
2. ./bin/shader_runner tests/shaders/glsl-fs-atan-3.shader_test -auto
Comment 1 lu hua 2013-08-14 01:51:34 UTC
Following webglc cases also fail with same bisect commit:
conformance/canvas/drawingbuffer-hd-dpi-test.html	
conformance/glsl/functions/glsl-function-sign.html	
conformance/glsl/functions/glsl-function-step-float.html	
conformance/glsl/functions/glsl-function-step-gentype.html
Comment 2 Ian Romanick 2013-08-14 02:33:27 UTC
Can you try Ken's patch from the mesa-dev mailing list:

http://lists.freedesktop.org/archives/mesa-dev/2013-August/043270.html
Comment 3 lu hua 2013-08-14 06:07:13 UTC
(In reply to comment #2)
> Can you try Ken's patch from the mesa-dev mailing list:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2013-August/043270.html

Fixed by this patch.
Comment 4 Kenneth Graunke 2013-08-15 22:47:40 UTC
Fixed on master by:

commit 90129da82c91b731c7c65e8e3adbde58d8c62840
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Tue Aug 13 16:07:56 2013 -0700

    i965/fs: Fix Sandybridge regressions from SEL optimization.
    
    Sandybridge is the only platform that supports an IF instruction
    with an embedded comparison.  In this case, we need to emit a CMP
    to go along with the SEL.
    
    Fixes regressions in Piglit's glsl-fs-atan-3, fs-unpackHalf2x16,
    fs-faceforward-float-float-float, isinf-and-isnan fs_basic, and
    isinf-and-isnan fs_fbo.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68086
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Matt Turner <mattst88@gmail.com>
    Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
    Tested-by: lu hua <huax.lu@intel.com>

There is no need to backport this to 9.2, since the SEL optimization doesn't exist there.
Comment 5 lu hua 2013-08-19 02:49:49 UTC
Verified.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.