A function parameter of pointer type must be in one of the following storage classes:
- function
- private
- workgroup
A function parameter must one the following types:
- atomic-free plain type
- a pointer type
- a texture type
- a sampler type
Bug: tint:896 tint:894
Change-Id: Id8cec1bdc8e5be2c8c18a8420cec8f13f6aeddd0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55940
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
We've decided that these will be omitted for now.
Move the check-spec-examples script into the tools/src directory, and update the go modules.
Add a bash script to build and run this.
Change-Id: I852f8ddb1b9b987410a2a49cf6d14e54c3cf3f0e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56381
Auto-Submit: 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>
Common logic between the HLSL, WGSL and MSL writers has been moved into
the TextGenerator base class.
Fixed: tint:892
Change-Id: I0f469516947fe64817ce6251e436da74e5e176e8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56068
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
They are only valid on entry point parameters and return types, and
struct members. They must only be used on floating point scalar and
vector types. If the interpolation type is flat, the sampling type
must not be specified.
Bug: tint:746
Change-Id: Iab17816bc9947a74593a5937bdf513ac9ec664f1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56241
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
logical negation operand must be 'bool' or 'vecN<bool>'
complement operand must be be 'i32', 'u32', 'vecN<i32>' or 'vecN<u32>'
Negation operand must be 'i32', 'f32', 'vecN<i32>' or 'vecN<f32>'
Bug: tint:916 chromium:1216597
Change-Id: Ic88a124a32d16b542560da3ca1c159968d4043a0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55860
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Functions that are not entry points cannot have any attributes on
their return types.
Validate the builtin store types for return types.
Bug: tint:851
Change-Id: I718356b3ab06db4b4502a53b81790e4de0ecfeac
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56100
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>
Entry point function parameter can only have builtin and location
attributes (unless a disable validation attribute is present). Other
functions cannot have any decorations on their parameters.
Fix parameter creation in the CanonicalizeEntryPointIO transform for a
case where it was not stripping attributes that are not valid for
function parameters.
Change-Id: I000908aa2dc007c52d100c5f65bca051a49bec9a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55863
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Use a DisableValidationDecoration to allow these storage classes only
for variables generated by the SPIR-V sanitizer. This is also used in
the Inspector tests, since the Inspector is currently run *after* the
SPIR-V sanitizer. These tests will be removed when this is no longer
the case.
Also validate that builtin/location decorations are not used on
variables (unless they have input/output storage class).
Fix or delete all of the other tests that were wrongly using these
storage classes and attributes.
Bug: tint:697
Change-Id: I8be7fb16191f5e2bed9f7dfb700e51f3b97fd1fe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55862
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>
Describes what Tint system raised the diagnostic.
Use this information in the fuzzers to distinguish between expected and unexpected failure cases in the Transform fuzzer tests.
Fixed: chromium:1206407
Fixed: chromium:1207154
Change-Id: I3b807acafe384a2fc363d2a4165a29693450b3cf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55254
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
This reverts commit 6330260f7d.
Reason for revert: Need by this CL which was also reverted:
https://dawn-review.googlesource.com/c/tint/+/55402
Original change's description:
> Validate that in/out storage classes are not used
>
> Use a DisableValidationDecoration to allow these storage classes only
> for variables generated by the SPIR-V sanitizer.
>
> Fix or delete all of the tests that were wrongly using these storage
> classes.
>
> Bug: tint:697
> Change-Id: Ife1154f687b18529cfcc7a0ed93407fd25c9868e
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55404
> Reviewed-by: Ben Clayton <bclayton@google.com>
> Kokoro: Kokoro <noreply+kokoro@google.com>
> Auto-Submit: James Price <jrprice@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: I54db99d6d0fdf28c27a1f2b9858b84f5cd197409
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: tint:697
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55660
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Use a DisableValidationDecoration to allow these storage classes only
for variables generated by the SPIR-V sanitizer.
Fix or delete all of the tests that were wrongly using these storage
classes.
Bug: tint:697
Change-Id: Ife1154f687b18529cfcc7a0ed93407fd25c9868e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55404
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
After implementing validation and fairly exhaustive tests, discovered
that conversion of scalar vector to bool vector did not work in the
spir-v backend. For module scope variables, we use and rely on the
FoldConstants transform to ensure no conversion needs to take place.
This is necessary because we cannot easily introduce temporary values
and refer to them when casting at module scope. Note that for the same
reason, module-level conversions are always constant foldable, so this
works. For function-level conversions, implemented support to emit a
comparison against a zero value, and store the result in the bool
vector.
Bug: tint:865
Change-Id: I0528045e803f176e03428bc7eac31ae06920bbd7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54744
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
The Resolver already has this information, so just propagate it to the
semantic variable.
Change-Id: Id9fc58d3fc706a1433aba7cb3845b4f53f3fc386
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55120
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 translates to/from OpNot for SPIR-V, and ~ for all three textual
language backends.
Fixed: tint:866
Change-Id: Id934fb309221e3fca0e7efa33edaaae137fd8085
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54980
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>
https://github.com/gpuweb/gpuweb/pull/1801
indexes must be of type 'i32' or 'u32'
Bug: tint:867
Change-Id: Ie5bfacf87af5a06d5428dc510145e96fb156c42e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54720
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Replaces arrays with an explicit stride with an array to a structure holding the element padded with a `[[size]]` decoration.
Note that the HLSL writer is still not correctly emitting structure fields with a `[[size]]`, which will be fixed in a follow up change.
Bug: tint:182
Bug: tint:895
Fixed: tint:180
Fixed: tint:649
Change-Id: Ic135dfc89309ac805507e9f39392577c7f82d154
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54582
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
These have been deprecated, and their usages in Dawn, CTS and samples have been updated.
Fixed: tint:846
Change-Id: I74b831fd5be2e7ca02e8208835eac8beddcef9af
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54325
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Sometimes a stack of transforms will generate multiple different
DisableValidationDecorations on a single node.
Change-Id: Id4afa148360f98796e67017a9bbdd8adf12eed58
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54700
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>
Previously we were only validating return statements that had values,
which meant we were not catching issues when the return value was
omitted in a non-void function.
Fixed: chromium:1219037
Change-Id: If68f67a4a79e46894eee08322726000074c9095b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54561
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Also split out validation tests from call_test.cc into call_validation_test.cc.
Bug: tint:886
Change-Id: I1e1dee9b7c348363e89080cdecd3119cc004658f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54063
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Move these module-scope variables to entry point parameters and pass
them as arguments to functions that use them. Disable entry point IO
validation for them.
Emit [[texture()]] and [[sampler()]] attributes on these entry point
parameters.
Fixed: tint:145
Change-Id: I936a80801875a5d0b6cd98a2e8f3e297a2f53509
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53961
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
And produce a warning if these are used. Hard to test, as we don't want to introduce fake functions in our definition file.
Also add missing cast in EnumMatcher.
Bug: tint:806
Change-Id: I21f189e4befe419f6d5544acfc52387d9a5da782
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54001
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Adding this information to each entry point reported by the inspector.
BUG=tint:855
Change-Id: I043e48afed1503a4267dc4cb198fb86245984551
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53820
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
- Add resolver/call_test.cc for new unit tests, and move a couple that
were in resolver/validation_test.cc to it
- Fix CalculateArrayLength transform so that it passes the address of
the u32 it creates to the internal function
- Fix tests broken as a result of this change
Bug: tint:664
Change-Id: If713f9828790cd51224d2392d42c01c0057cb652
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53920
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
To declare a front_facing builtin variable
- declare as a parameter of the entry point function or it can be
- declared as members of structures that are entry point function parameters
The type of the function-parameter/structure-member must be boolean.
Bug: tint:357
Change-Id: I09c380ae5784d29e776e483d966a0f7d8cd2286c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53821
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
TypeDecls (alias, structure) are not a types - they declare types.
ast::TypeName is what's used for a ast::Type.
Previously we were trying to automatically convert these to TypeNames in the builder, but having these inherit from ast::Type was extremely error prone.
reader/spirv was actually constructing ast::Structs and using them as types, which is invalid.
Change-Id: I05773ad6d488626606019015b84217a5a55a8e8a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53802
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Add a Resolver::IsPlain() method to check for plain types, which is
then used instead of IsStorable() for validating array and struct
subtypes.
Remove validation of assignment and constructor RHS types, instead
validating the type of the variable declaration. This catches
additional errors that were previously missed, such as using a pointer
for a var declaration with no constructor.
Change-Id: I5786a262159d2a42cc05b44743c6c26f6b5647c0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53960
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>
Required a lot of test fixes.
ProgramBuilder: :ConstructValueFilledWith() was a major source of unreached AST types, and this has been removed with more powerful type-building helpers in resolver_test_helper.h.
Change-Id: I1f2007cdaef7f319ab4ef8b4fb8c37687a0fb5d8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53800
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
The tests that were in ptr_ref_* should have been in var_let_*. Move these to the right place, and add more tests that actually test the pointer access.
Bug: tint:846
Change-Id: I383fcbf7eeb4a1428cf50c52bc2958720458adcb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53388
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This also completes the work to resolve the access controls for each
storage type.
Fixed: tint:846
Change-Id: Iab24057ec14620a2978ec63c4a91ba12d1bc6e9b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53381
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Handle access control on var declarations instead of via [[access]]
decorations. This change does the minimal work to migrate the WGSL
parser over to the new syntax. Additional changes will be needed
to correctly generate defaulted access qualifiers, as well as
validating access usage.
The [[access]] decorations are still supported by the WGSL parser,
with new deprecated warnings, but not for aliases. Example:
var x : [[access(x)]] alias_to_struct;
Making this work is far more effort than I want to dedicate to backwards
compatibility, and I do not beleive any real-world usage will be doing
this.
Still TODO:
* Adding access control as the optional, third parameter to ptr<>.
* Calculating default accesses for the various storage types.
* Validating usage of variables against the different accesses.
Bug: tint:846
Change-Id: If8ca82e5d16ec319ecd01f9a2cafffd930963bde
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53088
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Use the new [[stage()]] decorations in intrinsics.def to validate that intrinsics are only called from the correct pipeline stages.
Fixed: tint:657
Change-Id: I9efda26369c45c6f816bdaa53408d3909db403a1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53084
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This was done in ValidateArray(), but this comes after the call to DefaultAlignAndSize(), which will ICE if the type is not storable.
Change-Id: Ia3f1b640f42a45362487c3156237b59ea6fed68b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53043
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This was calling Type() without actually checking that the resolve succeeded.
Have the caller resolve the AST type to the semantic type, so that there's a sensible place to handle errors.
Change-Id: I8fae91854377994f68a924fe8104a48a8afe150e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53042
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Add a template file to generate intrinsic_table.inl and
include this from intrinsic_table.cc.
Speeds up execution of the unittests by 20 - 30%, and
reduces the executable size by a couple of percent.
Bug: tint:832
Change-Id: Ifa7f3c42202c92e97b46ed1716ece48660dd29dd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52504
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: David Neto <dneto@google.com>
This avoid duplicating the logic in two places, and makes it easier to
implement according to the spec.
Bug: tint:376
Change-Id: If62f508e2c76b5b661e66aae9ff20b8e874a65d8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52323
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Add a transform that pushes these into the entry point and then passes
them by pointer to any functions that need them.
Since WGSL does not allow non-function storage class at
function-scope, add a DisableValidation attribute to bypass this
check.
Fixed: tint/726
Change-Id: Ic1f4cd691a54c19e77a60e8ba178508e4249bfd9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51962
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This will soon be used by a new MSL sanitizing transform.
Change-Id: I254c0d26a843cf6153dc8d146389d09b615e8d89
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51961
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
They are always identifiers, and this removes unnecessary type casting
from usages of CallExpression::func().
Change-Id: I7a11dc10658abab578395e20ad830cd5bc5a5b71
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51960
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>
Also spruce up texture validation tests so that they validate error
messages.
Bug: tint:805
Change-Id: I6c86fc16014b127a7ef8254e5badf9b5bed08623
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51860
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
BlockStatement for the root block of a function
Add some basic tests for this lot.
Bug: tint:812
Change-Id: I26b65717798cbff576a44bd78fbcb5c8f0d013e6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51368
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Alastair Donaldson <allydonaldson@googlemail.com>
You've helped us bridge two worlds.
Good Night, Sweet Prince.
Fixed: tint:724
Change-Id: I0b4ba960e9cf5dcff7df9d2f332ea36d6663c440
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51667
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Also add validation for when a named type is declared more than once.
Bug: tint:803
Change-Id: Ifa93b34bc5afd31eba9bfdc4514791ec07c767ec
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51622
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Allows us to put block-type-specific data on the specific subtype instead of littering a common base class
Change-Id: If4a327a8ee52d5911308f38b518ec07c3ceebcb7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51367
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
A helper class and macro used to simplify scope-based assignment of
variables.
Change-Id: I02b3a05240a2c4628f813de931c40d8fba3cb07b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51480
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Change the type of the values in an ast::WorkgroupDecoration to be
ast::Expression nodes, so that they can represent both
ast::ScalarExpression (literal) and ast::IdentifierExpression
(module-scope constant).
The Resolver processes these nodes to produce a uint32_t for the
default value on each dimension, and captures a reference to the
module-scope constant if it is overridable (which will soon be used by
the inspector and backends).
The WGSL parser now uses `primary_expression` to parse arguments to
workgroup_size.
Also added some WorkgroupSize() helpers to ProgramBuilder.
Bug: tint:713
Change-Id: I44b7b0021b925c84f25f65e26dc7da6b19ede508
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51262
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
The workgroup size should not be a property of the function in the
AST, and this lays the groundwork for allowing both literals and
module-scope constants to be used for this attribute.
Bug: tint:713
Change-Id: I014be879e2adb81cfc5b0ea0e221035fae626223
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51261
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 change implements pointers and references as described by the WGSL
specification change in https://github.com/gpuweb/gpuweb/pull/1569.
reader/spirv:
* Now emits address-of `&expr` and indirection `*expr` operators as
needed.
* As an identifier may now resolve to a pointer or reference type
depending on whether the declaration is a `var`, `let` or
parameter, `Function::identifier_values_` has been changed from
an ID set to an ID -> Type* map.
resolver:
* Now correctly resolves all expressions to either a value type,
reference type or pointer type.
* Validates pointer / reference rules on assignment, `var` and `let`
construction, and usage.
* Handles the address-of and indirection operators.
* No longer does any implicit loads of pointer types.
* Storage class validation is still TODO (crbug.com/tint/809)
writer/spirv:
* Correctly handles variables and expressions of pointer and
reference types, emitting OpLoads where necessary.
test:
* Lots of new test cases
Fixed: tint:727
Change-Id: I77d3281590e35e5a3122f5b74cdeb71a6fe51f74
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50740
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Rename:
* type() to StoreType()
* storage_class() to StorageClass()
Move away from snake_case methods in the semantic namespace.
Try to avoid generic 'type()' method names.
Also add an assertion to detect doubly nested references (these are
invalid).
Bug: tint:727
Change-Id: I975a3f1e5fbed7947cc2fc156fee892b282c63de
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51220
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Semantic information about block statements the resolver would
temporarily create while resolving is now exposed in a
sem::BlockStatement class.
In the process, semantic information about statements in general is
overhauled so that a statement has a reference to its parent
statement, regardless of whether this is a block.
Bug: tint:799
Bug: tint:800
Change-Id: I8771511c5274ea74741b8c86f0f55cbc39810888
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50904
Commit-Queue: Alastair Donaldson <allydonaldson@googlemail.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Also validate that workgroup_size is only applied to compute stages,
and not duplicated.
Fixed: tint:703
Change-Id: I02f4ddea305cad25ee0a99e13dc9e7fd1d5dc3ea
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51120
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: James Price <jrprice@google.com>
Change-Id: Iaa098d7851ffaf01aac384fe623c94809948fe2e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51182
Commit-Queue: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
In preparation for implementing
https://github.com/gpuweb/gpuweb/issues/1604, this change removes the
sem::AccessControl node. Instead, the ast::AccessControl::Access enum is
now on the sem::StorageTexture class, as well as on sem::Variable. For
sem::Variable, the field is set when the variable's type is either a
storage buffer or a storage texture.
Bug: tint:802
Change-Id: Id479af36b401d067b015027923f4e715f5f69f25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51020
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Keep track of any constant IDs specified in the shader, and then
allocate IDs for the remaining constants when creating the semantic
info.
Bug: tint:755
Change-Id: I6a76b1193cac459b62582cde7469b092dde51d5d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50841
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>
Add a new parameter to BindingRemapper::Remappings that allows resulting binding points to collide.
When enabled, the output of the transform contains two or more module-scoped variables with the same binding point, used by the same entry point, then these variables will be decorated with an internal decoration to disable validation for the collision.
This is to work around collisions generated for the HLSL backend where the variables actually exist in different register classes, which is permitted by D3D12.
The transform will only generate these decorations if it needs to.
Fixed: tint:797
Change-Id: Id8a87523801bd0cd0dd54227ebabd4299bc20c27
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50742
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
An [[internal]] decoration that specifically disables certain validation checks.
Begin with a single kFunctionHasNoBody mode.
Migrate the Resolver to using this instead of allowing any InternalDecoration to disable the checks for no-body.
Bug: tint:797
Change-Id: I213b9a6844a456775ede06d60e456d9f77a449d0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50741
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Variable() is called for globals, locals and parameters. Much of the logic is the same.
Move all the common logic down into Variable(). This:
* Removes some yucky default parameters
* Adds type validation that was missing for globals (broken tests fixed)
* Gives me a single place to implement the Reference type wrapping
Bug: tint:727
Change-Id: I70f4a3603d7fa781da938508aa2a1bc80ec15d77
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50580
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Validate that:
* That resource variables have resource bindings
* Only resource variables have resource bindings
* That a [[binding]] decoration is paired with a [[group]]
* That binding points are not reused in the same entry point
Fixed: tint:235
Fixed: tint:645
Bug: tint:645
Change-Id: I2542934b4c6a2b4bbde48242932c04c796033a90
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50500
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Give them sensible names.
Make them act consistently.
Remove those that were not used.
Change-Id: Ib043a4093cfae9f81630643e1a0e4eae7bca2440
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50305
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
We don't want the WGSL parser to have to maintain type lookups.
If the WGSL language is updated to allow module-scope variables to be declared in any order, then the single-pass approach is going to fail horribly.
Instead do the check in the Resovler.
With this change, the AST nodes actually contain the correctly declared storage class.
Fix up the SPIR-V reader to generate StorageClass::kNone for handle types.
Fix all tests.
Bug: tint:724
Change-Id: I102e30c9bbef32de40e123c2676ea9a281dee74d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50306
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
With the parsers now using ast::Types, nothing should be producing these any more.
This change also removes Resolver::Canonical(), which is now unneeded as there are no sem::Aliases to remove.
Bug: tint:724
Change-Id: I0c1a49f49372c1fcc37864502f07c5c76328d471
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50304
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Using semantic info.
Change-Id: Iec9a592d9d66930535ead78fab69a6085a57a941
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50302
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
There's now no need to have both.
Removes a whole bunch of Sem().Get() smell, and simplifies the resolver.
Also fixes a long-standing issue where an array with an explicit, but equal-to-implicit-stride attribute would result in a different type to an array without the decoration.
Bug: tint:724
Fixed: tint:782
Change-Id: I0202459009cd45be427cdb621993a5a3b07ff51e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50301
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
BUG=tint:773
Change-Id: I94e8624647c645efe7ed558caa3d3bd05dd72f63
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50260
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
There's now no need to have both.
Removes a whole bunch of Sem().Get() smell, and simplifies the resolver.
Bug: tint:724
Fixed: tint:761
Change-Id: I756a32680ac52441fd6eebf6fc53dd507ef5e538
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49961
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Don't have a separate diagnostic list, just put the errors straight into the ProgramBuilder's diagnostics.
This also fixes an issue where we were taking the stringified diagnostic list and creating a single error on resolution failure. This was the cause of the `error: error:` messages sometimes seen.
Also fix a stupid negated-logic bug around the "resolving failed, but no error was raised" ICE.
Change-Id: Iddf1f61e4be21137731dfc204210562abbf612b0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49963
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Unless they are pipeline overridable.
Fixed several tests that were violating this.
Change-Id: Ibb10e03bb150086a4d9e68a3f7b2d3e21f282918
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50042
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Only allow them on constants, where no other decoration is valid.
Change-Id: I83f19667adb1dd4ebbba86827324a45a8f1a80a4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50041
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
BUG=tint:778
Change-Id: If06e453b6fd58b8c74bed996532b3b9f9f7804ee
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50044
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Two incompatible changes landed simultaniously.
Also fix a warning about variable shadowing.
Change-Id: I84c9ba48fb87a348a5b0e622ca2fdb191f95a6b2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49964
Commit-Queue: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
It's common to want this when indexing matrices.
Change-Id: Ic60a3a8d05873119d78a3cb0860d129e33ac3525
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49880
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Remove all sem::Type references from the AST.
ConstructedTypes are now all AST types.
The parsers will still create semantic types, but these are now disjoint
and ignored.
The parsers will be updated with future changes to stop creating these
semantic types.
Resolver creates semantic types from the AST types. Most downstream
logic continues to use the semantic types, however transforms will now
need to rebuild AST type information instead of reassigning semantic
information, as semantic nodes are fully rebuilt by the Resolver.
Bug: tint:724
Change-Id: I4ce03a075f13c77648cda5c3691bae202752ecc5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49747
Commit-Queue: Ben Clayton <bclayton@chromium.org>
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>
Currently untestable as nothing currently calls Type(const ast::Type* ty).
Bug: tint:724
Change-Id: I92dd772acd758b0960a7e9a19f15c91414ab505c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49527
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
This is the resolved, semantic, return type of the function.
Bug: tint:724
Change-Id: I4ef9f7874414a3ea48131d0102da776f6d82a729
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49526
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
There are no downstream usages, so we can skip deprecation. Allowing
the ID to be omitted will be done in a separate patch.
Fixed: tint:754
Change-Id: I3bd6de4d0f426fc3c66708bfd5b411a4051b375b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49581
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Include the pointer - helps debugging
Change-Id: I50058095710eb528446bbd8d8e3069dd657c75c8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49521
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Once the AST switches to pure ast::Type nodes, we need a way to fetch the semantic type for a structure member.
Bug: tint:724
Change-Id: I4b55c1ec0220e29ca4ff3131cf2d41409821a538
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49347
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
A literal has an implicit type, so there should be no type on the AST node.
This highlighted that the resolver was nto canonicalizing TypeConstructorExpression types, which has been fixed.
This required preservation of the declared type name in order for error messages to contain aliased names.
Bug: tint:724
Change-Id: I21594a3e8a0fb1b73c6c5b46a14b8664b7f28512
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49345
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Make all the sem::Type pointers const.
The later stages still have not been fixed up, so there's liberal usage of const_cast where we create semantic nodes.
Bug: tint:745
Change-Id: I160b791f2b7944f8966bc961e061d1e5996c1973
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49343
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
* ProgramBuilder: added a bunch of overloads that take Source
* Added MultiTokenSource RAII helper to build source ranges for
multi-token types
* Added comparison operators to Source::Range and Source::Location to
make it easier to write tests to compare Source ranges
* Moved CombineSourceRange from resolver.cc to a static function in
Source named Source::Combine()
* Added Source tests for all ast type nodes returned by the wgsl parser
Bug: tint:724
Change-Id: I6fb6211a3c42c14693df8746af6a30f5aa56f2af
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48963
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This is a waste of memory, and now fires a TINT_ASSERT() in the resolver.
Add some more information to the resolver assertion message so that its easier to identify the node. This is especially useful when there's no source information available.
Fixed: tint:740
Bug: tint:469
Change-Id: I0cd4529db7b3906e64da6ed7290163509eb0c3f2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48689
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Change-Id: I26518ecd17cd3a69357698d1e1a2201fcbc460e0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48880
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
BUG=tint:736
Change-Id: I6baae6507597383dd58b0d86605cbd44d5488c8f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48842
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
CTS needs updating to fix this validation error, and this is blocking autorolls for tint and dawn.
Change-Id: Ie3aaa37f0914fdb39099e7c15e65be0bcca0677a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48682
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Fixup many tests that were just returning void.
Change-Id: Ic93db5b187c679dc1c24a356b48a64e41ba9a823
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48560
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Not currently called (nothing currently attaches ast::Type nodes to the AST), but implements some of the boilerplate that'll be shortly required.
This change also removes a bunch of duplicated enumerators from the sem namespace for their counterpart in the ast namespace.
Bug: tint:724
Change-Id: I0372a9f4eca2f9357ff161e7ec1b67eae1c4c8f6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48603
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Emit the new typ::I32, typ::U32, typ::F32, typ::Void, typ::Bool types instead of the just the sem::Types.
Used as a stepping stone to emitting the ast::Types instead.
Bug: tint:724
Change-Id: Ic492e33bc909e0821b581af05242d956631db2e3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48602
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Adds rejecting unsupported image formats and texture dimensions.
BUG=tint:718
Change-Id: Ic6375d1e75524336462ea59b159cb4c8601f55fd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48500
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
More of a workaround. Will be properly fixed by tint:724 .
Fixed: tint:728
Change-Id: If4887a781106010d12209365f36f27b493a9f173
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48422
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Fixes:tint:717
Change-Id: I7f23086ec516c1196a9cdc741bbaa150754a533a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47940
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
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>
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>
This was only called for function-scope variable declarations.
In calling this, there were inevitable tests failing, which have now been fixed.
Added a test for the single runtime-array-length validation rule that this function was checking.
Fixed: tint:345
Change-Id: Ic453c38158c1290a5e1ef6de56af0c621d97982a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48381
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Fixed: tint:94
Change-Id: I1d3e512c030ec16031b8c8fcfbde0cd1db5d1ea4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48380
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
AST nodes must not be shared. Diamonds in the AST will cause all sorts
of exciting, non trivial bugs.
All AST nodes must be reached by the Resolver. There are two common
reasons why they may not be:
(a) They were constructed and not attached to the AST. Several
transforms scan the full list of constructed AST nodes to find nodes
of a given type. Having detached nodes will likely cause bugs in
these transforms. Detached nodes is also just a waste of memory.
(b) They are attached to the AST, but the resolver did not traverse
them. Having the resolver skip over parts of the AST will fail to
catch validation issues, and will leave semantic gaps, likely
breaking downstream logic.
Bug: tint:469
Change-Id: I143b84fd830699f874d2936146f0e93197db610c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47778
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
Variables in the storage storage class and variables with a storage
texture type must have an access attribute applied to the store type.
https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables
A variable in the storage storage class is a storage buffer variable. Its
store type must be a host-shareable structure type with block attribute,
satisfying the storage class constraints.
Fixup tests, including those that were producing warnings about `var <in>`
The WGSL writer seems to want to put a newline after every decoration block, leading to some ugly output. I'll fix this as a separate change.
Fixes: tint:531
Fixes: tint:692
Change-Id: If09d987477247ab4a7c635f6ee6e616a06061515
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47763
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
The canonical type was stopping at the first encountered type::AccessControl, which meant the alias in access<alias<i32>> was not being unwrapped.
Bug: tint:705
Change-Id: Idcbd824808d8ee3098fb1861add5014d7d46b0ad
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47762
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
type::Array was the only type with special handling in that we'd resolve
all array types first in ResolverInternal, then proceed with resolving
the AST in order of declaration. This CL removes this special handling,
and instead, we now process Arrays as we process ast::Variables of
array type. This change also allows us to pass down a Source location
for validation messages when processing Arrays.
Updated some Builder tests that weren't creating a variable of the array
type they declared.
Bug: tint:707
Change-Id: I8483b3a979bc1e5e04feb1ca4d281e96e9e654be
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47825
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
We define the canonical type as a type stripped of all aliases. For
example, Canonical(alias<alias<vec3<alias<f32>>>>) is vec3<f32>. This
change adds Resolver::Canonical(Type*) which caches and returns the
resulting canonical type. We use this throughout the Resolver instead of
UnwrapAliasIfNeeded(), and we store the result in semantic::Variable,
returned from it's Type() member function.
Also:
* Wrote unit tests for Resolver::Canonical()
* Added semantic::Variable::DeclaredType() as a convenience to
retrieve the AST variable's type.
* Updated post-resolve code (transforms) to make use of Type and
DeclaredType appropriately, removing unnecessary calls to
UnwrapAliasIfNeeded.
* Added IntrinsicTableTest.MatchWithNestedAliasUnwrapping to ensure we
don't need to pass canonical parameter types for instrinsic table
lookups.
* ProgramBuilder: added vecN and matMxN overloads that take a Type* arg
to create them with alias types.
Bug: tint:705
Change-Id: I58a3b62538356b8dad2b1161a19b38bcefdd5d62
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47360
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Add semantic::Swizzle and semantic::StructMemberAccess, both deriving from MemberAccessorExpression
Add semantic::Function::Parameters() to list the semantic::Variable parameters for the function.
Change-Id: I8cc69f3738380c14f61d051ee2989be6194d148d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47220
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@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>
Also update comments and arch design to remove references to the
Validator.
Bug: tint:642
Change-Id: Ic0b4779ae4712a393ff209014daf25e23f32be6d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47061
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>
An tint-internal decoration used to add metadata between a sanitizer transform and a backend.
Will be used for declaring backend-specific intrinsic calls.
Change-Id: Ia05ba7dada0148de2d490605ba4d15c593075356
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46868
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Derives from semantic::Expression.
Maps to ast::IdentifierExpressions that resolve to a variable.
Breaks pure-immutability of semantic::Variable, as we have discussed in the past.
Change-Id: I362d4d1ed61291282a60626b84fb15566655fb14
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46627
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
* Moved global variable resolving logic to new function Resolver::GlobalVariable, and moved validation logic there.
* Moved global variable-related tests to resolver tests.
* Fixed many tests that started failing after this change, mainly because many globals were declared with no storage class. I set most of these to "Input".
Bug: tint:642
Change-Id: I0f8ea2091ed2bb3faa358f9497cd884b2994a40f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46940
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@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>
Sarah
Ben Clayton
James Price
Antonio Maiorano
Ryan Harrison
Alastair F. Donaldson