Bug 34687 - [GLSL] allowing initializing a shared global to different values
Summary: [GLSL] allowing initializing a shared global to different values
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: All Linux (All)
: medium normal
Assignee: Ian Romanick
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-24 18:54 UTC by Gordon Jin
Modified: 2016-02-10 12:59 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
add new piglit test case (5.21 KB, patch)
2011-02-24 18:54 UTC, Gordon Jin
Details | Splinter Review

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.