Bug 34687

Summary:	[GLSL] allowing initializing a shared global to different values
Product:	Mesa	Reporter:	Gordon Jin <gordon.jin>
Component:	glsl-compiler	Assignee:	Ian Romanick <idr>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium	CC:	christophe.prigent, idr, jian.j.zhao
Version:	git
Hardware:	All
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:
Attachments:	add new piglit test case

Description Gordon Jin 2011-02-24 18:54:23 UTC

Created attachment 43774 [details] [review]
add new piglit test case

GLSL spec 1.10 section 4.2 says:
" Shared globals are global variables declared with the same name in independently compiled units (shaders) of the same language (vertex or fragment) that are linked together to make a single program.
Shared globals share the same namespace, and must be declared with the same type. They will share the same storage. 
...
All initializers for a shared global must have the same value, or a link error
will result. "

The attached case initialized the shared global variable to different values in the two vertex shaders, and expects a link error.

It passes on nvidia:
# bin/glsl-link-global-init
Failed to link: Vertex info
-----------
0(1) : error C1038: declaration of "f" conflicts with previous declaration at 0(1)
PIGLIT: {'result': 'pass' }

But fails on mesa (with i965 or swrast):
Program should have failed linking, but it was (incorrectly) successful.
PIGLIT: {'result': 'fail' }

Comment 1 Ian Romanick 2011-03-01 18:05:24 UTC

This test is almost identical to glsl-link-initializer-02.  The only difference is that the existing test uses a const and the new test does not.

The specific rules about global initializers are changing in GLSL, so we should update our compiler and the tests to match the new behavior.  Basically, we want to do what Nvidia does.  I'll talk this over with Chad and have him port the tests to his new linker test framework, then fix them.

Comment 2 Gordon Jin 2011-03-01 22:35:09 UTC

(In reply to comment #1)
> This test is almost identical to glsl-link-initializer-02.  The only difference
> is that the existing test uses a const and the new test does not.

Right. Using "const" in both shaders make it pass.

Comment 3 Gordon Jin 2011-10-24 19:58:58 UTC

Ian, would you commit my piglit test?

Comment 4 Ian Romanick 2011-10-28 15:28:48 UTC

I've posted a modified version of this test to the piglit mailing list for review.  It should get pushed early next week.

http://lists.freedesktop.org/archives/piglit/2011-October/001140.html

Comment 5 Ian Romanick 2011-10-31 18:11:34 UTC

I've just posted some patches to the mesa-dev mailing list to fix this issue.  The first patch in the series is:

http://lists.freedesktop.org/archives/mesa-dev/2011-October/013961.html

Comment 6 Gordon Jin 2012-05-10 23:59:23 UTC

(In reply to comment #4)
> I've posted a modified version of this test to the piglit mailing list for
> review.  It should get pushed early next week.
> http://lists.freedesktop.org/archives/piglit/2011-October/001140.html

Ian, did you push? If so, what's the piglit case name?

Comment 7 Timothy Arceri 2016-02-10 12:59:08 UTC

This should have been fixed by:

commit	f37b1ad937dd2c420f4c9fd9aa5887942bd31f3f
linker: Check that initializers for global variables match

The piglit test is:

tests/shaders/glsl-link-initializer-01d.vert

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.