Fix dependency graph traversal for bitcasts. These were not being traversed, leading to an ICE if the bitcast type was an alias, as the symbol was not resolved for later use by the resolver.
Add missing validation for bitcasts. We were permitting any bitcast that wasn't a being cast to a pointer type, when the spec only allows:
* numeric_scalar to numeric_scalar
* vecN<numeric_scalar> to vecN<numeric_scalar>
Add lots of tests.
Fixed: chromium:1276320
Change-Id: I9e5487ec7649ac543f73fc878e7e282bf932d8cb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71681
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
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>
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>
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>
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>
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>
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>
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>
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 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 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>
These methods are going to be removed as they provide little benefit over the WGSL form, are a maintainance burden and they massively bloat our codebase.
This change introduces sem::CallTargetSignature, which can be used as a std::unordered_map key.
This is used in writer/spirv to replace a map that was keyed off ast::Function::type_name().
Bug: tint:1225
Change-Id: Ic220b3155011f21b14d49eecc8042001148e4ca5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66443
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>
These return weird names that love the use of '__' and have little relation to WGSL.
Improve the duplicate case error message.
Clean up control_block_validation_test.cc by making used of the ProgramBuilder helpers.
Bug: tint:1225
Change-Id: I8c4cf3943145cf8372c00d33ae0166c0c0bcbb8b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66442
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
The intrinsics that did anything useful with this were deprecated
several releases ago.
Change-Id: I79e3c901b6a78583853a067ec46cfa98e346517c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66262
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Arguments are the values passed to a function.
Parameters receive arguments.
Fixed: tint:811
Change-Id: I82fe71aa795b8b365bc78981e84c86b419eb3eb2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66263
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Use global declaration order for allocating implicit
pipeline-overridable constant IDs, instead of iterating over an
unordered_map.
Bug: tint:1155
Change-Id: Ia5ff534c617b0d57e45fc20dd0a5a591854e6473
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65522
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Attach the error to the continue statement, and add notes to show
where the variable is both declared and used.
Change-Id: Ie9939a5ca674e7216069bbb1d8dc82ab6949367c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65521
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
An expression that is inside an if-statement condition does not have a
"current block", which is what we were using to see if the usage was
inside a continuing block. Use the current statement to find the
containing block instead.
Fixed: chromium:1251664
Change-Id: Icc808ca1cf6a1b51757da8303901fa5ecb693e83
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65520
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
MSL has a limit on the number of threadgroup memory arguments, so use
a struct to support an arbitrary number of workgroup variables.
This commit introduces a `State` object to this transform, which is
used to track which structs have been cloned eagerly, in order to
avoid duplicating them.
Bug: tint:938
Change-Id: Ia467db186e176a08f160455eab5fd3b3662f56b8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65360
Auto-Submit: James Price <jrprice@google.com>
Kokoro: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This reverts commit af8cd3b7f5.
Reason for revert: breaking roll into Dawn.
Original change's description:
> msl: Use a struct for threadgroup memory arguments
>
> MSL has a limit on the number of threadgroup memory arguments, so use
> a struct to support an arbitrary number of workgroup variables.
>
> Bug: tint:938
> Change-Id: I40e4a8d99bc4ae074010479a56e13e2e0acdded3
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/64380
> Kokoro: Kokoro <noreply+kokoro@google.com>
> Auto-Submit: James Price <jrprice@google.com>
> Reviewed-by: Ben Clayton <bclayton@google.com>
> Commit-Queue: James Price <jrprice@google.com>
TBR=bclayton@google.com,jrprice@google.com,noreply+kokoro@google.com,tint-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I58a07c4ab7e92bda205e2bbbab41e0b347aeb1e8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: tint:938
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65162
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Kokoro: Corentin Wallez <cwallez@chromium.org>
MSL has a limit on the number of threadgroup memory arguments, so use
a struct to support an arbitrary number of workgroup variables.
Bug: tint:938
Change-Id: I40e4a8d99bc4ae074010479a56e13e2e0acdded3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/64380
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
The subsequent commits will implement this for all remaining backends.
Bug: tint:752
Change-Id: Id26f920fea93b5678466705612dcbfc229dc878a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/63921
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: James Price <jrprice@google.com>
Ben Clayton
Antonio Maiorano
James Price
Corentin Wallez
Sarah