Implement support for array constructors in the ESSL3 compiler

GoogleCodeExporter commented 9 years ago

The ESSL3 compiler needs to support array constructors. This includes parsing 
constructors like float[3](); and also float[](); where the size of the array 
is implicit.

Original issue reported on code.google.com by oetu...@nvidia.com on 10 Mar 2015 at 12:23

Blocked on: #960

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 2728eda4bc1d9aa3ac34c610b307c66660dc2376

Code-Review  0 : Olli Etuaho, Zhenyao Mo
Code-Review  +1: Jamie Madill
Code-Review  +2: Nicolas Capens
Verified     0 : Jamie Madill, Nicolas Capens, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I55476483c9e83241e8978cd58f05ef303c7c8680
Reviewed-at    : https://chromium-review.googlesource.com/260260

Revert "A special state to track field selection is not necessary."

This reverts commit f8dc4fb66d2ee473b6ba37626f02756422863f6c.

The special state to track field selection is actually necessary to avoid
reduce/reduce conflicts when array constructors are added to the grammar.

BUG=angleproject:941
TEST=WebGL conformance tests, angle_unittests

src/compiler/translator/glslang.l
src/compiler/translator/glslang.y
src/compiler/translator/glslang_lex.cpp
src/compiler/translator/glslang_tab.cpp

Original comment by bugdro...@chromium.org on 16 Mar 2015 at 3:37

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : d6b1428771146333d65473e2638693b579b6b2e7

Code-Review  0 : Nicolas Capens, Olli Etuaho, Zhenyao Mo
Code-Review  +2: Jamie Madill
Verified     0 : Jamie Madill, Nicolas Capens, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : If1de33ea250893527be7f0d7930d4678a0864684
Reviewed-at    : https://chromium-review.googlesource.com/260571

Move binary op array check to ParseContext

Also piece together an addAssign function in ParseContext that uses the
binary op array check. This will make it easier to change the
array-related checks in the future to use shaderVersion. Moving
validation out from IntermBinary::promote also makes the architecture
clearer, promote()'s role should be mainly to determine the type of the
return value of the binary operation, not to do validation.

BUG=angleproject:941
TEST=angle_unittests, WebGL conformance tests

src/compiler/translator/IntermNode.cpp
src/compiler/translator/ParseContext.cpp
src/compiler/translator/ParseContext.h
src/compiler/translator/glslang.y
src/compiler/translator/glslang_tab.cpp

Original comment by bugdro...@chromium.org on 19 Mar 2015 at 4:20

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : e79904c37c361bc689c86565ccea01575a7fdd2f

Code-Review  0 : Nicolas Capens, Olli Etuaho, Zhenyao Mo
Code-Review  +2: Jamie Madill
Verified     0 : Jamie Madill, Nicolas Capens, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I728d511ab07af94bc3382dc2796c1e9ac79d1442
Reviewed-at    : https://chromium-review.googlesource.com/260801

Accept equality and assignment for arrays in parsing

This is enough to support the operations in GLSL output. HLSL output will
likely require additional work to support this.

BUG=angleproject:941

src/compiler/translator/IntermNode.cpp
src/compiler/translator/OutputHLSL.cpp
src/compiler/translator/ParseContext.cpp

Original comment by bugdro...@chromium.org on 19 Mar 2015 at 4:59

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 7fb4955da9f760766918abc640c95ebcd41caee4

Code-Review  0 : Olli Etuaho, Zhenyao Mo
Code-Review  +1: Jamie Madill
Code-Review  +2: Nicolas Capens
Verified     0 : Jamie Madill, Nicolas Capens, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : Ie2ca7c016a3f0bcb3392a96d6d20d6f803d28bf0
Reviewed-at    : https://chromium-review.googlesource.com/261530

Support array equality operator in HLSL output

This requires adding functions to the shader source that can do the
comparison for a specific array size.

