Bug 97715 - [ILK,G45,G965] piglit.spec.arb_separate_shader_objects.misc api error checks
Summary: [ILK,G45,G965] piglit.spec.arb_separate_shader_objects.misc api error checks
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Tapani Pälli
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 98335
  Show dependency treegraph
 
Reported: 2016-09-09 16:52 UTC by Mark Janes
Modified: 2016-11-03 16:23 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Mark Janes 2016-09-09 16:52:16 UTC
Regressed by:
4542c7ed5fc6d8cb2495d322b4f06d802d7292cc
Author:     Tapani Pälli <tapani.palli@intel.com>
i965: release GLSL IR in LinkShader after it's not needed

-----------------------------
$ arb_separate_shader_object-api-errors -auto -fbo

Standard Error

piglit: error: waffle_context_create failed due to WAFFLE_ERROR_UNKNOWN: eglCreateContext failed with error EGL_BAD_MATCH(0x3009)
piglit: error: Failed to create waffle_context for OpenGL 3.1 Forward-Compatible Core Context
-----------------------------

I found the same regression in Tapani's CI build:

http://otc-mesa-ci.jf.intel.com/view/dev/job/tpalli/137/

Tapani, is the test incorrect?  Should this bug be filed against piglit?
Comment 1 Tapani Pälli 2016-09-09 17:06:29 UTC
Huh, I think  did see that but seems quite unrelated to the changes (?) Will try to figure this out. Sorry if I broke stuff!
Comment 2 Tapani Pälli 2016-10-20 06:43:50 UTC
Problem is I don't have hardware to test this, running whole CI to test changes is too slow :/

I run CI against commit before this:
2cd02e30d2e1677762d34f1831b8e609970ef0f3

(job 182 under tpalli)

while other tests pass in arb_separate_shader_objects what I find strange is that there is no 'arb_separate_shader_object-api-errors' test in the logs in that run .. I do wonder if something just has triggered this test to be 'active' on these platforms.
Comment 3 Kenneth Graunke 2016-10-24 23:22:06 UTC
Tapani, you can reproduce this with:

$ INTEL_DEVID_OVERRIDE=0x42 ./bin/arb_separate_shader_object-api-errors -auto -fbo

It looks pretty serious.
Comment 4 Tapani Pälli 2016-10-25 05:02:28 UTC
(In reply to Kenneth Graunke from comment #3)
> Tapani, you can reproduce this with:
> 
> $ INTEL_DEVID_OVERRIDE=0x42 ./bin/arb_separate_shader_object-api-errors
> -auto -fbo
> 
> It looks pretty serious.

true, I have forgotten about INTEL_DEVID_OVERRIDE, now can be reproduced easily, thanks!
Comment 5 Tapani Pälli 2016-10-25 05:26:18 UTC
Yes, it looks like we can't release IR for SSO programs since for those relink is expected to happen when attaching/detaching shader stages. I'll do CI run with a patch for this.
Comment 6 Tapani Pälli 2016-10-26 10:32:56 UTC
Why this fails for the dusty gens is that they will run this particular test in compatibility profile. According to commit 76cfb472077dc83c892b4cddf79333341deaa7b5 it is OK to link programs without any attached shaders with compatibility profile.

Taking these in to account, I'm quite confused how to fix this ..

IMO Issue #14 of the GL_ARB_separate_shader_objects seems to be in direct conflict with the compatibility profile spec.
Comment 7 Timothy Arceri 2016-11-03 02:12:45 UTC
Fixed by:

author	Timothy Arceri <timothy.arceri@collabora.com>
commit	d2861d682a235993844989f7742c9539c3e10245

mesa/glsl: delete previously linked shaders earlier when linking

This moves the delete linked shaders call to
_mesa_clear_shader_program_data() which makes sure we delete them
before returning due to any validation problems.

It also reduces some code duplication.

From the OpenGL 4.5 Core spec:

   "If LinkProgram failed, any information about a previous link of
   that program object is lost. Thus, a failed link does not restore
   the old state of program.

   ...

   If one of these commands is called with a program for which
   LinkProgram failed, no error is generated unless otherwise noted.
   Implementations may return information on variables and interface
   blocks that would have been active had the program been linked
   successfully. In cases where the link failed because the program
   required too many resources, these commands may help applications
   determine why limits were exceeded."

Therefore it's expected that we shouldn't be able to query the
program that failed to link and retrieve information about a
previously successful link.

Before this change the linker was doing validation before freeing
the previously linked shaders and therefore could exit on failure
before they were freed.

This change also fixes an issue in compat profile where a program
with no shaders attached is expect to fall back to fixed function
but was instead trying to relink IR from a previous link.

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97715
Cc: "13.0" <mesa-stable@lists.freedesktop.org>


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.