The name now lives on the ast::Struct. Use that instead.
Bug: tint:724
Change-Id: I4ee5e9b29973e468edd8df8c5448816b36f0fca6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48384
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Copy all of the type classes from src/type into ast.
Required the merging of:
* type::Struct into the existing ast::Struct - ast::Struct now has a name.
* type::AccessControl into the existing ast::AccessControl enumerator - The old ast::AccessControl enumerator is now ast::AccessControl::Access
Bug: tint:724
Change-Id: Ibb950036ed551ec769c6d3d2c8fb411809cf6931
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48383
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This is to avoid name conflicts once we move all classes from namespace
`type` to `sem`.
Bug: tint:724
Change-Id: I23cdec636cb5bcf0bbba03ee7bb7c44252ddade7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48361
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This is to avoid name conflicts once we move all classes from namespace
`type` to `sem`.
Bug: tint:724
Change-Id: Icc3d81ae62eb3b329ce28e78a23ea27f29c9263b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48360
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Handle the case where the OpBranchConditional in a loop header
branches to two distinct blocks inside the loop construct.
This is an if-selection in disguise.
Create an kIfSelection with the same set of blocks as the kLoop,
and with the continue target as the merge.
Fixed: tint:524
Change-Id: I5150d19a2b4388da409e2da6e68ffafdc5d21a9a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47560
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
EmitControlBarrier() was calling MakeOperand() to create ast::Expressions that held a ScalarConstructorExpression, which held a IntLiteral.
The literal value was then taken, and the rest was discarded.
Detached AST nodes will become an ICE, so do the work to find the literal value.
Bug: tint:469
Change-Id: I522bfe8db84e853e189c714b18598feb0d49e58b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48049
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
This is a trivial mapping to/from WGSL in all cases.
Bug: tint:478
Change-Id: I7f21a2392543a880906b54fddbdb8bbd149a526e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48140
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Fixed: tint:673
Change-Id: I5a58d9e504446ccff724e368b5ea2cf835d2271b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47768
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Don't end the EXPECT_THAT() with `<< ToString(p->builder(), fe.ast_body());`
This string is already emitted in the error, and calling ast_body() a second time triggers an assert as the builder is already finalized.
Change-Id: Ied6aca73937e86aa6ce2d8cff8a09f67b5082349
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47767
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Removes the need to pass the storage class to the SPIR-V reader
builtin mapping function.
Added a deprecation warning for sample_mask_{in,out}.
Bug: tint:715
Change-Id: I948ff2de2d5de7bd05e1c6ff45bd721c856900e3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47743
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Use the variable storage class to determine the correct builtin to use
in the SPIR-V generator.
Added a deprecation warning for frag_coord.
Bug: tint:714
Change-Id: I5ad4956f9345e2f39f4af16e84668dec345ac82e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47742
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This will be used to detect accidental leaks of program objects between programs.
Bug: tint:709
Change-Id: I20f784a2c673d19a04a880b3ec91dfe2eb743bdb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47622
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
SymbolTable::New() used to build and return a symbol without a registered name. When you asked for the name of the symbol it would return tint_symbol_N, where N is the numerical identifier for the symbol.
This approach was a major tripping hazzard for transforms that liked to fetch the source program name, and register it in the new program (in this situation, you should always use `CloneContext::Clone(Symbol)`).
Without special casing for unnamed symbols, you could end up promoting the unnamed symbol to a named symbol, and then colliding against a new unnamed symbol.
This is exactly what happened in tint:711.
Instead, with this change:
* The concept of unnamed symbols has been removed. All symbols now have a name.
* The signature of `SymbolTable::New()` has been changed to take a name parameter (which defaults to 'tint_symbol'). This can be used to create a new, unique named symbol (possibly with a suffix), which will not collide with any existing symbols. Note these symbols may still collide if `SymbolTable::Register()` is called with the same name. All Transforms that currently use `SymbolTable::Register()` will be fixed in another change.
* The CloneContext has been updated to use `SymbolTable::New()` instead of `Register()`. This means that any symbols defined before a clone will not collide.
* `CloneContext::CloneSymbols()` has been added which allows a transform to pre-clone all the symbols from the source program. This can be used to avoid the authored identifiers being suffixed with a number, in the case a transform calls New() before the symbol is cloned.
* `Symbol::to_str()` has been changed to return `$<id>` instead of `tint_symbol_N`. This is to avoid any confusion between the actual name and the symbol ID.
Bug: tint:711
Bug: tint:712
Change-Id: I526e4b49b7027545613859de487e6a275686107a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47631
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
When finding the true-head, false-head, and potentially the
premerge-head blocks of an if-selection, there was an overly
aggressive check for the true-branch or false-branch landing
on a merge block interior to the if-selection. The check was
determining if the merge block actually corresponded to the selection
header in question. If not, then it was throwing an error.
The bug was that this check must be performed only if the
target in question is actually inside the selection body.
There are cases where the target could represent a structured
exit, e.g. to an enclosing loop's merge or continue, or
an enclosing switch construct's merge.
There is still a latent bug: if either the true branch
or false branch represent such a kLoopBreak, kLoopContinue, or
kSwitchBreak edge, then those are not properly generated.
That will be fixed in a followup CL.
Bug: tint:243, tint:494
Change-Id: I141cce07fa0a1dfe5fad20dd2989315e4cd7b688
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47482
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
This is a commonly used pattern.
Change-Id: I698397c93c33db64c53cbe8662186e1976075b80
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47280
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
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>
This was updated/clarified by the SPIR WG.
Bug: tint:3
Change-Id: Ie4c503f0e5f80ffeabada9c526375588e81a5ceb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45740
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
Downstream users have all caught up to the change.
Remove the "uniform_constant" token from the WGSL parser.
Fixed: tint:332
Change-Id: I046f93d5e6c26b89d419763e73b1ca583250570f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45462
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>
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>
These errors were captured, but not printed.
Fix the lint error that was not being displayed.
Change-Id: I56da5c3a044b8a8e41695883ce780aca6245ad04
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44780
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Add a return type decoration list field to ast::Function.
Bug: tint:513
Change-Id: I41c1087f21a87731eb48ec7642997da5ae7f2baa
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44601
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
This just handles non-struct parameters for now. Structs will be
handled in a later patch.
Bug: tint:513
Change-Id: Idfb202a599fcd84400b89515f21bfed6fd3795b5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44081
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
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>
* Disable "undefined-var-template" in code, rather than in build files
* Add back some missing headers required when building in this context
* Make sure gtest/gmock do not override the default runtime library
Change-Id: I12c05943fc1d2dee4733ae70db7da026f67e0dad
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44180
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
All includes from .cc to .h are preserved, even when transitively included.
It's clear that there are far too many includes in header files, and we should be more aggressive with forward declarations. tint:532 will continue to track this work.
There are, however, plenty of includes that have accumulated over time which are no longer required directly or transitively, so this change starts with a clean slate of *required* includes.
Bug: tint:532
Change-Id: Ie1718dad565f8309fa180ef91bcf3920e76dba18
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44042
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Move out of the src root and into its own subdirectory
Rename methods to remove the 'Determine' prefix.
Fixed: tint:529
Change-Id: Idf89d647780f8a2e7495c1c9e6c402e00ad45b7c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44041
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Put all errors straight into the ProgramBuilder::Diagnostics()
Fixes a TODO. Kills an assert().
Bug: chromium:1185569
Change-Id: I4e6f3b06106c3cfe75cf2bcdfc56b14ad73e81d9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44046
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