There's no automated test coverage specifically for this functionality,
since all deqp tests that cover this also require array constructors to
be supported. The change has been tested by manually inspecting shader
output. No regressions were seen in automated tests listed below.

TEST=dEQP-GLES3.functional.shaders.*, angle_unittests
BUG=angleproject:941

src/compiler/translator/OutputHLSL.cpp
src/compiler/translator/OutputHLSL.h

Original comment by bugdro...@chromium.org on 25 Mar 2015 at 3:49

GoogleCodeExporter commented 9 years ago

To test this array constructors with dEQP tests, array assignment support in 
the HLSL compiler needs to be in place first. Since that's quite complex, 
opened separate issue 960 for that.

Original comment by oetu...@nvidia.com on 30 Mar 2015 at 1:58

Now blocked on: #960

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : fc7cfd1cf5267d1891c77433c35249d3f69d70af

Code-Review  0 : Jamie Madill, Olli Etuaho
Code-Review  +2: Nicolas Capens
Verified     0 : Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : Ic12b5d0c5547057551df90086453e897a161d559
Reviewed-at    : https://chromium-review.googlesource.com/263221

Fix array function parameter syntax in HLSL equality function

The fact that passing shaders to the platform compiler is deferred until
linking is performed allowed this bug to pass undetected in earlier manual
testing.

TEST=WebGL conformance tests
BUG=angleproject:941

src/compiler/translator/OutputHLSL.cpp

Original comment by bugdro...@chromium.org on 31 Mar 2015 at 3:27

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 3739d235f818ad90b48b06622431e2c79231559b

Code-Review  0 : Nicolas Capens, Olli Etuaho
Code-Review  +1: Jamie Madill
Code-Review  +2: Geoff Lang
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : Id871c872c5b92e2a5bf81c00080ac23004916a75
Reviewed-at    : https://chromium-review.googlesource.com/264671

Make checking for array non-constness depend on input version

First, remove duplicate check for const qualifier on arrays. Only keep
the check inside arrayQualifierErrorCheck().

Second, ESSL3 will introduce array initializers and by extension constant
arrays, so it should allow const qualifier on arrays. These checks are
somewhat superfluous in ESSL1 as well, since the parser already checks
for missing initializers, but it's useful to keep the informative error
messages around.

Add a few tests to make sure that when the ESSL3 implementation
progresses, it still checks for missing initializers on constant arrays.

TEST=angle_unittests
BUG=angleproject:941

src/compiler/translator/ParseContext.cpp
src/compiler/translator/ParseContext.h
src/tests/compiler_tests/MalformedShader_test.cpp

Original comment by bugdro...@chromium.org on 9 Apr 2015 at 9:00

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 47fadcb975e91ff33527cc5f460eb08a97d2a0ff

Code-Review  0 : Nicolas Capens, Olli Etuaho
Code-Review  +1: Jamie Madill
Code-Review  +2: Geoff Lang
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I711565230b35df077f268cec6fdeac6c4c03b4cd
Reviewed-at    : https://chromium-review.googlesource.com/264672

Remove code related to redeclaration of arrays

It should not be possible to redeclare arrays. This seems to be another
thing left over from earlier erroneous code which allowed declaring
unsized arrays.

TEST=angle_unittests
BUG=angleproject:941

src/compiler/translator/ParseContext.cpp

Original comment by bugdro...@chromium.org on 9 Apr 2015 at 9:01

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 43ce600c57c671b68df65b058f1eccbb5115180f

Code-Review  0 : Jamie Madill, Nicolas Capens, Olli Etuaho
Code-Review  +2: Geoff Lang
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I9906792e2c7fe3805288255b855afb46840a77a7
Reviewed-at    : https://chromium-review.googlesource.com/264811

Fix a bunch of mistakes in MalformedShaderTest

A number of mistakes had slipped in to this test that made the
compilation of some shaders always fail regardless of the semantic checks
applied by the parser.

TEST=angle_unittests
BUG=angleproject:941

