Bug 96320 - [glsl] C-style const arrays trigger assertion in GLSL 4.30
Summary: [glsl] C-style const arrays trigger assertion in GLSL 4.30
Status: REOPENED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: All All
: medium normal
Assignee: Matt Turner
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-02 02:52 UTC by Ben Russell
Modified: 2016-06-06 22:31 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Russell 2016-06-02 02:52:00 UTC
Overview:

When constructing a C-style array in a shader when the version is 430, this assertion happens:

glsl/ast_to_hir.cpp:2103: virtual bool ast_expression::has_sequence_subexpression() const: Assertion `!"ast_aggregate: Should never get here."' failed.

The shader compiles successfully when the version is 420.

Steps to reproduce:

1) Put any of these in a shader with a #version 430 directive, and attempt to compile the shader. If you do not hit a syntax error, these should trigger the assertion (if you set the version to 420 they will compile successfully):

const float[] EXAMPLE1 = {1.0, 2.0, 3.0};
const float[] EXAMPLE2 = {1.0, 2.0, 3.0, };
const vec3[] EXAMPLE3 = {vec3(1.0, 2.0, 3.0), vec3(4.0, 5.0, 6.0)};
const vec3[] EXAMPLE4 = {vec3(1.0, 2.0, 3.0), vec3(4.0, 5.0, 6.0), };
const vec3[] EXAMPLE5 = {{1.0, 2.0, 3.0}, {4.0, 5.0, 6.0}};
const vec3[] EXAMPLE6 = {{1.0, 2.0, 3.0}, {4.0, 5.0, 6.0}, };
const vec3[] EXAMPLE7 = {{1.0, 2.0, 3.0, }, {4.0, 5.0, 6.0, }};
const vec3[] EXAMPLE8 = {{1.0, 2.0, 3.0, }, {4.0, 5.0, 6.0, }, };

These compile without any issues on both 420 and 430 (and also earlier versions, for that matter):

const float[] NONC1 = float[](1.0, 2.0, 3.0);
const vec3[] NONC2 = vec3[](vec3(1.0, 2.0, 3.0), vec3(4.0, 5.0, 6.0));

For completeness, this one is a syntax error on 420 and 430:

const vec3[] INVALID1 = vec3[]({1.0, 2.0, 3.0}, {4.0, 5.0, 6.0});

Actual results:

Assertion, followed by an abort.

Expected results:

The shader should compile successfully.

Build Date & Hardware:

Git Revision: 98d40b4d1195ebfaa2fd9ed43755ca6896422c1a
Git Revision Date:   Wed Jun 1 09:21:01 2016 +1000
Build Date: 2016-01-06, around 15:00 NZST (+1200)
GPU: Intel HD 530 (Skylake GT2) - i965 driver
Core GL version: 4.3 (GLSL 4.30)
OS: Void Linux amd64
Comment 1 Matt Turner 2016-06-02 05:50:38 UTC
Weird but cool. I can reproduce. I noticed that if I remove the const qualifier, it compiles.
Comment 2 Dave Airlie 2016-06-06 02:38:03 UTC
I've just returned false instead of the unreachable, and it works here.

I've sent a patch to the list to address it.
Comment 3 Dave Airlie 2016-06-06 03:40:03 UTC
This should be fixed in 
https://cgit.freedesktop.org/mesa/mesa/commit/?id=4336196b7fc61166a36babdabdf5dd004ec9ee55

I'm not 100% sure the fix is perfect, but it works for me!
Comment 4 Matt Turner 2016-06-06 20:59:17 UTC
Interesting. Thanks for the heads up.

Looks like the difference in behavior between 4.20 and 4.30 is due to commit 92635a84a.

Ian, it looks like adding 'const' to a declaration and initializer like

>     float[] EXAMPLE1 = {1.0, 2.0, 3.0};

under #version 430 is enough to trigger the problem.

It seems strange to consider "1.0, 2.0, 3.0" to be a constant expression in the same vein as the example with "float a[2,3]".

So I think we're doing something wrong and returning false is papering over the problem.

I don't think comma separated lists of an aggregate initializer are intended treated as constant expressions.

I'm going to reopen for now.
Comment 5 Matt Turner 2016-06-06 21:38:48 UTC
At the same time, I don't think this needs to be tracked on the release bug.
Comment 6 Ian Romanick 2016-06-06 22:31:32 UTC
Hm... I'm trying to remember how I implemented the C-style initializers.  I *thought* it used different rules in the parser, and those rules generated different productions... much like vec4(1,2,3,4) doesn't generate an ast_sequence.  I'll have to dig in a bit.


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.