Migrate some of the validation logic over to use the results of behavior
analysis.
The most significant changes are:
* Unreachable-statements now consider merge-points of control flow. For
example, if all branches of a if-statement or switch-statement either
return or discard, the next statement will be considered unreachable.
* Unreachable statements are no longer an error, but a warning. See
https://github.com/gpuweb/gpuweb/issues/2378.
* Statements that follow a loops that does not break, or have a
conditional will now be considered unreachable.
* Unreachable statements produced by the SPIR-V reader are now removed
using the new RemoveUnreachableStatements transform.
Some other new changes include additional validation for the continuing
block for for-loops, to match the rules of a loop continuing block.
The new cases this validation is testing for are not expressible in
WGSL, but some transforms may produce complex continuing statements that
might violate these rules. All the writers are able to decay these
complex for-loop continuing statements to regular loops.
Bug: tint:1302
Change-Id: I0d8a48c73d5d5c30a1cddf92cc3383a692a58e61
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71500
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
This change implements the behavior analysis for expressions and
statements as described in:
https://www.w3.org/TR/WGSL/#behaviors
This CL makes no changes to the validation rules. This will be done as a
followup change.
Bug: tint:1302
Change-Id: If0a251a7982ea15ff5d93b54a5cc5ed03ba60608
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68408
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
The SwitchCaseBlockStatement was bound to the BlockStatement of an ast::CaseStatement, but we had nothing that mapped to the actual ast::CaseStatement.
sem::CaseStatement replaces sem::SwitchCaseBlockStatement, and has a Block() accessor, providing a superset of the old behavior.
With this, we can now easily validate the `fallthrough` rules directly, instead of scanning the switch case. This keeps the validation more tigtly coupled to the ast / sem nodes.
Change-Id: I0f22eba37bb164b9e071a6166c7a41fc1a5ac532
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71460
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Produces a direct SEM -> SEM pointer, reducing AST <-> SEM hopping.
Change-Id: I233b4c47d4e55b5f2c6e14ed08699a302b8fb64d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71321
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Break up Resolver::Statement() into multiple resolver functions.
Move simple statement validation out to resolver_validation.cc
Change-Id: Ifa29433af0a9afa39a66ac3e4f7ca376351adfbf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71102
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
If we encounter a global twice, the stack vector was not popped, which could trigger false cyclic-dependency errors. Because the cyclic dependency did not actually exist, later logic could break, expecting to find a global->global edge which did not exist.
Fixed: chromium:1273451
Change-Id: I9d99f1aeeaea042d9ed847a878c4717803122240
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70980
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Add transform::Unshadow to renamed shadowed symbols. Required by a
number of other transforms.
Replace Resolver symbol resolution with dep-graph.
The dependency graph now performs full symbol resolution before the
regular resolver pass.
Make use of this instead of duplicating the effort.
Simplfies code, and actually performs variable shadowing consistently.
Fixed: tint:819
Bug: tint:1266
Change-Id: I595d1812aebe1d79d2d32e724ff90de36e74cf4b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70523
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
These also need to depend on types / values.
Bug: tint:819
Bug: tint:1266
Change-Id: Ia044d7823aca845dc57a887a164e07137d913429
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70522
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
A find or return-default utility.
Rename `find_or_replace.h` to `map.h` which contains both these utilties that operate on maps.
Change-Id: Iaa76ea3f5c5a1210e413c131dd0556c126ee5d0a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70521
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Performs a module-scope (global) declaration dependency analysis, so
that out-of-order global declarations can be re-ordered into dependency
order for consumption by the resolver.
The WGSL working group are currently debating whether out-of-order
declarations should be included in WebGPU V1, so this implementation
currently errors if module-scope declarations are declared out-of-order,
and the resolver does not currently use this sorted global list.
The analysis does however provide significantly better error diagnostics
when cyclic dependencies are formed, and when globals are declared
out-of-order.
The DependencyGraph also correctly now detects symbol collisions between
functions and types (tint:1308).
With this change, validation is duplicated between the DependencyGraph
and the Resolver. The now-unreachable validation will be removed from
the Resolver with a followup change.
Fixed: tint:1308
Bug: tint:1266
Change-Id: I809c23a069a86cf429f5ec8ef3ad9a98246766ab
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69381
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Try to lex identifiers before punctuation, and backtrack if we only
see a single underscore or something that starts with two underscores.
Rename the modf and frexp return types to start with two underscores.
Spec PR:
https://github.com/gpuweb/gpuweb/pull/2326
Change-Id: Id283af100babfe84faa183345cb8a60848140caa
Fixed: tint:1292
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70160
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
resolver.cc has grown very large, and is difficult to navigate.
The logic is identical to before, but validation logic has been moved to its own .cc file.
Bug: tint:1313
Change-Id: I4b7f3208815efe8ff02f2ad006b7cd31b5e37006
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69380
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Add a new 'Target' to the ast::CallExpression, which can be either an
Identifier or Type. The Identifier may resolve to a Type, if the Type is
a structure or alias.
The Resolver now resolves the CallExpression target to one of the
following sem::CallTargets:
* sem::Function
* sem::Intrinsic
* sem::TypeConstructor
* sem::TypeCast
This change will allow us to remove the type tracking logic from the WGSL
parser, which is required for out-of-order module scope declarations.
Bug: tint:888
Bug: tint:1266
Change-Id: I696f117115a50981fd5c102a0d7764641bb755dd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68525
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Replace all calls to create<ast::TypeConstructorExpression>() with
Construct(). TypeConstructorExpression will be folded into
ast::CallExpression, but the Construct() call signature will remain
identical.
Bug: tint:888
Change-Id: Ifb28a90ccf5184c8090c2e32fa8c82f3996dfa33
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69108
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Nothing yet creates or uses these.
Also add Constant to sem::Call.
These will be needed for TypeConstructors / TypeCasts.
Bug: tint:888
Change-Id: I5b8c64062f3262bdffd210bb012db980c5610b26
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69107
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Now that all LiteralExpressions derive from Expression, and Expr()
already handles literals, there's no reason to have this duplicated
functionality.
Bug: tint:888
Change-Id: I677b04a3cc325311ef25f6b9983fee7548e830f4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69105
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
ast::CallExpression will soon encompase function & intrinsic calls,
along with type-constructors and type-casts. The latter two cannot be
wrapped with a CallStatement, so change
ProgramBuilder::WrapInStatement() to always assign the expression to a
temporary.
Fix the few places that actually relied on this behavior to use
CallStmt() explicitly.
Bug: tint:888
Change-Id: I48b8a2be73128df9cd2b4bdcc00ae81c4a872359
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69104
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Pipeline overidable constants are not compile-time constant.
If a module-scope const has an [[override]] decoration, do not assign
the constant value to it, as this will propagate, and the constant value
may become inlined in places that should be overridable.
Also: Rename sem::GlobalVariable::IsPipelineConstant() to
IsOverridable() to make it clearer that this is not a compile-time known
value. Add SetIsOverridable() so we can correctly set the
IsOverridable() flag even when there isn't an ID.
Change-Id: I5ede9dd180d5ff1696b3868ea4313fc28f93af4b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69140
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Literals are now expressions, so in keeping with all the other
expression types, suffix the class name with Expression.
I'm not overly keen on requiring everything to have an Expression
suffix, but consistency is better than personal preference.
Note: this should have been part of 30848b6, but I managed to drop
this change instead of squashing it. Opps.
Bug: tint:888
Change-Id: Idfaf96abe165a6bf5028e60a160e7408aa2bf9db
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68943
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
This cleans up the remnants of ArrayAccessorExpression which was renamed
in a838bb718.
Change-Id: Ie2c67a49e63774d8b153ec17c3185652708a91e5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68942
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
To fix this, we trick the compiler by wrapping the function body with an
if (true) { <function body> } followed by returning an unused value of
the return type.
Bug: tint:1081
Change-Id: I763bf768f40d07a1045f0a70017bb40d488c8428
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68822
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
ConstructorExpression is abstract, and now only has a single class deriving
from it - TypeConstructorExpression. Just use that instead.
Bug: tint:888
Change-Id: I7ee52ad645bcd8a8a68710c63a1fdf834b5b2a24
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68842
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
The object is not always an array. The index can be applied to vectors
too.
Change-Id: Ifb63d1862090d28cb48d692870e9dd01ddbce5df
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68841
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Literals are now expressions, so in keeping with all the other
expression types, suffix the class name with Expression.
I'm not overly keen on requiring everything to have an Expression
suffix, but consistency is better than personal preference.
Bug: tint:888
Change-Id: Ida1f1b98c71d5d0fe6399bca938010cd5d3a00c3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68840
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Just make Literal an expression. The ScalarConstructorExpression
provides no real value, aside from having a ConstructorExpression base
class that's common between ScalarConstructorExpression and
TypeConstructorExpression. TypeConstructorExpression will be folded into
CallExpression, so this hierarchy will serve no purpose.
First step in resolving the parser ambiguity of type-constructors vs
type-casts vs function calls.
Bug: tint:888
Change-Id: I2585d5ddbf6c0619a8f24c503e61ebf27c182ebe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68524
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
The semantic nodes cannot be fully immutable, as they contain cyclic
references. Remove Resolver::CreateSemanticNodes(), and instead
construct and mutate the semantic nodes in the single traversal pass.
Give up on trying to maintain the 'authored' type names (aliased names).
These are a nightmare to maintain, and provided limited use.
Significantly simplfies the Resolver, and allows us to generate more
semantic to semantic references, reducing sem -> ast -> sem hops.
Note: This change introduces constant value propagation across constant
variables. This is unlocked by the earlier construction of the
sem::Variable.
Change-Id: I592092fdc47fe24d30e512952511c9ab7c16d7a1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68406
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Explicitly name references as either Direct or Transitive.
Rename workgroup_size() to WorkgroupSize().
Remove the return_statements. These were not used anywhere.
Change-Id: I7191665db9c3211d086dd90939abec7003cd7be7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68405
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
And put it into the utils namespace.
Change-Id: Ib4c6fadc63954196d572148d0e96ffec6e3bbb38
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68404
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
There's no need for the ScopeStack to include 'global' information. This
is easily obtainable from the element type.
Replace the get-by-reference, with a simpler return value.
Change-Id: Ic6f4c0f656a2019417d68ffb3fe85ba8343ad15e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68403
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Update or remove tests that try to do this.
Fixed: tint:491
Change-Id: I1f351a4abf68ae9bc6b100885fb1bcea08b31211
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68242
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This is slightly different from the other helpers, since this AST node
does not have a source.
Change-Id: I56e72eb0f26f80d52be9e4f51dd42c3d5149cb00
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67644
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This removes a lot of awkward logic from the MSL writer, and means
that we now handle all module-scope variables with the same transform.
Change-Id: I782e36a4b88dafbc3f8364f7caa7f95c6ae3f5f1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67643
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Use a transform to convert these to the vector form for the MSL and
SPIR-V backends.
MSL only has the scalar form from version 2.0 onwards.
Fixed: tint:1123
Change-Id: I384abd9872d9eae52a10a37cbd6aa96004692e9c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67360
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Use spec-spelling of 'constructible'.
Add missing test file to 'test/BUILD.gn'
See https://dawn-review.googlesource.com/c/tint/+/67064
Change-Id: Ie39773617fd0a363d63cc6449bf3905c9eb6786d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67380
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>
Produce a warning if the attribute is missing for integral vertex
outputs or fragment inputs. This will become an error in the future,
as per the WGSL spec.
Add the attribute to E2E tests.
Bug: tint:1224
Change-Id: Ia1f353f36cb7db516cf9e8b4877423dec3b3e711
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67160
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
And remove a whole load of const_cast hackery.
Semantic nodes may contain internally mutable fields (although only ever modified during resolving), so these are always passed by `const` pointer.
While all AST nodes are internally immutable, we have decided that pointers to AST nodes should also be marked `const`, for consistency.
There's still a collection of const_cast calls in the Resolver. These will be fixed up in a later change.
Bug: tint:745
Change-Id: I046309b8e586772605fc0fe6b2d27f28806d40ef
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66606
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Methods and functions are `CamelCase()`
Public fields are `snake_case` with no trailing `_`
Private fields are `snake_case` with a trailing `_`
Remove pointless getters on fully immutable fields.
They provide no value, and just add `()` noise on use.
Remove unused methods.
Bug: tint:1231
Change-Id: If32efd039df48938efd5bc2186d51fe4853e9840
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66600
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
We previously rejected nested structures for entry point IO only if
there was an attribute on the member in the outer struct. This change
rejects all nested structures instead.
Change-Id: I0d30c8521141154a63915e6c6d8fa31bc23f310e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66520
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
These are legacy methods that were written before the semantic type nodes.
These methods do not consider aliases, and any use of these is likely to be broken for aliases.
Fix up uses of these methods to use the semantic types instead.
Change-Id: Ia66749b279eddff655d3d755fef54a6263643e69
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66601
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
These constructor parameters of StructMember take a uint32_t.
The compiler will normally warn / error about this, but there's some quirk of std::make_shared that makes the compiler silence this warning (possibly because the constructor call is in the STL?).
Was noticed when experimenting with BlockAllocator::Create() to reduce binary size.
Change-Id: I8cdf5078150927e5624752ee7374305c0a5982f0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66448
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