src/tests/compiler_tests/MalformedShader_test.cpp

Original comment by bugdro...@chromium.org on 10 Apr 2015 at 8:17

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 6ed7bbe84854a848895950be28b2d06146f8e72a

Code-Review  0 : Jamie Madill, Nicolas Capens, Olli Etuaho
Code-Review  +2: Geoff Lang
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I6f6cbc0761a4fc971299bad48864309009c54e7f
Reviewed-at    : https://chromium-review.googlesource.com/264673

Clean up arrayErrorCheck

Remove some unnecessary TPublicType/TType conversions from the code, and
clean up code style. voidErrorCheck is changed to take TBasicType so that
it can be used with both TType and TPublicType.

TEST=angle_unittests
BUG=angleproject:941

src/compiler/translator/ParseContext.cpp
src/compiler/translator/ParseContext.h

Original comment by bugdro...@chromium.org on 10 Apr 2015 at 9:04

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 2935c581e61e9de67d3e3ec137b2b7fc6ec5405c

Code-Review  0 : Geoff Lang, Nicolas Capens, Olli Etuaho
Code-Review  +2: Jamie Madill
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : Ie133be62afc3e1f997370803cf21cada4e738935
Reviewed-at    : https://chromium-review.googlesource.com/264674

Simplify code related to variable declaration

Rename nonInitErrorCheck to declareVariable to clarify that it declares
variables. Merge arrayErrorCheck with that so that logic that is common
between array and non-array declarations is only in one place. This
will simplify adding array initializer handling. This also enables
redeclaring gl_LastFragData using ESSL3 array type syntax.

Comments in executeInitializer claimed that the TVariable object was
needed for error recovery, but that was not actually true, so it can also
use the new declareVariable method. Make "variable" a local variable
instead of a parameter to executeInitializer, since the parameter was
never used by callers of the function.

TEST=angle_unittests, WebGL conformance tests
BUG=angleproject:941

src/compiler/translator/ParseContext.cpp
src/compiler/translator/ParseContext.h

Original comment by bugdro...@chromium.org on 13 Apr 2015 at 7:56

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 33e98913379c260f67addbae331c9d20a7805485

Code-Review  0 : Nicolas Capens, Olli Etuaho
Code-Review  +1: Geoff Lang
Code-Review  +2: Jamie Madill
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : Ie45a8a581bf2931ecccfc41b26db49a257fcaf1d
Reviewed-at    : https://chromium-review.googlesource.com/265201

Add tests for disallowing arrays of arrays

TEST=angle_unittests
BUG=angleproject:941

src/tests/compiler_tests/MalformedShader_test.cpp

Original comment by bugdro...@chromium.org on 15 Apr 2015 at 7:36

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : e7847b082087a7c97cc42e2a89ac8f0725057030

Code-Review  0 : Nicolas Capens, Olli Etuaho
Code-Review  +1: Geoff Lang
Code-Review  +2: Jamie Madill
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I03b0220de93ca5719fdb7c1790a5999b8cb5b225
Reviewed-at    : https://chromium-review.googlesource.com/265202

Unify declaration parsing code

Remove the unused identifierSymbol parameter from parseSingleDeclarator
and unify the ordering of parameters and the code style of different
declaration and declarator parsing functions. Some minor functional
changes to array size handling are done mainly to unify error message
generation. There's soon going to be more of these functions, so it's
good to be systematic.

TEST=angle_unittests, WebGL conformance tests
BUG=angleproject:941

src/compiler/translator/ParseContext.cpp
src/compiler/translator/ParseContext.h
src/compiler/translator/glslang.y
src/compiler/translator/glslang_tab.cpp

Original comment by bugdro...@chromium.org on 15 Apr 2015 at 8:16

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 3875ffd1f4809f058ee96e00871619febd14c03b

Code-Review  0 : Geoff Lang, Jamie Madill, Nicolas Capens
Code-Review  +2: Olli Etuaho
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I900f2af843fd8046f23dd4b77352e77026bbba84
Reviewed-at    : https://chromium-review.googlesource.com/265652

