Bug 90258

Summary:	[IVB] spec.glsl-1_10.execution.fs-dfdy-accuracy fails intermittently
Product:	Mesa	Reporter:	Mark Janes <mark.a.janes>
Component:	Drivers/DRI/i965	Assignee:	Tapani Pälli <lemody>
Status:	RESOLVED FIXED	QA Contact:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity:	normal
Priority:	medium	CC:	huax.lu, mark.a.janes
Version:	git
Hardware:	x86-64 (AMD64)
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:
Attachments:	patch to fix the issue

Description Mark Janes 2015-04-30 16:34:15 UTC

The jenkins regression system began reproducing IVB failures for this piglit test somewhere after 10.5.  On the CI system it reproduces fairly regularly (10% of the time?).

The test passes all other platforms, and does not reproduce (yet) in the 10.5 branch.

Test Output:
/tmp/build_root/m64/lib/piglit/bin/shader_runner /tmp/build_root/m64/lib/piglit/tests/spec/glsl-1.10/execution/fs-dfdy-accuracy.shader_test -auto
Probe color at (36,164)
  Expected: 0.000000 1.000000 0.000000 1.000000
  Observed: 1.000000 0.000000 0.000000 1.000000

Comment 1 Tapani Pälli 2015-05-12 09:44:01 UTC

This happens quite reliably when running a while loop in bash, I've bisected this to following commit:

--- 8< -----------------------------------
dd5c8250537640f92dbc1ee63d516c6e3e2aaf77 is the first bad commit
commit dd5c8250537640f92dbc1ee63d516c6e3e2aaf77
Author: Matt Turner <mattst88@gmail.com>
Date:   Tue Apr 14 12:40:34 2015 -0700

    i965: Replace guess_execution_size with something simpler.
    
    guess_execution_size() does two things:
    
       1. Cope with small destination registers.
       2. Cope with SIMD8 vs SIMD16 mode.
    
    This patch replaces the first with a simple if block in brw_set_dest: if
    the destination register width is less than 8, you probably want the
    execution size to match.  (I didn't put this in the 3src block because
    it doesn't seem to matter.)
    
    Since only the FS compiler cares about SIMD16 mode, it's easy to just
    set the default execution size there.
    
    This pattern was already been proven in the Gen8+ generator, but we
    didn't port it back to the existing generator when we combined the two.
    
    This is based on a patch from Ken from about a year ago. I've rebased it
    and and fixed a few bugs.
    
    Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>

Comment 2 Tapani Pälli 2015-05-12 10:20:55 UTC

Between passing and a failing run there is no difference in the generated assembly, I checked this by comparing INTEL_DEBUG=vs,fs outputs.

Comment 3 Tapani Pälli 2015-05-12 11:01:30 UTC

However with patch and before it there are changes in assembly:

before:

add(8)          g7<1>F          g5<4,4,1>.xyxyF -g5<4,4,1>.zwzwF { align16 1Q };
add(8)          g8<1>F          g6<4,4,1>.xyxyF -g6<4,4,1>.zwzwF { align16 2Q };

after:

add(16)         g7<1>F          g5<4,4,1>.xyxyF -g5<4,4,1>.zwzwF { align16 1H };
add(16)         g8<1>F          g6<4,4,1>.xyxyF -g6<4,4,1>.zwzwF { align16 1H };


with help from Curro I think I can tackle this one!

Comment 4 Tapani Pälli 2015-05-12 11:20:56 UTC

Created attachment 115712 [details] [review]
patch to fix the issue

will test

Comment 5 Tapani Pälli 2015-05-13 06:11:47 UTC

pushed to master

(sorry Mark, I forgot to add tested-by tag, many thanks for testing!)

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.