search

KalebDark / angleproject

Automatically exported from code.google.com/p/angleproject
Other
0 stars 0 forks source link

Unify operand type validation in shader compiler #952

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
Right now validation of operand types is split among several different places 
in the code:

1. Built-in function call parameters are checked against the function 
signatures in the symbol table.
2. In ParseContext.
3. In Intermediate member functions, which have the main task of helping to 
build the intermediate representation: tying together nodes etc.
4. IntermUnary/Binary::promote(), functions which have the main task of setting 
the type of the node and changing the type of the operation when the operands 
require that.

I propose gradually moving validation from 3. and 4. to ParseContext, so that 
all the validation is in one place and Intermediate and promote() are left with 
clearly defined roles in the system. This has several advantages: the code is 
easier to understand and check for errors, and there's no need to pass data out 
of ParseContext when checks depend on for example shading language version. 
Asserts can be added to Intermediate and promote() to ensure that nothing 
regresses when doing this refactoring.

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

GoogleCodeExporter commented 9 years ago 
Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 47fd36a70cf5989e2e602b61ca56962913f06d04

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

Move some validation from IntermBinary::promote to ParseContext

This makes the role of promote() in the system clearer and helps to make
the code more understandable, since more of the checks are in the same
logical place.

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

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

Original comment by bugdro...@chromium.org on 20 Mar 2015 at 6:11

GoogleCodeExporter commented 9 years ago 
Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : 69c11b5dfccf7883f59e2a5eea86625b1a23d38a

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

Move validation from Intermediate::addUnaryMath to ParseContext

Intermediate should only have logic for creating node objects, validation
of parameter types belongs in ParseContext.

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

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

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

GoogleCodeExporter commented 9 years ago 
Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : f6c694bceaabf69782d50592c43103a4a5a1ad66

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      : I19bd8d53029e24f734c9436eceb446b37e7fcf26
Reviewed-at    : https://chromium-review.googlesource.com/262416

Assign built-in function return type in promote()

This finishes the refactoring of unary math operation handling so that
IntermUnary::promote has the complete code for setting the return type of
the node.

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

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

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

GoogleCodeExporter commented 9 years ago 
Project: angle/angle
Branch : master
Author : Olli Etuaho <oetuaho@nvidia.com>
Commit : dca3e796d1ca46692090a6dfd27432722504af9c

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      : I9f5a0abb6434ad2730441ea9199ec3f5382ebcda
Reviewed-at    : https://chromium-review.googlesource.com/262415

Refactor unary math operator handling to clarify responsibilities

Shuffle the code around so that each part has a clear responsibility:
IntermUnary::promote is responsible for setting the return type of the
node, Intermediate::addUnaryMath is responsible for creating the node
object, and ParseContext::createUnaryMath is responsible for validating
the operand type.

This removes duplicated bool type check for logical not.

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

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

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