Several tests fail DXC validation.
Many are fixed by specifying the entry point name.
Change-Id: I1c5c0f1f9c057156106334c2e9c0f15906a06813
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46867
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Checks the following things:
- Non-struct entry point parameters must have pipeline IO attributes
- Non-struct entry point return type must have a pipeline IO attribute
- Structs used as entry point parameters and return values much have
pipeline IO attributes on every member
- Structs used as entry point parameters and return values cannot have
runtime array or nested struct members
- Multiple pipeline IO attributes on a parameter, return type, or
struct member is not allowed
- Any given builtin and location attribute can only appear once for
the return type, and across all parameters
Removed tests for nested structs from the SPIR-V transform/backend.
Fixed a couple of other tests with missing pipeline IO attributes.
Fixed: tint:512
Change-Id: I4c48fe24099333c8c7fcd45934c09baa6830883c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46701
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Moved tests and fixed now broken tests.
Bug: tint:642
Change-Id: Iaa4483abde101f3963ca20e51c1069b2d64bbb5c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46661
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Added tests that test all combos of vec*mat, mat*vec, and mat*mat for 2,
3, and 4 dimensions.
Bug: tint:698
Change-Id: I4a407228261cf8ea2a93bc7077544e5a9244d854
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46660
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Example:
```
var a : i32;
var b : f32;
if (a == b) {
return vec4<f32>(0.4, 0.4, 0.8, 1.0);
}
```
Outputs:
```
error: test7.wgsl:6:9 error: Binary expression operand types are invalid for this operation: i32 equal f32
if (a == b) {
^^
```
Bug: tint:663
Change-Id: Idd2bb5a248b3c7d652483931d7dd58d5123e9ee8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46640
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
* Fixed resolving logical compares with lhs alias
* Fixed resolving multiply with lhs or rhs alias
* Fixed resolving ops with vecN<alias>and matNxM<alias>
* Fixed validation with lhs or rhs alias
* Fixed spir-v generation with lhs/rhs alias and added missing error
message
* Added tests for all valid binary expressions with lhs, rhs, or both as
alias
Bug: tint:680
Change-Id: I095255a3c63ec20b2e974c6866be9470e7e6ec6a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46560
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Use TINT_ICE() where we have diagnostics, TINT_ASSERT() where we do not.
Change-Id: Ic6e842a7afdd957654c3461e5d03ecec7332e6f9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46444
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Required special casing the ElseStatement, as this isn't actually owned by a BlockStatement.
Change-Id: Ic33c207598b838a12b865a7694e596b2629c9208
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46443
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
I inserted this non-validation code in the wrong method.
Change-Id: I43f6d8a0cbf6f0b5f20e453785b141b50d159044
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46442
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This will be used by the generators to determine how to handle
location decorations.
Change-Id: Ib0e0ce852a5da3819781b402c5625a440c4c9544
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46400
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
* With this change, ProgramBuilder::WrapInStatement(expr), which
produced an "expr = expr" assignment expression, would fail validation
in some cases like for call expressions. Replaced this with a
declaration of a variable with type inferred from expr.
* Moved existing validation tests to resolver\assignment_validation.cc,
and added missing tests: AssignFromPointer_Fail.
* Fixed broken tests as a result of this change.
Bug: tint:642
Change-Id: I6a738b67edb0e116a46e936d652c2dcb4389281e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46161
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
There is still no way to spell this out in WGSL, but this adds support
for VariableDecls with an ast::Variable that has nullptr type. In this
case, the Resolver uses the type of the rhs (constructor expression),
which is stored in semantic::Variable.
Added tests for resolving inferred types from constructor, arithmetic,
and call expressions.
Bug: tint:672
Change-Id: I3dcfd18adecebc8b969373d2ac72c21891c21a87
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46160
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
In anticipation of adding support for type inference, no longer use
ast::Variable::type() everywhere, as it will eventually return nullptr
for type-inferred variables. Instead, the Resolver now stores the final
resolved type into the semantic::Variable, and nearly all code now makes
use of that.
ast::Variable::type() has been renamed to ast::Variable::declared_type()
to help make its usage clear, and to distinguish it from
semantic::Variable::Type().
Fixed tests that failed after this change because variables were missing
VariableDeclStatements, so there was no path to the variables during
resolving, and thus no semantic info generated for them.
Bug: tint:672
Change-Id: I0125e2f555839a4892248dc6739a72e9c7f51b1e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46100
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
* Formerly, we reported the same error message if we detected no default
clause or more than one. I made it so that we output a different error
message for each. This makes it more clear, and in the case of more than
one, the error source location points at the second default clause,
rather than at the switch statement.
* Add functions to ProgramBuilder to more easily define switch and case
statements.
* Fix broken tests as a result of this change.
Bug: tint:642
Change-Id: Iab4e610a563165862d9bc190772d32a4dd24ac45
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45880
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Improved error message to use friendly names. Fixed tests that broke as
a result of this change.
Bug: tint:642
Change-Id: I9a1e819e1a6110a89c826936b96ab84f7f79a084
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45582
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Call validation was already implemented in Resolver. This change
completes it by deleting the relevant code in Validator, and moving and
updating the builtins validation test to use the Resolver.
Also added the "v-0004" error code for when detecting recursion, as was
done for the similar error in the Validator.
Bug: tint:642
Bug: tint:487
Change-Id: If7973bfd2d19681a0cbf48c6d427e17a3b927cde
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45463
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This was mostly already implemented in the Resolver, except for adding a
variable scope for blocks.
Moved tests and improved them to only add Source on the error node.
Bug: tint:642
Change-Id: I175dd22c873df5933133bc92276101aeab3021ed
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45460
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
ast::IdentifierExpression nodes can appear outside of functions
(e.g. as initializers for module-scope variables), so we cannot assume
that current_block_ is not nullptr.
We already have several tests that do this, but for some reason the
nullptr dereference does not cause problems on our presubmits.
Change-Id: I612f3eb0d5711a0b1d0bb71663be7cca388b2b3c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45580
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Formerly, the resolver would process arrays and structs first, then
global variables, and finally functions. As we move validation from
Validator to Resolver, we need to process these nodes in declaration
order instead so that we can validate use-before-declaration. This
matches how the Validator processed nodes.
Fixed all tests that failed after this change mainly because of
variables declared after usage.
Bug: tint:642
Change-Id: I01a9575dcfff545b0a056195ec5266283552da38
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45383
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
* Fixed many tests that now failed validation. Most of the time,
functions declared that they returned a type, but with no return
statement.
* ProgramBuilder::WrapInFunction now returns the function is creates,
and std::moves its StatementList.
* ProgramBuilder: Added Return function to create ast::ReturnStatements
more easily.
Bug: tint:642
Change-Id: I3011314e66e264ebd7b89bf9271392391be6a0e5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45382
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This performs very basic type verification for assignments and variable initializers.
Pulls part of the validation logic out of the Validator into the Resolver.
Involves fixing up a bunch of broken tests.
Bug: tint:642
Fixed: tint:631
Change-Id: Ifbdc139ff7eeab810856e0ba9e3c380c6555ec20
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45281
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Fixes a TODO
Bug: tint:60
Change-Id: Ica44d6dbff682374473cacec9d0515e6d3b02f4c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45245
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
* Moved Validator::ValidateConstructedType, which only validated
structs, to Resolver as ValidateStructure.
* Moved relevant tests to new files, and also updated all failing tests
to validate Source location.
* Fixed other tests that broke now that we're validating structs.
Bug: tint:642
Change-Id: Iefc08ef548f52d8c3798d814d2183c56d1236c2d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45160
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Fix msl tests that were impacted by this
Change-Id: I00f4280c2f059358d9187babda9e44f2d16b096e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45244
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
This will be used to validate layout rules, as well as preventing
illegal types from being used in a uniform / storage buffer.
Also: Cleanup logic around VariableDeclStatement
This was spread across 3 places, entirely unnecessarily.
Bug: tint:643
Change-Id: I9d309c3a5dfb5676984f49ce51763a97bcac93bb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45125
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: David Neto <dneto@google.com>
This will allow us to collect up usage information of the structures in a single pass.
Bug: tint:320
Bug: tint:643
Change-Id: Iaa700dc1e287f6df2717c422e66ec453b23b22dc
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45123
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
This is the size of the structure without the trailing alignment
padding. This is what the Dawn needs from the Inspector.
Fixed: tint:653
Change-Id: Iaa01ba949e114973e4a33e084fc10ef9e111016c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45120
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Generate a global variable for the return value and replace return
statements with assignments to this variable.
Add a list of return statements to semantic::Function.
Bug: tint:509
Change-Id: I6bc08fcac7858b48f0eff62199d5011665284220
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44804
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This change validates that the operand types and result type of every
binary operation is valid.
* Added two unit tests which test all valid and invalid param combos. I
also removed the old tests, many of which failed once I added this
validation, and the rest are obviated by the new tests.
* Fixed VertexPulling transform, as well as many tests, that were using
invalid operand types for binary operations.
Fixed: tint:354
Change-Id: Ia3f48384256993da61b341f17ba5583741011819
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44341
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Added enforcement for vector constructor type rules according to the
table in https://gpuweb.github.io/gpuweb/wgsl.html#type-constructor-expr.
This surfaced a number of existing tests that violated some of these
rules or had a type-declaration related bug, so this CL fixes those as
well (these tests either passed the incorrect number of arguments to a
vector constructor or relied on implicit conversions between numeric
types).
Fixed: tint:632
Fixed: tint:476
Change-Id: I8279be3eeae50b64db486ee7a91a43bd94fdff62
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44480
Commit-Queue: Arman Uguray <armansito@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Let's keep these for the SPIR-V reader case. The way things currently work is actually nicer than attempting to generate size / align decorations in the SPIR-V reader.
Change-Id: I83087c153e3b3056e737dcfbfd73ae6a0986bd7c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44684
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
This was broken by a rebase of the Default Struct Layout change.
This went unnoticed because there was no test coverage for these. Added.
Also replace `[[offset(n)]]` decorations with padding fields.
Bug: tint:626
Change-Id: Iad6f1a239bc8d8fcb15d18a204d3f5a78a372350
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44683
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: David Neto <dneto@google.com>
Move the RoundUp() and IsPowerOfTwo() methods from Resolver.cc to this file.
Add tests
Change-Id: Ib7af53dfa5e69083ec4fc2484da92a84c9468818
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44682
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Implements https://github.com/gpuweb/gpuweb/pull/1447
SPIR-V Reader is still TODO, but continues to function as the offset
decoration is still supported.
Bug: tint:626
Bug: tint:629
Change-Id: Id574eb3a5c6729559382812de37b23f0c68fd406
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43640
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Remove the decoration groupings (Array, Function, Struct,
StructMember, Type, Variable), such that all *Decoration classes now
subclass ast::Decoration directly. This allows for decorations to be
used in multiple places; for example, builtin decorations are now
valid for both variables and struct members.
Checking that decoration lists only contain decorations that are valid
for the node that they are attached to is now done inside the
validator.
Change-Id: Ie8c0e53e5730a7dedea50a1dec8f26f9e7b00e8d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44320
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
The readers must not produce invalid ASTs.
If readers cannot produce a valid AST, then they should error instead.
If a reader does produce an invalid AST, this change catches this bad behavior early, significantly helping identify the root of the broken logic.
IsValid() made a bit more sense in the days where the AST was mutable, and was constructed by calling setters on the nodes to build up the tree.
In order to detect bad ASTs, IsValid() would have to perform an entire AST traversal and give a yes / no answer for the entire tree. Not only was this slow, an answer of 'no' didn't tell you *where* the AST was invalid, resulting in a lot of manual debugging.
Now that the AST is fully immutable, all child nodes need to be built before their parents. The AST node constructors now become a perfect place to perform pointer sanity checking.
The argument for attempting to catch and handle invalid ASTs is not a compelling one.
Invalid ASTs are invalid compiler behavior, not something that should ever happen with a correctly functioning compiler.
If this were to happen in production, the user would be utterly clueless to _why_ the program is invalid, or _how_ to fix it.
Attempting to handle invalid ASTs is just masking a much larger problem.
Let's just let the fuzzers do their job to catch any of these cases early.
Fixed: chromium:1185569
Change-Id: I6496426a3a9da9d42627d2c1ca23917bfd04cc5c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44048
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Try and make sense of the huge number of tests we have.
Rename tests so they have a consistent naming style.
Change-Id: I0c089d5945778a8718480a1a2f854435e7b0e79a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44162
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
First step in splitting out resolver tests into multiple files
Change-Id: I58c66ad5e348a50b3e028dff5749cfacb273ea62
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44161
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
I'm going to start pulling apart the resolver tests into separate files, and the test helper shouldn't go into the root tint namespace.
Change-Id: Ie7d131c5b92837c6c9df05b2938cf014a0402ce2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44160
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Also remove unused fields of Resolver (block_to_info_, block_infos_). We can put them back when they're actually needed.
Fixed: tint:190
Change-Id: I1a02a24eca7fba32b8e1120abb88040138a39c6a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44051
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
