Bug 102680 - [OpenGL CTS] KHR-GL45.shader_ballot_tests.ShaderBallotBitmasks fails
Summary: [OpenGL CTS] KHR-GL45.shader_ballot_tests.ShaderBallotBitmasks fails
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Intel 3D Bugs Mailing List
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 102590
  Show dependency treegraph
 
Reported: 2017-09-12 16:24 UTC by Kenneth Graunke
Modified: 2017-11-01 07:51 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Kenneth Graunke 2017-09-12 16:24:21 UTC
KHR-GL45.shader_ballot_tests.ShaderBallotBitmasks hits an assertion failure in the EU validator:

cts-runner: compiler/brw_reg_type.c:125: brw_hw_type_to_reg_type: Assertion `!"not reached"' failed.

It's trying to find an immediate of HW type 15.  This is compiling a SIMD8 VS.

I know Jason has a bunch of patches out related to ballot, perhaps one of them fixes it?
Comment 1 Matt Turner 2017-09-12 17:33:22 UTC
Yes, I think one of the patches I already reviewed from Jason's series fixes this (not sure why he resent already-reviewed patches in his huge series):

[PATCH 27/44] intel/eu/validate: Look up types on demand in execution_type()
Comment 2 Jason Ekstrand 2017-09-12 22:02:11 UTC
I just pushed the patch:

commit d496780fb2c7f2cf0e32b6a79dc528e5156dfcb3
Author: Jason Ekstrand <jason.ekstrand@intel.com>
Date:   Thu Aug 31 15:41:43 2017 -0700

    intel/eu/validate: Look up types on demand in execution_type()
    
    We are looking up the execution type prior to checking how many sources
    we have.  This leads to looking for a type for src1 on MOV instructions
    which is bogus.  On BDW+, the src1 register type overlaps with the
    64-bit immediate and causes us problems.
    
    Reviewed-by: Matt Turner <mattst88@gmail.com>
    Cc: mesa-stable@lists.freedesktop.org

Does that fix it?
Comment 3 Kenneth Graunke 2017-09-14 01:29:08 UTC
It fixes the assertion, but the test still fails.  Changing the title accordingly.
Comment 4 Juan A. Suarez 2017-09-15 15:18:19 UTC
Seems like the gl_SubGroup??MaskARB is somewhat wrong.
Comment 5 Kenneth Graunke 2017-10-07 07:27:52 UTC
This regressed back to hitting an assertion failure with Matt's commit 2572c2771d0cab0b9bc489d354ede44dfc88547b (i965: Validate "Special       Requirements for Handling Double Precision Data Types").  Unfortunately, he had no way to know that because we don't run the modern CTS in the CI yet.

It's a trivial fix, I'll send a patch shortly.
Comment 6 Matt Turner 2017-10-19 21:11:35 UTC
(In reply to Kenneth Graunke from comment #5)
> This regressed back to hitting an assertion failure with Matt's commit
> 2572c2771d0cab0b9bc489d354ede44dfc88547b (i965: Validate "Special      
> Requirements for Handling Double Precision Data Types").  Unfortunately, he
> had no way to know that because we don't run the modern CTS in the CI yet.
> 
> It's a trivial fix, I'll send a patch shortly.

commit 03087686ffb8c638831256fd17c124d250d7238b
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Sat Oct 7 00:14:34 2017 -0700

    i965: Don't try to decode types for non-existent src1.
Comment 7 Juan A. Suarez 2017-10-24 09:33:21 UTC
(In reply to Matt Turner from comment #6)
> (In reply to Kenneth Graunke from comment #5)
> > This regressed back to hitting an assertion failure with Matt's commit
> > 2572c2771d0cab0b9bc489d354ede44dfc88547b (i965: Validate "Special      
> > Requirements for Handling Double Precision Data Types").  Unfortunately, he
> > had no way to know that because we don't run the modern CTS in the CI yet.
> > 
> > It's a trivial fix, I'll send a patch shortly.
> 
> commit 03087686ffb8c638831256fd17c124d250d7238b
> Author: Kenneth Graunke <kenneth@whitecape.org>
> Date:   Sat Oct 7 00:14:34 2017 -0700
> 
>     i965: Don't try to decode types for non-existent src1.


This commit landed in upstream, but the test still fails.
Comment 8 Mark Janes 2017-10-24 15:33:46 UTC
To be clear, the patch that Juan refers to did indeed fix the regression which caused the test to *assert*.  So now it is back to the failure state, as it was for Ken's comment #3.
Comment 9 Kenneth Graunke 2017-10-24 18:28:37 UTC
Yup, assert -> fail -> assert -> fail.  One day we will pass, I'm sure of it! :)

Feel free to look at it though, I'm not currently working on this.
Comment 10 Neil Roberts 2017-10-31 18:00:44 UTC
I’ve sent some patches for this here:

https://patchwork.freedesktop.org/series/32918/

and here:

https://patchwork.freedesktop.org/patch/185574/
Comment 11 Neil Roberts 2017-11-01 07:51:58 UTC
I’ve pushed the first Mesa patch and the piglit patch thanks to review from Jason and Matt. I’ll leave the other patches because Jason has a series that will make it unnecessary.


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.