Add parser support for initializing sized arrays

Still missing from this patch: HLSL output, implicitly sized arrays.

Tested with WebGL 2 test sdk/tests/deqp/data/gles3/shaders/arrays.html

TEST=WebGL 2 conformance tests
BUG=angleproject:941

src/compiler/translator/ParseContext.cpp
src/compiler/translator/ParseContext.h
src/compiler/translator/glslang.y
src/compiler/translator/glslang_tab.cpp

Original comment by bugdro...@chromium.org on 15 Apr 2015 at 9:17

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 376f1b5d37de58f7ba391de2e96d1f16e6ceb697

Code-Review  0 : Nicolas Capens, Olli Etuaho
Code-Review  +1: Geoff Lang
Code-Review  +2: Jamie Madill
Verified     0 : Geoff Lang, Jamie Madill, Nicolas Capens
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : Ib55b7601848102a103af9db284a80f09abaeb021
Reviewed-at    : https://chromium-review.googlesource.com/265653

Add support for implicit array size

ESSL3 introduces implicit array size that is determined automatically
according to the initializer. Implicit sizes are resolved when parsing
constructors and when initializers are evaluated, so ANGLE's AST will not
contain implicit sizes.

Declarations where there are two differently sized arrays with the same
implicitly sized type, for example:

float[] a = float[](0.0), b = float[](0.0, 1.0);

will be transformed into declarations where the two arrays don't share
the array size like this:

float a[1] = float[1](0.0), float b[2] = float[2](0.0, 1.0);

so they are not a problem.

Unlike sized arrays, implicitly sized arrays don't have a size limit
enforced by the parser.

Include a test that verifies that non-initialization of an implicitly
sized array defined using ESSL3 type syntax is caught by the parser.

Additionally tested with WebGL 2 test
sdk/tests/deqp/data/gles3/shaders/arrays.html

TEST=WebGL 2 conformance tests, angle_unittests
BUG=angleproject:941

src/compiler/translator/ParseContext.cpp
src/compiler/translator/ParseContext.h
src/compiler/translator/Types.h
src/compiler/translator/glslang.y
src/compiler/translator/glslang_tab.cpp
src/tests/compiler_tests/MalformedShader_test.cpp

Original comment by bugdro...@chromium.org on 15 Apr 2015 at 9:23

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : cd94ef96ac263544bbdef900e04eefcaf916917e

Code-Review  0 : Geoff Lang, Olli Etuaho
Code-Review  +2: Jamie Madill
Verified     0 : Geoff Lang, Jamie Madill
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I59470555227985262b3e914ff6ca11e88d15fd8c
Reviewed-at    : https://chromium-review.googlesource.com/265647

Fix style issues in updateTree()

"entry" is an old C keyword, so it's better not to use it as a variable name.
Also fix a few other minor style issues in the code.

TEST=angle_unittests
BUG=angleproject:941

src/compiler/translator/IntermNode.cpp

Original comment by bugdro...@chromium.org on 16 Apr 2015 at 6:54

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : fc0e2bc0b7e4780dec62a356880ef7d12362e5d2

Code-Review  0 : Geoff Lang, Olli Etuaho
Code-Review  +1: Jamie Madill
Code-Review  +2: Zhenyao Mo
Verified     0 : Geoff Lang, Jamie Madill, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I43dee487578561c01dbde90c2f55a93dda2f057a
Reviewed-at    : https://chromium-review.googlesource.com/266001

Put each array declarator into a separate declaration in HLSL output

Since HLSL doesn't support arrays as l-values, HLSL output needs to split
declarators that initialize arrays to variable declaration and assignment
implemented via a function call. To prepare for this, it is necessary that each
declarator has its own declaration.

BUG=angleproject:941
TEST=angle_end2end_tests, WebGL conformance tests

