By making nodes reachable, the resolver has now caught a whole lot of additional problems, which have been fixed in this CL.
Some of these broken tests were attempting to use private and workgroup variables as function-scope declarations.
This is not legal, and these have been moved to module-scope variables.
Bug: tint:469
Change-Id: I09c1a8e72a33d3e4df13554e8b07d0a169fcf575
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48226
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Module scope variables of the private and workgroup storage class are
not currently implemeted.
Change-Id: Ibb73faec5b5bcb11199e07ba4d7aa4df9c875450
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48221
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Previously all uses of reentrant object creation in Dawn native
needed to manually AcquireRef. Change them to use CreateFooInternal that
returns a ResultOrError<Ref<>> and are renamed to CreateFoo.
Bug: dawn:723
Change-Id: Ifcda3659d02cc5a4c63c248dc53af7fee7c4a61d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/46626
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Detached AST nodes will become an ICE, so only create nodes if they are going to be used.
Bug: tint:469
Change-Id: I29b3275a355e30c934e6e508185a19981dcc8a42
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48050
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
EmitControlBarrier() was calling MakeOperand() to create ast::Expressions that held a ScalarConstructorExpression, which held a IntLiteral.
The literal value was then taken, and the rest was discarded.
Detached AST nodes will become an ICE, so do the work to find the literal value.
Bug: tint:469
Change-Id: I522bfe8db84e853e189c714b18598feb0d49e58b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48049
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Instead of directly fowarding to ReturnStatement, pass the parameter through `Expr()`.
Also add a no-arg overload.
This means we can write `Return(1);`, `Return("a")`, `Return()`
Change-Id: I8f5df630f540f9cecdf82d24e2810a0844e025e8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48046
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
As of https://github.com/gpuweb/gpuweb/pull/1352 the spec indicates that
GPURenderPassColorAttachmentDescriptor and
GPURenderPassDepthStencilAttachmentDescriptor should use .view rather
than .attachment to indicate the TextureView associated with the
render pass attachment.
Bug: dawn:762
Change-Id: I70d615e19d8e7aae5b26aa5965c7109289ab868b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47902
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
This is not valid. Will become an ICE.
Bug: tint:469
Change-Id: I02c0eea16daf7d83f4d6c251e06d9cac0dfd7ce4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47777
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Tint now sorts struct members with entry point IO attributes.
Bug: tint:710
Change-Id: I7bb37cab2f101fbcf55dd8b42ee3b3af1250bf1a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47840
Commit-Queue: Brandon Jones <bajones@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Brandon Jones <bajones@chromium.org>
The CloneContext will currently de-duplicate any pointers that are cloned multiple times, however this isn't ideal behavior for AST nodes.
AST nodes must be unique in the tree, so a non-transform clone of the AST from one program to another should only ever Clone() called once per node. Hitting the map for these is inefficent, worse still, there are cases in the transforms where we actually *want* to create N copies of the same source node. The current implementation makes this impossible.
This change introduces ShareableCloneable, a new base class that derives from Cloneable.
Repeated calls to CloneContext::Clone() for Cloneable pointer types will now produce a new, unique instance per call.
Repeated calls to CloneContext::Clone() for ShareableCloneable pointer types will de-duplicate as before.
type::Type objects are shared, want deduplication while cloning, so now derive from ShareableCloneable.
ast::Node continues to derive from Cloneable.
Bug tint:469
Change-Id: I5ca906d507de6271d5d715cfdc962a55b721e821
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47776
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
This is a trivial mapping to/from WGSL in all cases.
Bug: tint:478
Change-Id: I7f21a2392543a880906b54fddbdb8bbd149a526e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48140
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
[Is,As] contain a static_assert() that checks a cast is actually possible (TO -> FROM share a common base class).
This has been extremely valuable - it's caught numerious impossible casts due to stupid mistakes - however it makes certain generic templates impossible to write.
Add a compile-time FLAGS argument to Is() and As(), which accepts a new kDontErrorOnImpossibleCast flag.
When specified, this static_assert will always pass, allowing impossible casts to be attempted.
Change-Id: I5ff434b329c04d007f4e6976301bf30c73ab3f8d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47775
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This reverts commit 5c9b6a8f81abf54f1b8ba1c6d54c4037ecadba52.
The issue has now been fixed in Tint and rolled into Dawn.
Fixed: dawn:758
Change-Id: Id7deb6cea9f22ee8c5f345a28d2d37499b47d692
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/48080
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Error: explicitly-defaulted constructor cannot have default arguments
This broke chromium, and needs fixing before we can restart the autorollers.
Change-Id: I4b04dd127b5fb7cf84e5782e07bcf68b2befc904
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48041
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Fixed: tint:673
Change-Id: I5a58d9e504446ccff724e368b5ea2cf835d2271b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47768
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Allows bash scripts to share the same name as the directory.
Add fix-tests bash script.
Change-Id: Iaf1943d50ec1fd3f382a2c7823fb7cdd13b1d9a2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47766
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
This test landed simultaneously with a change that checks that storage buffers have an access qualifier, leading to broken tests.
Also fix lint issues that have crept in.
Change-Id: I4a7f0bc67d8d3e7170ddea117342a510741a495c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48040
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Almost all transforms should clone all symbols before doing any work,
to avoid any newly created symbols clashing with existing symbols the
source program and causing them to be renamed.
The Renamer is the exception to this, and so an optional flag is used
to prevent automatic cloning of symbols for this transform.
Bug: dawn:758
Change-Id: I84527a352825b2eaa43eabe225beb9e0999bf048
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48000
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Produces ugly code for the common case.
Change-Id: I8e5b1215e19fc6461dc40b8a91922db25f9cbd76
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47764
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@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>
By comparing the old substring to the new full body, we can do a pretty
good job at creating a new substring.
This is not an exact science. Always carefully examine the newly
generated strings for correctness.
Change-Id: I8bdf591539a32ec42d0444aa054d88a933e4d250
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47765
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
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>
Move transform::Transform::Output to transform::Output.
There's no need for this to be an nested class, it stutters, and it also
prevents Dawn from forward declaring it.
Add move assignment operator to DataMap.
Change-Id: Ibe1af03abc1a872790d20ee6ec8cf18a511ea0b4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47772
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Adds additional modes to the Renamer for renaming just HLSL or MSL
keywords. The sanitizers no longer rename reserved keywords
automatically, which isn't usually necessary anyway since Dawn renames
all symbols.
The tint executable automatically renames reserved keywords when
targeting HLSL and MSL. It now also has an option to rename
everything, which is useful for testing the renamer.
Change-Id: Idbfd53226805e851050024402be8d8f251b88707
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47960
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
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>
Matches most recent spec changes. setBlendColor has been marked as
deprecated.
Bug: chromium:1199057
Change-Id: I4584ce789bd7d14401244509d5ada62a46236a5d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47901
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
Several of the enum names have changed recently. Update them to match
the spec and mark the older ones as deprecated.
BUG: chromium:1199057
Change-Id: I7a29588dd18b8fb738773c2478b173093f2aa834
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47860
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Assert in each AST constructor that symbols belong to the program of the parent.
Bug: tint:709
Change-Id: I82ae9b23c88e89714a44e057a0272f0293385aaf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47624
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Assert in each AST constructor that child nodes belong to the program of the parent.
Bug: tint:709
Change-Id: Icc89b69691d099e358ff632a0ca6fd7943cb0193
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47623
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>
A Dawn CL landed at the same time as a Tint roll in Dawn, causing
failures in a large number of D3D12 use_tint_generator. Disable testing
of that configuration entirely while we investigate the issue and fix
it.
Bug: dawn:758
Change-Id: I6c897efc637821a20281c91668889ae95c1c2b1a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47774
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Don't end the EXPECT_THAT() with `<< ToString(p->builder(), fe.ast_body());`
This string is already emitted in the error, and calling ast_body() a second time triggers an assert as the builder is already finalized.
Change-Id: Ied6aca73937e86aa6ce2d8cff8a09f67b5082349
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47767
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
HLSL interface matching rules impose additional requirements on the
order of structure members. We now sort members such that all members
with location attributes appear first (ordered by location slot),
followed by those with builtin attributes.
Fixed: tint:710
Change-Id: I90940bcb7a5b9eeb1f50f132d406d4cf74e47ea2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47822
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Fix a test that did not have the position builtin as a vertex shader
output.
Change-Id: I8dceba59d8327938e725e7d5e62b4b9c43695710
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47820
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Pre-clone the program's symbols at the start of the transform,
otherwise the original program's symbols may get mangled. This causes
problems for Dawn when the entry point name is changed.
Change-Id: I414c798fb5f51afe44e8b97619f77452f97f0782
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47824
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Changes I/O to use function parameters and return values, removes
unnecessary "-> void" return types, and changes "const" to "let".
BUG: dawn:755
Change-Id: Iabbfcc280fae37d73cba6a2f7e2215ed579a04e0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47700
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
