Inline the `continuing` block in the places where `continue` is called.
Simplifies the emission, and fixes emission of `let` statements in the loop.
This fix matches the same approach in writer/hlsl.
See: https://dawn-review.googlesource.com/c/tint/+/51784
Fixed: tint:833
Fixed: tint:914
Change-Id: If4d8cde62dfaf8efa24272854ca7ff5edc0a8234
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55341
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Mostly just deleting unneeded code, and a few additional cleanups as a
result.
Bug: tint:697
Change-Id: I31ceea93feb34994f51a1b6d294a35cf0c127447
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55282
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Generate a uniform buffer that will receive the lengths of all storage
buffers, and use this to implement calls to arrayLength(). The
transform is provided with a set of mappings from storage buffer
binding points to the corresponding index into the array of buffer
lengths. The transform reports whether it generated the uniform
buffers or not.
Use this transform from the MSL sanitizer, using the binding number as
the index into the array. This matches the behavior of spirv-cross,
and so works with how Dawn already produces this uniform buffer.
Bug: tint:256
Change-Id: I2682d2d024e8daa30f78270b8cfb6bbb32632133
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54480
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Zero the workgroup memory for all backends.
We can probably disable this for the backends that support workgroup zeroing, but that's an optimization we can perform later.
Fixed: tint:280
Change-Id: I9cad919ba3a15b8cedfe6939317d1f6b95425453
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55244
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Zero initializes all referenced workgroup storage classed variables used by each entry point.
Bug: tint:280
Fixed: tint:911
Change-Id: I3fca26a10f015f08fedef404720bbe6fd7b343a9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55243
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Instead of a ConstantBuffer.
HLSL requires that each structure field in a UBO is 16 byte aligned.
WGSL has much looser constraints with its UBO field alignment rules.
Instead generate an array of uint4 vectors, and index into this, much
like we index into [RW]ByteAddressBuffers for SSBOs.
Extend the DecomposeStorageAccess transform to support uniforms too.
This has been renamed to DecomposeMemoryAccess.
Change-Id: I3868ff80af1ab3b3dddfbf5b969724cb87ef0744
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55246
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
If during clone, we register a type, function or global declaration, we could end up with the declaration held twice by the AST Module.
AST nodes must only be referenced once.
Change-Id: I5c517699ea80422800639088b97a50ba9ac27b70
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55245
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Removing parsing support for the 'in' and 'out' storage classes is
enough to prevent anyone from using the old syntax. The Input and
Output storage classes will remain in the AST for now, as the SPIR-V
reader still has codepaths that use them, and the SPIR-V writer
currently still relies on them.
Bug: tint:697
Change-Id: Ifef9eda8bcbf2f243b1e1d8d4fab25784cd3f80e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54841
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Storage buffers are emitted as `ByteAddressBuffer`s in HLSL, so we have to jump through hoops to support atomic ops on storage buffer atomics.
Workgroup atomics are far more conventional, but very little code can be shared between these two code paths.
Bug: tint:892
Change-Id: If10ea866e3b67a093e87aca689d34065fd49b705
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54651
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@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>
This is required for implementing module-level conversions in the spir-v
backend (upcoming CL).
Bug: tint:865
Change-Id: I7fd38c6b1628c791851917165991bc247fc113c2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54740
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This will be relied on by the upcoming arrayLength transform.
Update test expectations.
Change-Id: Ib74b647abcd6f4393f9899ce40bbf06f6e53e7f4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55180
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@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>
Allows for reflection of the specific textures that a sampler has
sampled.
BUG=tint:699
Change-Id: Iba47baf5c99c4d03671caf2c01747bd6ad12b1e2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54901
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
The WorkgroupSize builtin decoration applies to a composite constant.
Because WGSL does not yet support specializable constants for this,
use the *default* values for that SPIR-V spec constant.
Update end-to-end test expectations.
Fixed: tint:503
Change-Id: I012b316d13544ab9282e3276b58906327adab133
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41960
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
Some overload mismatches just make no sense without this
Change-Id: I2827808ab7dfd9e2d179b5207da8ad3c2ce56d99
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55040
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
- When storing to sample_mask output, write to the 0th element
- Only make a return struct if it has members
- Adjust type signedness coercion when loading special builtins.
- Adapt tests
- Update expectations for end-to-end tests
- Handle sample_mask with stride
Input variables normally don't have layout. But they can have it
up through SPIR-V 1.4.
Handle this case in the SPIR-V reader, by seeing through the
intermediate alias type created for the strided array type.
Bug: tint:508
Change-Id: I0f19dc1305d3f250dbbc0698a602288c34245274
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54743
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@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>
Dynamic indexes are limited to references to matrices and arrays
https://github.com/gpuweb/gpuweb/pull/1801
Bug: tint:867
Change-Id: I114daa053c8a4ffd23ce784ac4538567a551cc21
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54701
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Sarah Mashayekhi <sarahmashay@google.com>
Combined with the new PadArrayElements transform, arrays with strides
are now correctly emitted.
Fixed: tint:182
Fixed: tint:895
Change-Id: I26a1be94dee6e4c9d9747c8317a932fc1fb3c810
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54640
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@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>
Fixes issues with using arrays as function return types.
Fixed: tint:848
Change-Id: Iee8af0f2cea9d19e448176446c6599be2bd32316
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54321
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
And replace the MSL writer's logic to do this with the transform.
We need to do the same thing in HLSL, and in the future GLSL too.
Partially reverts fbfde720
Change-Id: Ie280e011bc3ded8e15ccacc0aeb12da3c2407389
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54242
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Previously the Clone() of the AST would clone all the functions, globals
and type declarations in a temporary vector, then assign this to the
ast::Module. This meant that adding new module-scope declarations inside
callbacks of ReplaceAll() would place them right at the top, before any
of the cloned declarations.
As top-level declarations are not statements, ensuring that a new object
comes before the current ReplaceAll() declaration is surprisingly
tricky.
With this change, we can now safely assume that calling
ProgramBuilder::Var(), ProgramBuilder::Func(), ProgramBuilder::Alias()
or ProgramBuilder::Structure() inside a ReplaceAll() will add that
module-scoped declaration before the currently processed top-level
declaration.
Change-Id: I52772fdc85940c8ac8d941fbd53374a4dd64a9f4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54320
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
And fix issues where global variables would not be emitted unless they were transitively referenced by an entry point.
This change requires crbug.com/tint/697 to be fixed before landing.
Change-Id: I712bd9d369e08c9a3cdfb0f114c3609584f91f28
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54241
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
No other outputs require signedness conversion, so we can simplify
one part of the code
Bug: tint:508
Change-Id: I71a111b312a0b44f780155fcf6b4420742cc9d88
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54483
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: 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>
It's an array in Vulkan SPIR-V, but a scalar u32 in WGSL.
Handle signedness change.
Note that input variables can't have an initializer, so that
doesn't need to be handled.
Bug: tint:508
Change-Id: I7cf4228b31f9c42e4e4436d78cbb1eb0c8196cd5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54482
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Also, complete the list of builtins that need signedness conversion.
Bug: tint:508
Change-Id: I785f283a435e07f9c08e2eb23245883aa272b6b1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54463
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Update the plan for pipeline I/O.
Bug: tint:508
Change-Id: I263d954403134a78106b3372ec9df3d9d868ef1d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54462
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
We have the end-to-end test-runner which validates all this stuff.
There's no need to also Validate in the unit tests.
Change-Id: I8fbc1ecd395efcedec4aa41085e691c88f3b66a5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54160
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@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>
Wrap the texture expression in parentheses when it has lower
precendence than the function call operator.
Cast integer coordinates to unsigned integers as required by MSL.
Fixed: tint:536
Change-Id: I957e6be3c51044959e25e0be96c2d2c65db18187
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53962
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
When moving private and workgroup variables into the entry point,
generate pointers to pass as arguments to sub-functions on demand,
instead of upfront. This removes a bunch of unnecessary dereferences
for accesses inside the entry point, and one function variable.
Change-Id: I7d1aabdf14eae33b569b3316dfc0f9fbd288131e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54300
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
The old non-pointer argument overload remains, but is now deprecated.
Bug: tint:806
Change-Id: Ic917ccec0f162414ce1b03df49e332ad84d8060f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54061
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
arrayLength() will take a pointer to a storage buffer array.
This pointer may pass through function scoped let statements.
To make this intrinsic easier to generate, inline the pointer lets and
remove chains of &*&*.
Bug: tint:806
Change-Id: Ib2c79a9c38cba7391cbb4313986af9a72b0f0435
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54060
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
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>
Functions cannot be called main, and we do not run the rename reserved
keywords transform as part of the sanitizer.
Change-Id: Iea67fe08c7e94e473d622b7f349caf8d09f41784
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54101
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@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>
When generating the store type for a variable, follow SPIR-V
instructions directly. Avoid using the SPIRV-Tools optimizer because
it deduplicates structures that differ only in non-semantic annotation (e.g. member names)
Bug: tint:737
Change-Id: I9503be4ae0a95a517d595da23e6ce2397e4fc9c1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53560
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@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>
Combines the calls to ty.alias() and AST().AddConstructedType()
Change-Id: I84eaf6275d37141cf8973db7e04d25d9cb9c6351
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53801
Commit-Queue: Ben Clayton <bclayton@google.com>
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>
Roughly 4x faster than validating with the MSL executable.
Change-Id: I6566fa29622475c459eb3a988a842a9e19d4be6f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53680
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
name intersect
v-2000: Function names must not have any intersect with module-scope
variable names.
A declaration must not introduce a name when that identifier is already
in scope with the same end scope as another instance of that name.
A module-scope variable and a function have the same end scope ie.
end of the program.
bug: tint:260
Change-Id: I95321d3e6c04b0344c3acfb7b77cb483b3659728
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52820
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
- Add missing DescriptorSet and Binding decorations.
- Add missing Phi inbound edge
- Add a preamble with an OpMemoryModel instruction
- Add a preamble and an empty entry point that is not involved with the
test (sometimes)
- Disable dumping of test with known bad output (tint:863, tint:98)
- Fix storage classes on variables
Bug: tint:863, tint:98
Change-Id: I56b92e8951f5749e094424f8e2da1a2396b5c10c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53261
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
The argument order between WGSL and SPIR-V is different (condition is first in SPIR-V, last in WGSL)
Fixed: tint:560
Change-Id: I56c659c441292e05f71a24d96dbc9f93f25b71f0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53620
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Much like uintN, intN, floatN - boolN is far more common, and easier to read than vector<bool, N>
Change-Id: I51f9edc003c590266316d3eba286ca2f6882da10
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53390
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
That would cause OOB reads.
Fixed: tint:836
Change-Id: I6e44ab5d140e99a701c190c13ed6aaba762e19d4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53660
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>
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>
ProgramBuilder is about as internal as you can get - this really should not be public.
MSVC seems to try an instantiate some of the template methods in ProgramBuilder when it is included externally (for PCH or DLL exports perhaps?), and failing with bizzare error messages that contain no point-of-instantiation.
As this header was never intended to be public in the first place, detect and error if the tint.h include guard is found while processing program_builder.h, and fix up the couple of bad transitive includes.
Fixes tint -> dawn autoroller
Change-Id: Ic9a81a44ab1b4a29a7297b94bbf85bcfdb1310b5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53384
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
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>
and allow them in any order.
We're about to add Yet Another optional parameter - access control.
For style, we probably want this field before the decorations, however
that means adding more default values for all the cases where we don't
need to specify the access control.
Instead use some template magic to allow extra parameters to be
specified in any order, and entirely optional, with sensible defaults.
This keeps things readable and without huge code refactorings.
Bug: tint:846
Change-Id: I4f61eb765de63ad98ae786d187d2bd1b42942756
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53087
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Add a config parameter for the CanonicalizeEntryPoint transform that
selects between emitting builtins as parameters (for MSL) or struct
members (for HLSL).
This fixes all of the shader IO issues in Tint's E2E tests for MSL.
Fixed: tint:817
Change-Id: Ieb31cdbd2e4d96ac41f8d8515fd07ead8241d770
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53282
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
This removes the need to check for an empty function body and means we
can unconditionally build the function constant initializers, instead
of deferring this work via captured lambdas.
Change-Id: I1571a065fb5885e2b29f0ffb41940a71e5b3f121
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53281
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
This fixes constructors for structures that contain padding members
due to explicit layout attributes.
Also fix one test that was wrongly using an identity type constructor
for a structure.
Fixed: tint:853
Change-Id: I0a3e84fcd7c6a7f2ad92a4970ed11378e6ce2465
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53240
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
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>
Specifies that the overload can only be used in the specific stages.
Actually validating this with the IntrinsicTable is TODO.
Bug: tint:657
Bug: tint:832
Change-Id: I11ffefee22e5f26103f008b23d16066a2a3ba90d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53050
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
* Add new template utilty functions, including the ability to split out multiple files.
* Add basic printing of the semantic overloads.
* Add a pointer from the overload to the function
* Change TemplateArguments from a list of FQN to a list of interface{} (any). This is required as once the overload is permutated, some arguments will need to hold integers.
This will be used by the test generator.
Bug: tint:832
Change-Id: Idbfbe85e52489b31850cbb0ee7430bb4b021530e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53046
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Add a deprecated warning for the old syntax.
Bug: tint:846
Change-Id: Id46af28054b32c2ba6f1b96556da023f46a6b87b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53048
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Add `[[internal]]` decoration on `storage_class.handle` - its not an entry that should ever appear in WGSL.
Bug: tint:832
Change-Id: I210f64c495bf37a8f48422919248806e7b096638
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53045
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Nothing was limiting the texel format to i32, u32, f32, and so the resolver was accepting any type here, and things would break in the backends.
Also limit texture access controls to read or write. read_write is not supported for textures currently.
Change-Id: I41c54a9b285800ad89aa1c4ea07e142ba0506384
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53044
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@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>
Formerly, tmpnam_s was used to ensure a unique file name, but then we'd
append an extension to it, invalidating its uniqueness. Instead, we now
do our own uniqueness check by attempting to create a unique empty file
for write with new names until it succeeds.
Bug: tint:812
Change-Id: I90b85074ad2281f9904f24e8fddda80d67e61ba7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53061
Auto-Submit: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Using WaitForMultipleObjects on the handles to stdout and stderr streams
is apparently not the right way to do this. Instead, we create 2
threads, one that reads stdout, and one that reads stderr, and we wait
on those threads, along with the process handle.
Bug: tint:812
Change-Id: I5975e4d649b58d0f8bffb76dec5a3f4079513e04
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53060
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Performs output validation with spirv-val for SPIR-V (as before), HLSL
validation with DXC, and MSL validation with the Metal Shader Compiler.
Disable HLSL tests that fail to validate
Bug: tint:812
Change-Id: If78c351b4e23c7fb50d333eacf9ee7cc81d18564
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51280
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This allows them to be used in various places that WGSL allows, such
as function return types and parameters, and as the type of the RHS of
an assignment.
Fixed: tint:814
Fixed: tint:820
Change-Id: Idb6a901b9a34e96bb9733cc158191e7b3bafaa0e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52844
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: 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>
Sometimes a value can come from a block which does not appear
in the structured block order. That block will never execute,
so we can safely use the null value instead.
Bug: tint:804
Change-Id: Idc1a6c4f76f14a1d76919a95e5a65e56018ce68f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52840
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: David Neto <dneto@google.com>
The LHS should be wrapped in parentheses if it has lower precedence
than the access. This fixes issues with pointer dereferences followed
by array accesses, where we were previously generating *a[i].
Change-Id: I639f94b642f32119350b0a0d23f4ff2a5d6c3c25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52843
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>
Pull the HLSL transformation out to a standalone transform that can be
used by both HLSL and MSL.
The new E2E tests do not yet pass for MSL because they produce array
assignments, which will be addressed in the next patch.
Fixed: tint:826
Change-Id: Idc27c81ad45e3d4ab96d82663927d2fc1384618e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52842
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>
Add a template file to generate parameter_usage.h and
parameter_usage.cc when using tools/intrinsic-gen
Bug: tint:832
Change-Id: I0ca4d092fdcda7d7846b968d43202f34450e516d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52644
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Add a template file to generate intrinsic_type.h and
intrinsic_type.cc when using tools/intrinsic-gen
Bug: tint:832
Change-Id: I35ddfd497d6ab424dc49a18a9fd7e5eb7f8363b0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52643
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Change-Id: I01e8957496cea7e406f6381517cc04aa4f05d8fb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52801
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: David Neto <dneto@google.com>
Change the GenerateGLSLStd450 method to GetGLSLStd450 and have it
return the import id. That's the only interesting use.
Change-Id: I4fc70e702af160bc797680e5e75e9742e9e24f16
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52421
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
The definition file for the WGSL intrinsics.
Will be parsed by a new `intrinsic-gen` parser to generate the IntriniscTable and tests.
Bug: tint:832
Change-Id: I3c523b6d86faa59150e2b6fb302d27c06f0e3cbe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52503
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Signed zeros are emitted.
Subormal numbers are emitted as hex float.
Handling Inf and NaN is unresolved in the spec.
See https://github.com/gpuweb/gpuweb/issues/1769
NaN tests are disabled, due to platform dependence.
Windows x86-64 seems to always set the high mantissa bit
on NaNs.
Fixed: tint:76
Change-Id: I06c69b93b04c869a63ec2f574022996acc7a6dbe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52321
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
MaybeEmitCombinatorialValue() didn't check that MakeOperand() or RectifySecondOperandSignedness() didn't error, leading to ICEs.
Bug: crbug.com/tint/804
Change-Id: Ic78487a70a591e718c7b5936c6678e7a19c4b626
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51927
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Previous logic relied on the fact that for most operations, for a given
lhs type, there was exactly one rhs type allowed, except for
multiplication. This is no longer true in the spec, so making this test
work is more complex now. Instead, we simplify the test by having it
take the cartesian product of all possible (type * type * op), and
skipping any triplet not found in the all_valid_cases table.
Bug: tint:376
Change-Id: Icc76158823630e34dc8f25dfb0a0827545693108
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52322
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
The LHS should be wrapped in parentheses if it has lower precedence
than the access. This fixes issues with pointer dereferences followed
by member accesses, where we were previously generating *a.b.
Fixed: tint:831
Change-Id: I8a194ad4f54c80a01c24eb983ec8064037575216
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51963
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
Inserts a new node at the end of a (potentially empty) list.
Change-Id: Ibd487b995aee4a3ae2c3d17b5eb6e8562c43a1c4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52320
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>
Inserts a new node at the start of a (potentially empty) list.
Change-Id: Ie2896fa3e1beabeb89f64e69a84091986ffbf7a2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51369
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: 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>
This fixes issues with passing constant arrays to functions.
Change-Id: I6e2f1c3f64df836c0b6a55ab925cf3c2bc317733
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51861
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>
Performs basic peephole optimizations on the AST.
Use in transform::Hlsl.
Required to have the DecomposeStorageAccess transform operate correctly with the output of InlinePointerLets transform, specifically when declaring `let` pointer expressions to storage buffers.
Fixed: tint:221
Fixed: tint:492
Fixed: tint:829
Change-Id: I536390921a6492378104e9c3c100d9e761294a27
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51921
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Those tests already exist under src/ast.
Add a test of sem::Texture::dim() instead
Change-Id: I4b77d46b9a4e3aca8292d64e2477bb002ab5c1cb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51805
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
- Translates from OpImageSampleDrefExplicitLod, but the Lod must
be a constant 0, or the reader issues an error.
The requirement for Lod 0 is a constraint from Metal, inherited
by WGSL.
Fixed: tint:425, tint:482
Change-Id: I8fdf10dbd9c5ac3e24398519a28202c84677c38d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51803
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@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>
Inline the `continuing` block in the places where `continue` is called.
Simplifies the emission, and fixes emission of `let` statements in the loop.
Also fix random indenting of intrinsic functions.
Fixed: tint:744
Fixed: tint:818
Change-Id: I06994dbc724bc646e0435a1035b00760eaf5f5ab
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51784
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Add a special-case for pointer-to-array types, where the * and the
variable name need to be enclosed in parentheses in between the array
element type and the size.
Move the `const` qualifier to before the array size.
Add E2E tests to cover all non-handle types used in various places.
Fixed: tint:822
Change-Id: I93b7d6867f92397aa47838ab2c94530b6e634617
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51823
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>
Much like arrays, the SPIR-V writer cannot cope with dynamic indexing of matrices.
Fixed: tint:825
Change-Id: Ia111f15e0cf6fbd441861a4b3455a33b82b692ab
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51781
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
The special-case for zero-valued constructors is unnecessary, as an
empty initializer list already correctly zero-initializes for all
types. This was causing an additional {} to be emitted for empty
structures, which the MSL compiler rejects.
Fixed: tint:821
Change-Id: Ib48c73eadef15b517e14b248229ecfbbfeb13f81
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51822
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>
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>
These were the last two types to migrate away from typ::TypePair.
Bug: tint:724
Change-Id: Ibc03e35db00236081a5792f787864ab69ce0d00c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51665
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Use this as part of the Spirv sanitizer.
Cleans up buggy dynamic array indexing logic in the SPIR-V writer.
Fixed: tint:824
Change-Id: Ia408e49bd808bc8dbf3a1897eb47f9b33b71fdfb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51780
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>
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>
Also contains a UBSAN fix for tests that didn't quite all migrate to ast::I32.
Bug: tint:724
Change-Id: I2a3ad6d6a69595b7da6bb502a87fb655a7f5961a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51486
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Add `transform::InlinePointerLets` - a Transform that moves all usage of function-scope `let` statements of a pointer type into their places of usage.
Make the HLSL writer transform pointer parameters to `inout`.
Fixed: tint:183
Change-Id: I0a7552fa6cd31c7b7691e64feae3170a81cc6c49
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51281
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@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>
Use the WGSL_SPEC_CONSTANT preprocessor macros as parameters to
[numthreads()] when the dimension is overridable.
Remove the macro #undef to make this possible.
Bug: tint:713
Change-Id: Icd927044a64a8b8a2f029f9e2db8168ec6a861de
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51264
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>
Generate an OpSpecConstantComposite instruction decorated with the
WorkgroupSize builtin. Only support a single stage with an overridable
workgroup size.
Bug: tint:713
Change-Id: I139123c0af8326fcbd796cb2fc9d223882206e19
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51263
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: 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>
This functionality is no longer needed, so just remove it.
Change-Id: Iebb4e4105b9e3b4a6a700594298503ad6cb68d31
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51484
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>
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>
Add more E2E tests to cover pointers with different storage classes.
Fixed: tint:815
Change-Id: I224a794cdf60648ce71dc9a0922d489542995be1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51404
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
When the LHS type is not one of the common expression types
Fixes operator precedence issues around pointers and member accesses.
Change-Id: I81b01c0c1050d087d5d68a93d15a66ee14b2fed6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51365
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@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 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>
Omits an object from a vector when that vector is cloned
Bug: tint:183
Change-Id: I543c885609591dcd3b930ca00b8c1a78bc61f920
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51301
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@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>
Don't construct these with undefined values
Change-Id: I6225d9be0973ebc1a8594526aa32ec6775b5e865
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51300
Commit-Queue: Ben Clayton <bclayton@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>
Adds texture_external to the inspector, allowing us to recognize the
type and return provide binding information. Includes a basic test.
Bug: Dawn:728
Change-Id: Ib0f39998359dc22a530ad222141229f9ba30552f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51161
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Brandon Jones <brandon1.jones@intel.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Once the resolver correctly handles address-of and indirections,
the logic around these will be different enough that a parameterized
test no longer makes sense.
Bug: tint:727
Change-Id: I79c138b12bd6f49d9dbee02c62e594a9b1b4a2db
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51185
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: David Neto <dneto@google.com>
Add AddressOf() and Deref()
Add overloads of Expr() that take a source
Change WrapInStatement() to create a `let`. Unlike `var`, `let` can be
used to hold pointers.
Bug: tint:727
Change-Id: Ib2cd7ab7a7056862e064943dea04387f7e466212
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51183
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Use this to check that type and expr are both not nullptr.
Cleans up the random mix of checking `expr`, `type` or both.
Change-Id: I84eb4a16e0dc22e18d6868f62c0314effe9858a2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51184
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Currently unused, but will be soon.
Also add String() methods on all the types ifndef DEBUG.
Again unused, but these are helpful for debugging type related bugs.
Bug: tint:727
Change-Id: I7598fd7f68f4819ef4b60dbfc08322fd004d9152
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51180
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
This is what it is (currently) called in the spec
Bug: tint:727
Change-Id: Ie24f42499ed20c0c45ef4e9474bc6bb6a19bfa36
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51181
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@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>
The backends now use sem::Variable::ConstantId() instead, since the
AST does not have the correct ID when the parameterless version of the
override attribute is used.
Change-Id: I816295c8b2e4baa1671c9d042920bf001b1b4336
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50845
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Prove this error is caught by SPIR-V validation.
This requires a recent version of SPIRV-Tools.
Fixed: tint:793
Change-Id: I30977de7c4d1c291ab9ea13df80189426a842521
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51040
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
The Resolver, Inspector, and backends already handle this form of the
attribute correctly, so this is straightforward.
Fixed: tint:755
Change-Id: I8c394f1e58d64827d8a53b81402e30a472ca5441
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50844
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
This will be needed by Dawn to handle overriding pipeline constants by
name in the API.
Bug: tint:755
Change-Id: I857d8ef086b1964dac8d5cfff5c78d794234e8a8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50843
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Generator::Generate() no longer takes an argument.
Change-Id: Ibee65ffd6672fb1507aec33c7690c5b05ce51fac
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50960
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
This enables the backends to emit overridable constants that had no ID
specified in the attribute.
Bug: tint:755
Change-Id: I86587205e065715257f546b546e792a5262562e8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50842
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
Nothing currently generates these.
Resolver currently doesn't know how to handle these.
Backends currently stubbed enough to build without warnings.
Bug: tint:727
Change-Id: I4b9863ae098b903b51a63c36c10bc6e390efbbb3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50746
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
There's now one remainin use of the semantic info in the WGSL writer - the transformation of [[offset]] into padded fields.
This does not belong in the WGSL writer, but instead part of the transform::Wgsl sanitizer.
Bug: tint:798
Change-Id: I95ba11f022c41150cc12de84a4085cd7d42019fe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50822
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
To declare a local variable, we write `var name : type`, not `var<function> name : type`.
This change fixes all the places where we were feeding StorageClass::kFunction into variable declarations.
Note that the resolved, semantic variable correctly infers the `kFunction` StorageClass.
Change-Id: I6221fabae1de0435044f29b9a91808421d5cace6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50821
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@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>
Removes the texture_external overload for textureSample and replaces it
with a texture_external overload of textureSampleLevel to match merged
spec. Adds a transform that adds the implicit level parameter to
textureSampleLevel. Modifies unit tests to reflect change.
Bug: dawn:728
Change-Id: I2dbc9232b4343db1075be79fda0054231860f3b1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50701
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Make all test programs valid.
Change-Id: Id7eb790519e3dec30a5b826f06585d277995b9b5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50720
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>
WGSL only has vector normalize, not scalar.
Scalar normalize is always 1.0, so return that directly.
Bug: tint:765
Change-Id: I08332c20922f5834f65284b9aaeb22995423171d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50603
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Emit AST equivalent to:
var foo: sampler;
instead of
var<handle> foo: sampler;
This is necessary to pass new checks in the resolver.
Change-Id: I5eca30ace5871086bc3c1783cde2225f1aee34ba
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50602
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
But I liked the comment where it was.
I think the linter was wrong, but it doesn't like my argument.
Change-Id: Ie3bb6e2b4bd08fae6d72a076622b1e74f517f23c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50680
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Bug found when passing program through spirv writer and then reader.
This failed in the writer.
Bug: tint:792
Change-Id: I512798240710fd262da81a73d6646deb975ac5e9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50563
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@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>
Bug found when passing program through spirv writer and then reader.
Bug: tint:792
Change-Id: I2acb17554b122c840cf3cc69149f6e004fb37bc7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50562
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Adds a transform that adds an additional 'level' parameter to textureLoad calls
when used with a texture_external. Adds a test for the textureLoad
transform. Additionally adds a test for calls to textureDimensions with
a texture_external.
Bug: dawn:728
Change-Id: I613ce8185e9a4c49529fd8e48323e586c95dde04
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50420
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
The only multisampled texture type supported by WGSL is
texture_multisampled_2d
Fixed: tint:780
Change-Id: Ie968311c802f858d087a411cda8f336627ad657b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50446
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
Bug found when passing program through spirv writer and then reader.
Bug: tint:792
Change-Id: I904b959ba1d540b4377f8b9027286182ee0c34e9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50561
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This was originally used to find and fix tests that were not
initializing constants, but these were independently fixed by jrprice@
recently (see https://dawn-review.googlesource.com/c/tint/+/50042)
recently. Still, this change is useful to avoid this happening again.
Bug: tint:792
Change-Id: I0119a1a6ade7a70a0d110ef0aac80f4eaf37a7b4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50560
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@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>
These originate from a time before Castable, and there were hand-written Is<T>() methods on each of the base types.
Castable has its own tests that ensure the Is<T>() and As<T>() work as expected.
We don't need to check that this logic works for every type that derives from castable.
Change-Id: Iaa376dc4b4b5ee413a83fc5f9113cd3ef91dfe4a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50540
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@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>
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>
Adds the external texture transform to always run in the spirv, msl, and hlsl writers.
Bug: dawn:728
Change-Id: I15a96e877e1e88f9a4a4b015cf47251b41b18e35
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50024
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Brandon Jones <brandon1.jones@intel.com>
WGSL now supports type inferencing with `let` declarations, so `var->type()` may return nullptr.
Calling a method (`Is<T>()`) on a nullptr object is UB. Instead used the free-function form which can handle nullptr objects.
Change-Id: I8faf875e1550eac1306ad731a65f3662bc4e5b43
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50307
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Brandon Jones <brandon1.jones@intel.com>
Commit-Queue: Ben Clayton <bclayton@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>
This makes it easier to triage test failures.
Bug: tint:765
Change-Id: I72d12e9eff3466d3d02952edce14499a61a4782b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49842
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@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>
As() was being called on a nullptr object.
Change-Id: Iedd7f1c57d12f1ad5ea2af60317817841f5d1392
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50303
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
If the initial array accessor in a chain uses a non-literal index, use
the path that copies the source to a function variable, and then
perform a load from the OpAccessChain result if necessary.
Fixed: tint:426
Change-Id: Ie2f3f388170c02c1d6b73355f0b3bc49c3d3a4e5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49800
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: David Neto <dneto@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>
Semantic types are no longer cloneable, entirely avoiding the issue this hack was working around.
Bug: tint:724
Change-Id: Iec876beccfda476a5539fb4d669eeba24d263500
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49962
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Having a common base class will tighten up some of the dynamic casting that we do.
Bug: tint:724
Change-Id: I45c6f200e1ec242ddb08ce75bd1c4c037a21a38d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49882
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Includes a significant refactoring of helper functions in
tint_common_fuzzer.cc/.h
BUG=tint:722
Change-Id: I1fdab0113bae02c4a0bf8da0d1b7729f05a2fc5b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49902
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
This is valid when they are overridable constants. The resolver will
validate whether or not this is the case.
Change-Id: Ic6659c58e5046117f64794efec3300b0245a5e91
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50043
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>
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>
The code to auto-generate initializers for overridable pipeline
constants was in the non-const code path, so move it to the right
place and fix the tests that were wrongly testing non-const variables.
Bug: tint:254
Change-Id: Ifc96681492768ecf844f67e114377cc643ef7609
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50040
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@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>
This CL: all SPIR-V modules in parser_impl_module_var_test.cc
Bug: tint:765
Change-Id: I2e8427aea729793a4e34e2ac0c6fc689f65bda04
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49645
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
This prevents two initializers with different types from incorrectly
using the same OpConstantComposite instruction.
Fixed: tint:777
Change-Id: I2cdef9d5b0a83c26cbd4be318fed218e6f3e93b0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49981
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>
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>
These should always be generated by the resolver, not manually constructed by transforms.
This also fixes duplicate intrinsic output from DecomposeStorageAccess.
Bug: tint:724
Change-Id: I979d55f7b141f38c0504dc72cc3c63e8353ac14f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49881
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
This is disabled by default and can be enabled via transform data. The
separate EmitVertexPointSize transform will be removed once Dawn
starts using the Spirv sanitizer to do this.
Bug: tint:753
Change-Id: I676c4cef5bc53f2dbf2330645faa4a0f2bfe11bd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49980
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
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>
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>
Instead of a typ::TypePair.
Bug: tint:724
Change-Id: Ife8fac86093eb128bc98974e1f6614a73c42a9e6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49753
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
The error message was already being logged, but the status
flag was not being set.
Change-Id: I7f6f2f45c15a64dd089e221c62218afe9e774edd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49643
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Catch the nullptrs early.
Change-Id: Ie8ee30355c79c6cbd589831f6587752b125d45c0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49754
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
This was a stub implementation copypasta'd from the semantic types.
By removing this, ProgramIDOf(ast::Node*) will be used instead, and these checks will actually work.
Bug: tint:724
Change-Id: I72f2fc5d89d81fb485b3549cb8138517c83c9482
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49752
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Helps catch brokenness before asking the developer to stare at AST dumps.
Change-Id: Id39602225ad2d4a1b72dcdf998b164feda870cf1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49751
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
The --dump-spirv option tells tint_unittests to output the
SPIR-V assembly text for a module which did not make the SPIR-V reader
fail. This lets us get extract a corpus of SPIR-V modules, and
lets us more easily verify that the test shaders are valid in the first
place.
Also:
- Add test/extract-spvasm.py to split that output to separate SPIR-V
assembly files
- Add optional second argument test/test-all.sh to specify a directory
look for input files.
- BUILD.gn: Add dependency from //test:tint_unittests_main to
//test:tint_unittests_config to pick up source dependency on
the internal header of the SPIRV-Tools optimizer, needed by
the indirection through src/reader/spirv/parser_impl_test_helper.h
This is useful for bulk testing
Fixed: tint:756
Change-Id: I4fe232ac736003f7d9be35544328302d652381ea
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49605
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
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>
If the array accessor expression uses a literal index, generate an
OpCompositeExtract instruction. Dynamic indices will be handled in a
follow-up patch.
Fixed: tint:767
Change-Id: I79a980d7d558a49def30816d04dcaa566f284c8f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49701
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
This is a step to being able to dump the SPIR-V for
successfully converted modules.
Bug: tint:756
Change-Id: Ifa48c88835ff10824c542a4e13d8a2d3a7f7f484
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49604
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Register all aliases, and don't share type nodes
Bug: tint:724
Change-Id: I7ac20c8a75f232fa7fe1e8c1a5690f411888f6e4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49750
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reconstructs the AST nodes needed to build the given semantic type.
Bug: tint:724
Change-Id: Iadf97a47b68088a6a1eb1e6871fb3a7248676417
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49745
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
typ::Types originating from ConvertType() will hold direct pointers to ast::Struct and ast::Aliase. These must not be used directly. Instead TypeNames should be created to refer to these.
Bug: tint:724
Change-Id: I7f511e0da56e5d3ac587cdb22b0baf38474bed1e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49746
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Add a new constructor that only takes a ProgramBuilder.
This allows cloning objects to and from the same ProgramBuilder.
Also clean up tests.
Change-Id: I7c7bbaced4956f9094d0a6231aa4d7f7b6f17d4c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49744
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
And add a few additional helper methods.
Stepping stone to having the module only reference AST nodes.
Bug: tint:724
Change-Id: Ib321dadce5f739afe4f71cbafde9dd2d1c6431bb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49743
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
And use MaybeCreateTypename() in more places.
Bug: tint:724
Change-Id: I232229c76205e148a2235dc5706522adb0789bcc
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49742
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
My last CL (https://dawn-review.googlesource.com/c/tint/+/49542) made it
so that AST nodes are re-created every type we resolve a spirv type id
to a tint type. This creates duplicate nodes, most of which are cleaned
up at the end of parsing. However, duplicates added as global
constructed types are not handled, so this CL makes sure we do not add
said duplicates.
Bug: tint:769
Bug: tint:724
Change-Id: Ifab46aa3b843a841c4371659dbec64891fdabc9a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49760
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
The spirv parser now creates ast types along with sem types via
typ::Type. All sem::Type* were replaced with typ::Type, and its `ast`
member is used over the `sem` member to make it easier to migrate to
ast-only.
The parser was written to take advantage of the fact that types were
resolved to semantic types during parsing. For instance, a mapping of
spirv typeid to sem::Type* was used throughout (`id_to_type_`) to
resolve types once, and to support type aliasing. Since the goal is to
only create AST types, and to resolve only in the Resolver, I made many
changes to remove this dependency on semantic types. For instance, we
now always call ConvertType(typeid) instead of looking up via
id_to_type. Similarly, the `signed_type_for_` and `unsigned_type_for_`
maps were replaced with `UnsignedTypeFor` and `SignedTypeFor` functions.
Bug: tint:724
Change-Id: I3aee3928834febd71b473d6a8d8cb77b1ac94e21
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49542
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
> clang --version
Apple clang version 11.0.3 (clang-1103.0.32.29)
../src/castable.h:199:11: error: definition of implicit copy assignment operator for 'CastableBase' is deprecated because it has a user-declared destructor [-Werror,-Wdeprecated]
Seems to only be raised with this particular flavor of clang?
Change-Id: I9a549c753cd28a11855138a97823e7c1c92fab25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49532
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>
Kokoro: Kokoro <noreply+kokoro@google.com>
The HLSL writer doesn't really care much for ast::Types, so most of the work here is repointing the logic to fetch the resolved, semantic type.
Output for aliases has changed, as the semantic type resolves away aliases.
Bug: tint:724
Change-Id: I6eb9d5c2fbcd62eef0f9dd145360714db649e13f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49530
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@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>
The MSL writer doesn't really care much for ast::Types, so most of the work here is repointing the logic to fetch the resolved, semantic type.
Bug: tint:724
Change-Id: I017b83c9f661cc01cbde377991aab184c1733348
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49529
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
The SPIR-V writer doesn't really care much for ast::Types, so most of the work here is repointing the logic to fetch the resolved, semantic type.
Bug: tint:724
Change-Id: I7647e17b015bac8394bc0fc76daceb7c0a391a47
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49528
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@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>
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>
Instead of printf-ing and returning an empty string.
When there's a lot of other test output, it can be really hard to actually spot the error message.
With the error being returned, this will likely appear in a EXPECT_EQ() style error message.
Change-Id: I733741a68de4843e986037c5e309c5a4c3d66124
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49522
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>
This takes precedence over semantic types.
WGSL should be printed as faithfully as possible to the parsed program.
The semantic nodes are more abstract than the AST - aliases are folded
away, decorations may be absorbed into the semantic types, etc.
Bug: tint:724
Change-Id: I500af3d20e4ea3f9bfcb7664f0ad2360aeeecd68
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49350
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Perform a program clone at the end of parsing to remove any unreachable AST nodes.
Actually fixing the parser to never create these looks like a huge amount of work.
Fixed: tint:749
Change-Id: Ib956634257e0933c9702d6be22f79942f7cf4c51
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49520
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
The built-in function "mix" in WGSL should be translated into the
intrinsic function "lerp" in HLSL according to the HLSL document.
With this patch the dawn sample CubeReflection will be able to run
correctly with tint generator on D3D12 backend.
Bug: tint:758
Change-Id: I7e84987c02fd2090d5e7af8f4aba995fc95a6fdb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49601
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This reverts commit e6307e5100.
Reason for revert: This change assumes that vertex shaders do not return void, but the validation for this is currently disabled since CTS still has vertex shaders that return void.
Original change's description:
> transform/EmitVertexPointSize: Handle entry point parameters
>
> Generate a new struct that contains members of the original return
> type with the point size appended to it, and replace return statements
> as necessary.
>
> The SPIR-V sanitizer then special-cases this builtin when handling
> entry point IO to always use a RHS which is a literal.
>
> Fixed: tint:732
> Change-Id: Id718632a5e671f3e7c82a304f5bc1fc223a6c8ee
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49440
> Kokoro: Kokoro <noreply+kokoro@google.com>
> Reviewed-by: Ben Clayton <bclayton@google.com>
> Commit-Queue: James Price <jrprice@google.com>
> Auto-Submit: James Price <jrprice@google.com>
TBR=bclayton@google.com,jrprice@google.com,noreply+kokoro@google.com
Change-Id: I6113b2d8994eb088cb890a6af52e392b2d5a45f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49582
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@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>
This is currently unused.
Bug: tint:508
Change-Id: I9027d468496014123a41b9941cf9df35868b6e1a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49000
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This simplifies the callsites, which were previously each having to
handle the "empty list" case (and soon: trailing commas). This is also
a better match for the grammar rules in the WGSL spec.
Change-Id: I88ed54f94964f7b23a0fd9b584659037abb567ff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49465
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>
Function calls should be parsed in `primary_expression`. Renames the
old `postfix_expression` to `singular_expression`, with the recursive
part now becoming `postfix_expression`.
Fixed: tint:170
Change-Id: I1afad794880916db38a25d73447cdaf84abd6584
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49464
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>
Generate a new struct that contains members of the original return
type with the point size appended to it, and replace return statements
as necessary.
The SPIR-V sanitizer then special-cases this builtin when handling
entry point IO to always use a RHS which is a literal.
Fixed: tint:732
Change-Id: Id718632a5e671f3e7c82a304f5bc1fc223a6c8ee
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49440
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
This creates a new, unnamed symbol.
Change-Id: I0ea11e812296c3215ba624a18777d3e7233e2ae4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49441
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>
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>
Soon, we'll start migrating the AST from using sem::Types to ast::Types.
This change fixes up a bunch of places that makes the assumption that the semantic type is always expected.
Bug: tint:724
Change-Id: I96096bdf7177751ca6c6240e1739244cbeb82761
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49348
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@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>
Caught by the new seatbelts in the CloneContext.
Bug: tint:724
Change-Id: I1877248455e7654cb9c980cacd2f73562b006233
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49349
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
A simple raw pointer wrapper that will allow us to migrate typ aliases to ast::Types without immediately having to fix up all the `auto` declarations to `auto*`.
Bug: tint:724
Change-Id: Icff7e009b768d9e54c8c73059c700af788962e77
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49346
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Once we start migrating over to just creating `ast::Type`s instead of a
AST+SEM pair, we're going to start having the sem::Type of the TypePair
be nullptr. Only create composite AST / SEM types if the inner type has
a non-nullptr AST / SEM type.
Bug: tint:724
Change-Id: I9bec53d1be4e8dc43f5d6c86fc5f968bf61fb41e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49344
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
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>
(except for ast::Module)
CloneContext::Clone(typ::Type) now only clones the sem::Type. Attempting
to clone both the AST and SEM type will cause the cloned AST to be
disjoint. Another change will switch this over to cloning the AST.
Bug: tint:724
Change-Id: I2baf5491365d7dc25e6b25d02bfbb46bf90fd0d9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49341
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Transform these into whitespace so we can sensibly align the ^^ markers
with the text.
Change-Id: I151c0e55bc0a02c1cff6e381cb9839c9f9abdaf6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48694
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@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>
We have to be careful when the same function is used for
two entry points. The first entry point name will be registered
as the name for the function ID. But subsequent entry points
should have their names reserved in the namer, even if they
aren't associated with an ID.
Bug: tint:508
Change-Id: I3b6e7770ce49aa1b73594e57bdda5800febd55ed
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48864
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
In particular, "is_const=true" is required for formal parameters.
Change-Id: I05105da901a02d9bb51c88cab7430dd211c7b0ea
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49302
Commit-Queue: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This reverts commit 19b4b6cc2b.
Reason for revert: https://dawn-review.googlesource.com/c/tint/+/48980 was reverted which added the Sym() helper.
Original change's description:
> transforms: Use new ProgramBuilder::Sym() helper
>
> Change-Id: I8600a1935556632c94778248f3711531a22f4701
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48981
> Commit-Queue: James Price <jrprice@google.com>
> Reviewed-by: Ben Clayton <bclayton@google.com>
TBR=bclayton@google.com,jrprice@google.com,noreply+kokoro@google.com
Change-Id: Ie176df6e1b7606615e2acfae99e7853e6227185a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49261
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This reverts commit 05e16ed1c5.
Reason for revert: Breaks Dawn autoroll.
[ RUN ] VertexBufferRobustnessTest.DetectInvalidValues/Metal_Intel_R_UHD_Graphics_630__e_metal_enable_vertex_pulling
../../src/tests/DawnTest.cpp:945: Failure
Value of: self->mExpectError
Actual: false
Expected: true
Got unexpected device error: Tint SPIRV reader failure:
Parser: error: cannot store a value other than constant 1.0 to PointSize builtin: OpStore %8 %18
Original change's description:
> transform/EmitVertexPointSize: Handle entry point parameters
>
> Generate a new struct that contains members of the original return
> type with the point size appended to it, and replace return statements
> as necessary.
>
> Fixed: tint:732
> Change-Id: I2b5816144d5e95c65baca95dc0c50b4dfdd25ed3
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48980
> Auto-Submit: James Price <jrprice@google.com>
> Commit-Queue: James Price <jrprice@google.com>
> Reviewed-by: Ben Clayton <bclayton@google.com>
TBR=bclayton@google.com,jrprice@google.com,noreply+kokoro@google.com
Change-Id: Id795d5320471d09d66a964c8e469b60da8f0a672
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49260
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
These have now been removed from Dawn.
Change-Id: I50e784141e6eb07f4f0d3f508bb8c6199429dd63
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48044
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Generate a new struct that contains members of the original return
type with the point size appended to it, and replace return statements
as necessary.
Fixed: tint:732
Change-Id: I2b5816144d5e95c65baca95dc0c50b4dfdd25ed3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48980
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Adds support for the new shader IO syntax by processing entry
parameters and pushing them to function-scope variables as necessary.
Module-scope variables are still supported for now.
Fixed: tint:731
Change-Id: I36d7ce4e3a990b6323292cb7c685af37187d6fda
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48960
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 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>
This alias is already defined in src/ast/decoration.h and is probably
just a copy-paste error.
Change-Id: I9efc16881b551ccac287eef1f86ca7560e87131b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48982
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Precalculate the type_name to reduce complexity from O(N²) to O(N).
Fixed: chromium:1199700
Fixed: chromium:1200936
Change-Id: Ifd16508ace42d4a686f06d58121cc106842df7d3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48691
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@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>
As in other places, we need to #undef Bool for X11
Bug: tint:741
Change-Id: I0185ed3c252f540212504b88859c600618fbfe23
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48961
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@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>
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>
A minimal change to get a feel for what updating AST types to accept
typ::Type (for now) looks like.
Bug: tint:724
Change-Id: I33b33eb6af90e3629f76716a421f952f5a4bc11d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48841
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Many more changes to come, but this just gets things started.
Bug: tint:724
Change-Id: I6c1dab0d9b7998e273543f2a72b6f8eafeb19fda
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48840
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: 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>
To avoid breaking things, functions that return the type cast away
the constness for now. This, however, makes it easier to use typ::Type
with these classes, as typ::Type stores pointers to const types. This
also brings us one step closer to constifying types everywhere.
Bug: tint:724
Change-Id: Ia3f4b76f375184dd09b8041c1f60bf1afaefe629
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48740
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Used as a stepping stone to emitting the ast::Types instead.
Bug: tint:724
Change-Id: Ib2d6c150fe8aa7c1e2c502676922b14b1518a4be
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48686
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Used as a stepping stone to emitting the ast::Types instead.
Bug: tint:724
Change-Id: Idadb7d8b5d6fce1d898127675442221de07a633d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48685
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Used as a stepping stone to emitting the ast::Types instead.
Bug: tint:724
Change-Id: Ic81f0a00456e91ac089df32193c4323c2e779c29
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48684
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Used as a stepping stone to emitting the ast::Types instead.
Bug: tint:724
Change-Id: I19d7df9ab684db598783235c1720586966a232e0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48683
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
A general code cleanup
Change-Id: Ib251ca2d4b71f75736bdba8b4255965593a76c31
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48680
Auto-Submit: 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>
Commit-Queue: Antonio Maiorano <amaiorano@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>
Sarah
Ben Clayton
James Price
David Neto
Antonio Maiorano
Ryan Harrison
Stephan Hartmann
Brandon Jones
Alastair F. Donaldson
Zhaoming Jiang