src/compiler.gypi
src/compiler/translator/IntermNode.cpp
src/compiler/translator/IntermNode.h
src/compiler/translator/SeparateDeclarations.cpp
src/compiler/translator/SeparateDeclarations.h
src/compiler/translator/TranslatorHLSL.cpp

Original comment by bugdro...@chromium.org on 23 Apr 2015 at 11:00

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 78174db780d9c58e69dd0d1af9c062e8991e146c

Code-Review  0 : Olli Etuaho
Code-Review  +1: Jamie Madill
Code-Review  +2: Zhenyao Mo
Verified     0 : Jamie Madill, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I7036bed98fdb1478a383bb959ca03b42c3cb8100
Reviewed-at    : https://chromium-review.googlesource.com/266690

Replace EvqInternal with a separate flag to make it more flexible

The internal flag disables decorating a given symbol in output, effectively
placing it to a different namespace than user-defined symbols. This enables the
compiler to insert symbols to the tree when transforming it to be suitable for
HLSL output without running into name conflicts. In this patch the flag is
separated from the qualifiers since sometimes different qualifiers need to be
used with these internal symbols.

TEST=angle_unittests, angle_end2end_tests, WebGL conformance tests
BUG=angleproject:941

src/compiler/translator/BaseTypes.h
src/compiler/translator/IntermNode.h
src/compiler/translator/OutputHLSL.cpp
src/compiler/translator/RewriteElseBlocks.cpp

Original comment by bugdro...@chromium.org on 23 Apr 2015 at 11:01

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 822fa84e4399c448bb1c18458f41475b0089dd50

Code-Review  0 : Geoff Lang, Olli Etuaho
Code-Review  +1: Jamie Madill
Code-Review  +2: Zhenyao Mo
Verified     0 : Geoff Lang, Jamie Madill, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : Ida84fc343b767bea8b2d04e91c60cb8197d39039
Reviewed-at    : https://chromium-review.googlesource.com/266002

Support array initialization in HLSL output

Do this by separating each array initialization into a declaration and an
assignment. Array assignment is already supported in HLSL output by replacing
it with a function call.

The functionality is tested by the struct array constructor tests in dEQP.

BUG=angleproject:941
TEST=dEQP-GLES3.functional.shaders.arrays.constructor.*

src/compiler.gypi
src/compiler/translator/SeparateArrayInitialization.cpp
src/compiler/translator/SeparateArrayInitialization.h
src/compiler/translator/TranslatorHLSL.cpp

Original comment by bugdro...@chromium.org on 23 Apr 2015 at 11:01

GoogleCodeExporter commented 9 years ago

Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : a8c414ba24340109cd0921350ecb6e5437206549

Code-Review  0 : Geoff Lang, Jamie Madill, Zhenyao Mo
Code-Review  +2: Olli Etuaho
Verified     0 : Geoff Lang, Jamie Madill, Zhenyao Mo
Verified     +1: Olli Etuaho
Commit Queue   : Chumped
Change-Id      : I79f5170139116a3dcfb2be2df5f0f79a3d955ca8
Reviewed-at    : https://chromium-review.googlesource.com/266003

Add basic support for arrays as return values in HLSL output

In HLSL output, user-defined functions that have an array as their return value
get changed so that they have the array as an out parameter instead.

Still missing: support for calling a function that has array as a return value
without assigning the array. Also support for assignments inside complex
expressions.

TEST=dEQP-GLES3.functional.shaders.arrays.return.*
BUG=angleproject:941

src/compiler.gypi
src/compiler/translator/ArrayReturnValueToOutParameter.cpp
src/compiler/translator/ArrayReturnValueToOutParameter.h
src/compiler/translator/OutputHLSL.cpp
src/compiler/translator/TranslatorHLSL.cpp

Original comment by bugdro...@chromium.org on 24 Apr 2015 at 10:55

shairai / angleproject

Implement support for array constructors in the ESSL3 compiler #941