This reverts commit 594075a2f0.
Reason for revert: This is not a complete solution. It does not cover assignment of variables of the same struct type, but of different access types. For example:
```
[[block]] struct S{ i : 32; };
var<storage> gr : [[access(read)]] S;
var<storage> gw : [[access(write)]] S;
fn f() {
var a : S;
a = gr;
gw = a;
}
```
With my CL, we currently generate invalid assignments because the new types of 'S' for 'gr' and 'gw' are now all different types. We would need to generate functions to assign between them.
In the end, we will drop this strategy, and instead, Dawn will be modified to not rely on names.
Original change's description:
> Add DuplicateStorageStruct transform
>
> This transform is currently required by Dawn for it's GLSL backend so
> that SPIRV-Cross does not end up renaming the structures internally,
> which it does to fix aliasing problems. We can remove this transform
> if/once Dawn using binding numbers rather than names for uniform/storage
> buffer data.
>
> Bug: tint:386
> Bug: tint:808
>
> CloneContext: allow insert before/after of a node marked for removal
> Change-Id: I3ff3b37bca62db07d5c759250dd4777e279b7a4f
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51403
> Commit-Queue: Ben Clayton <bclayton@google.com>
> Kokoro: Kokoro <noreply+kokoro@google.com>
> Reviewed-by: Ben Clayton <bclayton@google.com>
TBR=bclayton@google.com,amaiorano@google.com,noreply+kokoro@google.com
Change-Id: I695347b0ffe8be23b9dc44885af378be56512406
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: tint:386
Bug: tint:808
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51500
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This transform is currently required by Dawn for it's GLSL backend so
that SPIRV-Cross does not end up renaming the structures internally,
which it does to fix aliasing problems. We can remove this transform
if/once Dawn using binding numbers rather than names for uniform/storage
buffer data.
Bug: tint:386
Bug: tint:808
CloneContext: allow insert before/after of a node marked for removal
Change-Id: I3ff3b37bca62db07d5c759250dd4777e279b7a4f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51403
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: 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>
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>
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>
This has been moved into the Spirv sanitizer, and Dawn is now using
this path.
Change-Id: Iffcbbc1c84e6bad0ebc51ded82d998bb307e2d6d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50564
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
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>
Don't create disjoint AST type nodes.
Instead use a new bespoke type hierarchy that can Build() the required
AST nodes.
Change-Id: I523f97054de2c553095056c0bafc17c48064cf53
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49966
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>
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>
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>
Adds a transform to reclassify single-plane texture_external types into 2d sampled textures. Adds a unit test for the transform.
Bug: dawn:728
Change-Id: Id1f88565aeacbfea47b140181c78ad122edbdae8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49641
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Have ast::Struct and ast::Alias derive from it.
NamedType currently derives from ast::Type, but this might change in the future.
Bug: tint:724
Change-Id: I46ab7751c89176f4dec34d63247d2459d02ab6bb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49525
Kokoro: Kokoro <noreply+kokoro@google.com>
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>
Remove the Generator::GenerateEntryPoint() APIs as they were mostly
unimplemented and not used by anything except the Tint sample app,
which now uses the new transform.
Change-Id: I1ccb303d6c3aa15e622c193d33b753e22bf39a95
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49160
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Also create it in wgsl parser. With this, we now create all the AST type
nodes in the wgsl parser.
Bug: tint:724
Change-Id: I575c9eb0ffbd60c3e7aca0b00227d9833ef65d58
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48962
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
All that remains in the wgsl parser that references sem::Type* are
the register_constructed funcs/types, and adding an ast::ExternalTexture
type.
Also:
* Added specialization of OperatorArrow for type::TypePairs that returns
the value by reference so that operator-> can chain properly.
* In a few places where we expect the type to implicitly cast to a
pointer for a bool expression, e.g. `if (type)` or `TINT_ASSERT(type)`,
I added access to the `.sem` member of the TypePair. I tried adding an
implicit cast to bool, but this results in ambiguity in gtest for
equality comparisons.
* Constified more type pointers in type nodes
* Replaced header includes with forward declares in wgsl/parser_impl.h
Bug: tint:724
Change-Id: Ie0875aa4d4a5e830e3466ac40c63cd185f357200
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48881
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
With typ::Type.
Involves decoupling the use of named types as sub-types of other types,
otherwise ty.array("x", Structure(...)) produces an AST tree with
duplicated AST nodes.
Bug: tint:724
Change-Id: I2b4e904dc23bb554a75d4612f6543f1911f469bd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48693
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Introduces typ::Type which wraps a templated pair of AST and SEM type pointers.
This is a temporary helper to ease migration of the thousands of tests that use the ProgramBuilder over from sem::Type to the new ast::Types.
Bug: tint:724
Change-Id: I4e2643a819cde97947d789fce7a74c251f837a58
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48601
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
- Move headers from libtint_sem_src into libtint_core_all_src so that
there is no circular header dependency between headers of these two
targets. libtint_sem_src is now just a hack to have a different name for
a few .cc files.
- Made libtint_core_src publicly depend on its deps so that headers of
libtint_core_all_src are made visible through it.
- Added spvtools dependencies in a couple places that were mistakenly
removed in a previous commit.
- Moved helpers common to multiple unittest targets out of
tint_unittests_core_src and into a tint_test_helpers target that
all tint_unittests_source_set depend on.
- Ran GN format that reordered some lists a bit.
Bug: None
Change-Id: I544a73d73366be9dd2ac8e56c7593fd9f2b86cf8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48600
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Copy all of the type classes from src/type into ast.
Required the merging of:
* type::Struct into the existing ast::Struct - ast::Struct now has a name.
* type::AccessControl into the existing ast::AccessControl enumerator - The old ast::AccessControl enumerator is now ast::AccessControl::Access
Bug: tint:724
Change-Id: Ibb950036ed551ec769c6d3d2c8fb411809cf6931
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48383
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Adds ExternalTexture type and basic unit tests in accordance with the
currently proposed WGSL specification.
Bug: dawn:728
Change-Id: I7a16a351ff098ba6df5b1e6305c390e3ca1c9d46
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47180
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This makes the .cc files match the header file name, which allows for
the text editor "header flip" feature to actually work. The reason these
files were named this way was because GN doesn't allow name conflicts in
the same source set, despite the files being in different directories.
This change splits the files into different source sets. To do so, we
use GN templates, which also reduces duplication in each target
definition.
Bug: tint:724
Change-Id: I9a7ed3912e4b85b2b38d360805203f3488b86c4c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48160
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This will be used to detect accidental leaks of program objects between programs.
Bug: tint:709
Change-Id: I20f784a2c673d19a04a880b3ec91dfe2eb743bdb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47622
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
The renamer logic in the HLSL writer was inconsistent, and actually
broke some valid shaders.
Change-Id: I8fbddc7e657f5509b18435fdf352a39d83c1b89c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47224
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
With this change, the Validator is fully removed from Tint, including
from it's public API.
Fix: tint:642
Change-Id: Id4867cc3866bb2ea09eff499537d58b938d18f43
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47125
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Used to used to replace calls to arrayLength() with a value calculated from the size of the storage buffer.
Bug: tint:185
Change-Id: If7ddc8dad2ed3d20c1d76b5f48bfd2c7634f96e2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46877
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Used to breakdown storage buffer reads and writes into primitive loads and stores.
Bug: tint:683
Change-Id: I92983deef7485bc1a6a5d9921ec82f8e19b09744
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46876
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Now that all validation has been moved to the Resolver, we can delete
the Validator. This change removes everything except validator.h with
the public no-op API. We can remove this once we remove dependencies on
this public API in Dawn.
Bug: tint:642
Change-Id: I644cd900615509c9cdd57c375c6217a50126e36c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47023
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Registering a new Symbol with the NameFor() of the source symbol creates
a new *named* symbol. When mixing these with unnamed symbols we can have
collisions.
Update CloneContext::Clone(Symbol) to properly clone unnamed symbols.
Update (most) the transforms to ctx.Clone() the symbols instead of
registering the names directly.
Fix up the tests where the symbol IDs have changed.
Note: We can still have symbol collisions if a program is authored with
identifiers like 'tint_symbol_3'. This will be fixed up in a later
change.
Change-Id: I0ce559644da3d60e1060f2eef185fa55ae284521
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46866
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@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>
After the transform, an entry point's parameters will be aggregated
into a single struct, and its return type will either be a struct or
void. All structs in the module that have entry point IO decorations
will have exactly one pipeline stage usage.
This will be used to sanitize entry points for the MSL and HLSL
generators.
Change-Id: I7d2fcfba961d2e20326086964207747d573b6e05
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46500
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
A transform to replace binding points and access control flags.
Required by Dawn for the HLSL and MSL backends
Fixed: tint:104
Fixed: tint:621
Fixed: tint:671
Change-Id: Ic6ccc4a8a7724697cc4af0fad25394a1973eeb22
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46262
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Skia changed to only add -Weverything for Skia files so this warning is
no longer needed.
Bug: dawn:706
Change-Id: I7ac111cfaf580ee0868fb318f953fe983ee17df2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45607
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This isn't in the WGSL spec, nor is it generated by readers.
This was only used inside the SPIR-V writer, but this remaining usage was removed in the parent change.
Change-Id: I1bbfde67dc760b761af010a7a144dccb52369148
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45343
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Requiring a temporary stack-allocated ast::Literal is an unpleasant requirement to generate a SPIR-V constant value.
GenerateU32Literal() was also creating an invalid AST - the type was U32, yet an an ast::SintLiteral was used.
Instead add Constant for holding a constant value, and use this as the map key.
This also removes the last remaining use of ast::NullLiteral, which will be removed in the next change.
Change-Id: Ia85732784075f153503dbef101ba95018eaa4bf5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45342
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Chromium uses clang 13.0 that likely removes the warning while Skia uses
clang 12.0 that still has the warning. Temporarily skip the
-Wno-return-std-move-in-c++11 in Chromium only while Skia changes to not
add warnings to it's third_party dependencies.
Bug: dawn:706
Change-Id: I625f0046204328dcf2cfb1eb9824f8a4a928b8ff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45240
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Implements https://github.com/gpuweb/gpuweb/pull/1447
SPIR-V Reader is still TODO, but continues to function as the offset
decoration is still supported.
Bug: tint:626
Bug: tint:629
Change-Id: Id574eb3a5c6729559382812de37b23f0c68fd406
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43640
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
This will allow Tint's dependent to depend on libtint without GN
discovering Tint's test and try to build them. In particular it will
help use Tint in Dawn in Skia's standalone build which doesn't have
//testing.
Bug: dawn:706
Change-Id: Idd28662b89aa75df7704eaae205328dce0b96fef
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44540
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Antonio Maiorano
Alastair F. Donaldson
Ben Clayton
James Price
Brandon Jones
David Neto
Corentin Wallez