Bug 104199 - [i965 bisected] BIO and EM Vision in >Observer_ is broken since commit af2c320190f3c73180f1610c8df955a7fa2a4d09
Summary: [i965 bisected] BIO and EM Vision in >Observer_ is broken since commit af2c32...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Francisco Jerez
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-10 19:50 UTC by Darius Spitznagel
Modified: 2017-12-12 20:12 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Bio vision good (249.33 KB, image/png)
2017-12-10 19:51 UTC, Darius Spitznagel
Details
Bio vision bad (277.11 KB, image/png)
2017-12-10 19:51 UTC, Darius Spitznagel
Details
EM vision good (351.72 KB, image/png)
2017-12-10 19:52 UTC, Darius Spitznagel
Details
EM vision bad (123.16 KB, image/png)
2017-12-10 19:53 UTC, Darius Spitznagel
Details
0001-intel-fs-bank_conflicts-Don-t-touch-Gen7-MRF-hack-re.patch (3.11 KB, patch)
2017-12-12 04:34 UTC, Francisco Jerez
Details | Splinter Review

Description Darius Spitznagel 2017-12-10 19:50:01 UTC
Hello Intel devs,

The Bio and EM Vision in >Observer_ is broken since commit...

af2c320190f3c73180f1610c8df955a7fa2a4d09 is the first bad commit
commit af2c320190f3c73180f1610c8df955a7fa2a4d09
Author: Francisco Jerez <currojerez@riseup.net>
Date:   Thu Jun 15 15:23:57 2017 -0700

    intel/fs: Implement GRF bank conflict mitigation pass.
    
    Unnecessary GRF bank conflicts increase the issue time of ternary
    instructions (the overwhelmingly most common of which is MAD) by
    roughly 50%, leading to reduced ALU throughput.  This pass attempts to
    minimize the number of bank conflicts by rearranging the layout of the
    GRF space post-register allocation.  It's in general not possible to
    eliminate all of them without introducing extra copies, which are
    typically more expensive than the bank conflict itself.
    
    In a shader-db run on SKL this helps roughly 46k shaders:
    
       total conflicts in shared programs: 1008981 -> 600461 (-40.49%)
       conflicts in affected programs: 816222 -> 407702 (-50.05%)
       helped: 46234
       HURT: 72
    
    The running time of shader-db itself on SKL seems to be increased by
    roughly 2.52%±1.13% with n=20 due to the additional work done by the
    compiler back-end.
    
    On earlier generations the pass is somewhat less effective in relative
    terms because the hardware incurs a bank conflict anytime the last two
    sources of the instruction are duplicate (e.g. while trying to square
    a value using MAD), which is impossible to avoid without introducing
    copies.  E.g. for a shader-db run on SNB:
    
       total conflicts in shared programs: 944636 -> 623185 (-34.03%)
       conflicts in affected programs: 853258 -> 531807 (-37.67%)
       helped: 31052
       HURT: 19
    
    And on BDW:
    
       total conflicts in shared programs: 1418393 -> 987539 (-30.38%)
       conflicts in affected programs: 1179787 -> 748933 (-36.52%)
       helped: 47592
       HURT: 70
    
    On SKL GT4e this improves performance of GpuTest Volplosion by 3.64%
    ±0.33% with n=16.
    
    NOTE: This patch intentionally disregards some i965 coding conventions
          for the sake of reviewability.  This is addressed by the next
          squash patch which introduces an amount of (for the most part
          boring) boilerplate that might distract reviewers from the
          non-trivial algorithmic details of the pass.
    
    The following patch is squashed in:
    
    SQUASH: intel/fs/bank_conflicts: Roll back to the nineties.
    
    Acked-by: Matt Turner <mattst88@gmail.com>

:040000 040000 f68427a37c7d2d4a8fa696449fa723c7995d2e68 169b720ac98eac61cef120e37212467fa689c938 M	src
Comment 1 Darius Spitznagel 2017-12-10 19:51:27 UTC
Created attachment 136069 [details]
Bio vision good
Comment 2 Darius Spitznagel 2017-12-10 19:51:59 UTC
Created attachment 136070 [details]
Bio vision bad
Comment 3 Darius Spitznagel 2017-12-10 19:52:48 UTC
Created attachment 136071 [details]
EM vision good
Comment 4 Darius Spitznagel 2017-12-10 19:53:22 UTC
Created attachment 136072 [details]
EM vision bad
Comment 5 Darius Spitznagel 2017-12-10 20:34:54 UTC
Apitrace is ready if needed.

This was tested with:
darius@pc1:~$ glxinfo | grep OpenGL
OpenGL vendor string: Intel Open Source Technology Center
OpenGL renderer string: Mesa DRI Intel(R) Haswell Desktop 
OpenGL core profile version string: 4.5 (Core Profile) Mesa 17.4.0-devel (git-b926da241a)
OpenGL core profile shading language version string: 4.50
OpenGL core profile context flags: (none)
OpenGL core profile profile mask: core profile
OpenGL core profile extensions:
OpenGL version string: 3.0 Mesa 17.4.0-devel (git-b926da241a)
OpenGL shading language version string: 1.30
OpenGL context flags: (none)
OpenGL extensions:
OpenGL ES profile version string: OpenGL ES 3.1 Mesa 17.4.0-devel (git-b926da241a)
OpenGL ES profile shading language version string: OpenGL ES GLSL ES 3.10
OpenGL ES profile extensions:

My specs:
Kernel 4.14.4
Xorg 1.19.5
iGPU Intel Haswell Iris Pro 5200
Comment 6 Tapani Pälli 2017-12-11 08:22:35 UTC
there is also bug #104197 that bisects to the same commit, FYI Curro
Comment 7 Tapani Pälli 2017-12-11 08:33:25 UTC
Darius, can you share me the apitrace? (lemody@gmail.com)
Comment 8 Darius Spitznagel 2017-12-11 11:38:02 UTC
(In reply to Tapani Pälli from comment #7)
> Darius, can you share me the apitrace? (lemody@gmail.com)

Mail with download link send.
Comment 9 Tapani Pälli 2017-12-11 12:11:46 UTC
I can reproduce the issue and bisection. Quick hack, if I simply skip optimize_reg_permutation (return map as is) the problem goes away.
Comment 10 Francisco Jerez 2017-12-12 04:34:30 UTC
Created attachment 136094 [details] [review]
0001-intel-fs-bank_conflicts-Don-t-touch-Gen7-MRF-hack-re.patch

This seems to help with your apitrace, can you give it a try?
Comment 11 Darius Spitznagel 2017-12-12 12:29:18 UTC
(In reply to Francisco Jerez from comment #10)
> Created attachment 136094 [details] [review] [review]
> 0001-intel-fs-bank_conflicts-Don-t-touch-Gen7-MRF-hack-re.patch
> 
> This seems to help with your apitrace, can you give it a try?

Yes, problem is fixed.

Thank you Curro, thank you Tapani.
Comment 12 Francisco Jerez 2017-12-12 20:12:39 UTC
Thanks.  Should be fixed in master now.


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.