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>
Emit the new typ::I32, typ::U32, typ::F32, typ::Void, typ::Bool types instead of the just the sem::Types.
Used as a stepping stone to emitting the ast::Types instead.
Bug: tint:724
Change-Id: Ic492e33bc909e0821b581af05242d956631db2e3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48602
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Introduces typ::Type which wraps a templated pair of AST and SEM type pointers.
This is a temporary helper to ease migration of the thousands of tests that use the ProgramBuilder over from sem::Type to the new ast::Types.
Bug: tint:724
Change-Id: I4e2643a819cde97947d789fce7a74c251f837a58
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48601
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
- Move headers from libtint_sem_src into libtint_core_all_src so that
there is no circular header dependency between headers of these two
targets. libtint_sem_src is now just a hack to have a different name for
a few .cc files.
- Made libtint_core_src publicly depend on its deps so that headers of
libtint_core_all_src are made visible through it.
- Added spvtools dependencies in a couple places that were mistakenly
removed in a previous commit.
- Moved helpers common to multiple unittest targets out of
tint_unittests_core_src and into a tint_test_helpers target that
all tint_unittests_source_set depend on.
- Ran GN format that reordered some lists a bit.
Bug: None
Change-Id: I544a73d73366be9dd2ac8e56c7593fd9f2b86cf8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48600
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Adds rejecting unsupported image formats and texture dimensions.
BUG=tint:718
Change-Id: Ic6375d1e75524336462ea59b159cb4c8601f55fd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48500
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
More of a workaround. Will be properly fixed by tint:724 .
Fixed: tint:728
Change-Id: If4887a781106010d12209365f36f27b493a9f173
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48422
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Fixes:tint:717
Change-Id: I7f23086ec516c1196a9cdc741bbaa150754a533a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47940
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
These do not exist in the WGSL spec, so removing them.
BUG=tint:717
Change-Id: I5eb2280fa2f0c923b67070a9ce4a7d2fc10d365d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48460
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
The name now lives on the ast::Struct. Use that instead.
Bug: tint:724
Change-Id: I4ee5e9b29973e468edd8df8c5448816b36f0fca6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48384
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Copy all of the type classes from src/type into ast.
Required the merging of:
* type::Struct into the existing ast::Struct - ast::Struct now has a name.
* type::AccessControl into the existing ast::AccessControl enumerator - The old ast::AccessControl enumerator is now ast::AccessControl::Access
Bug: tint:724
Change-Id: Ibb950036ed551ec769c6d3d2c8fb411809cf6931
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48383
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This is to avoid name conflicts once we move all classes from namespace
`type` to `sem`.
Bug: tint:724
Change-Id: I23cdec636cb5bcf0bbba03ee7bb7c44252ddade7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48361
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This is to avoid name conflicts once we move all classes from namespace
`type` to `sem`.
Bug: tint:724
Change-Id: Icc3d81ae62eb3b329ce28e78a23ea27f29c9263b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48360
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Adds ExternalTexture type and basic unit tests in accordance with the
currently proposed WGSL specification.
Bug: dawn:728
Change-Id: I7a16a351ff098ba6df5b1e6305c390e3ca1c9d46
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47180
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This was only called for function-scope variable declarations.
In calling this, there were inevitable tests failing, which have now been fixed.
Added a test for the single runtime-array-length validation rule that this function was checking.
Fixed: tint:345
Change-Id: Ic453c38158c1290a5e1ef6de56af0c621d97982a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48381
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Fixed: tint:94
Change-Id: I1d3e512c030ec16031b8c8fcfbde0cd1db5d1ea4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48380
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Handle the case where the OpBranchConditional in a loop header
branches to two distinct blocks inside the loop construct.
This is an if-selection in disguise.
Create an kIfSelection with the same set of blocks as the kLoop,
and with the continue target as the merge.
Fixed: tint:524
Change-Id: I5150d19a2b4388da409e2da6e68ffafdc5d21a9a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47560
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
Fixes "error : use of identifier 'Node' found via unqualified lookup
into dependent bases of class templates is a Microsoft extension
[-Werror,-Wmicrosoft-template]"
Change-Id: Id54cdff24189b73c625f951ae369ec292be4c81b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48340
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
AST nodes must not be shared. Diamonds in the AST will cause all sorts
of exciting, non trivial bugs.
All AST nodes must be reached by the Resolver. There are two common
reasons why they may not be:
(a) They were constructed and not attached to the AST. Several
transforms scan the full list of constructed AST nodes to find nodes
of a given type. Having detached nodes will likely cause bugs in
these transforms. Detached nodes is also just a waste of memory.
(b) They are attached to the AST, but the resolver did not traverse
them. Having the resolver skip over parts of the AST will fail to
catch validation issues, and will leave semantic gaps, likely
breaking downstream logic.
Bug: tint:469
Change-Id: I143b84fd830699f874d2936146f0e93197db610c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47778
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
By making nodes reachable, the resolver has now caught a whole lot of additional problems, which have been fixed in this CL.
Some of these broken tests were attempting to use private and workgroup variables as function-scope declarations.
This is not legal, and these have been moved to module-scope variables.
Bug: tint:469
Change-Id: I1fc3a10fa0e39e1c290a13323277d6e9257778c4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48048
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Check that pre-clone objects are owned by the source program.
Check that post-clone object are owned by the target builder.
Fixed: tint:469
Change-Id: Idd0eeb8dfb386e295b66b4b6621cc13dc1a30786
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48260
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
By making nodes reachable, the resolver has now caught a whole lot of additional problems, which have been fixed in this CL.
Some of these broken tests were attempting to use private and workgroup variables as function-scope declarations.
This is not legal, and these have been moved to module-scope variables.
Bug: tint:469
Change-Id: I8c91ace57701e9bec1f706eed0d9de7507c4b65a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48223
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
This makes the .cc files match the header file name, which allows for
the text editor "header flip" feature to actually work. The reason these
files were named this way was because GN doesn't allow name conflicts in
the same source set, despite the files being in different directories.
This change splits the files into different source sets. To do so, we
use GN templates, which also reduces duplication in each target
definition.
Bug: tint:724
Change-Id: I9a7ed3912e4b85b2b38d360805203f3488b86c4c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48160
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Implement emission of module scope variables of the private and workgroup storage classes.
Fix tests that were incorrectly emitting these as function-scope variables.
Change-Id: I509a7388794f4a57b06a3859d10afa2611d84991
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48222
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
By making nodes reachable, the resolver has now caught a whole lot of additional problems, which have been fixed in this CL.
Some of these broken tests were attempting to use private and workgroup variables as function-scope declarations.
This is not legal, and these have been moved to module-scope variables.
Bug: tint:469
Change-Id: If5e812da3f62f096e344c50782ab0f465ae74ea3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48224
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
By making nodes reachable, the resolver has now caught a whole lot of additional problems, which have been fixed in this CL.
Some of these broken tests were attempting to use private and workgroup variables as function-scope declarations.
This is not legal, and these have been moved to module-scope variables.
Bug: tint:469
Change-Id: I09c1a8e72a33d3e4df13554e8b07d0a169fcf575
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48226
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Module scope variables of the private and workgroup storage class are
not currently implemeted.
Change-Id: Ibb73faec5b5bcb11199e07ba4d7aa4df9c875450
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48221
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Detached AST nodes will become an ICE, so only create nodes if they are going to be used.
Bug: tint:469
Change-Id: I29b3275a355e30c934e6e508185a19981dcc8a42
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48050
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
EmitControlBarrier() was calling MakeOperand() to create ast::Expressions that held a ScalarConstructorExpression, which held a IntLiteral.
The literal value was then taken, and the rest was discarded.
Detached AST nodes will become an ICE, so do the work to find the literal value.
Bug: tint:469
Change-Id: I522bfe8db84e853e189c714b18598feb0d49e58b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48049
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Instead of directly fowarding to ReturnStatement, pass the parameter through `Expr()`.
Also add a no-arg overload.
This means we can write `Return(1);`, `Return("a")`, `Return()`
Change-Id: I8f5df630f540f9cecdf82d24e2810a0844e025e8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48046
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
This is not valid. Will become an ICE.
Bug: tint:469
Change-Id: I02c0eea16daf7d83f4d6c251e06d9cac0dfd7ce4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47777
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
The CloneContext will currently de-duplicate any pointers that are cloned multiple times, however this isn't ideal behavior for AST nodes.
AST nodes must be unique in the tree, so a non-transform clone of the AST from one program to another should only ever Clone() called once per node. Hitting the map for these is inefficent, worse still, there are cases in the transforms where we actually *want* to create N copies of the same source node. The current implementation makes this impossible.
This change introduces ShareableCloneable, a new base class that derives from Cloneable.
Repeated calls to CloneContext::Clone() for Cloneable pointer types will now produce a new, unique instance per call.
Repeated calls to CloneContext::Clone() for ShareableCloneable pointer types will de-duplicate as before.
type::Type objects are shared, want deduplication while cloning, so now derive from ShareableCloneable.
ast::Node continues to derive from Cloneable.
Bug tint:469
Change-Id: I5ca906d507de6271d5d715cfdc962a55b721e821
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47776
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
This is a trivial mapping to/from WGSL in all cases.
Bug: tint:478
Change-Id: I7f21a2392543a880906b54fddbdb8bbd149a526e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48140
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
[Is,As] contain a static_assert() that checks a cast is actually possible (TO -> FROM share a common base class).
This has been extremely valuable - it's caught numerious impossible casts due to stupid mistakes - however it makes certain generic templates impossible to write.
Add a compile-time FLAGS argument to Is() and As(), which accepts a new kDontErrorOnImpossibleCast flag.
When specified, this static_assert will always pass, allowing impossible casts to be attempted.
Change-Id: I5ff434b329c04d007f4e6976301bf30c73ab3f8d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47775
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Error: explicitly-defaulted constructor cannot have default arguments
This broke chromium, and needs fixing before we can restart the autorollers.
Change-Id: I4b04dd127b5fb7cf84e5782e07bcf68b2befc904
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48041
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Fixed: tint:673
Change-Id: I5a58d9e504446ccff724e368b5ea2cf835d2271b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47768
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
This test landed simultaneously with a change that checks that storage buffers have an access qualifier, leading to broken tests.
Also fix lint issues that have crept in.
Change-Id: I4a7f0bc67d8d3e7170ddea117342a510741a495c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48040
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Almost all transforms should clone all symbols before doing any work,
to avoid any newly created symbols clashing with existing symbols the
source program and causing them to be renamed.
The Renamer is the exception to this, and so an optional flag is used
to prevent automatic cloning of symbols for this transform.
Bug: dawn:758
Change-Id: I84527a352825b2eaa43eabe225beb9e0999bf048
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48000
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Produces ugly code for the common case.
Change-Id: I8e5b1215e19fc6461dc40b8a91922db25f9cbd76
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47764
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
Variables in the storage storage class and variables with a storage
texture type must have an access attribute applied to the store type.
https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables
A variable in the storage storage class is a storage buffer variable. Its
store type must be a host-shareable structure type with block attribute,
satisfying the storage class constraints.
Fixup tests, including those that were producing warnings about `var <in>`
The WGSL writer seems to want to put a newline after every decoration block, leading to some ugly output. I'll fix this as a separate change.
Fixes: tint:531
Fixes: tint:692
Change-Id: If09d987477247ab4a7c635f6ee6e616a06061515
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47763
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
The canonical type was stopping at the first encountered type::AccessControl, which meant the alias in access<alias<i32>> was not being unwrapped.
Bug: tint:705
Change-Id: Idcbd824808d8ee3098fb1861add5014d7d46b0ad
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47762
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Move transform::Transform::Output to transform::Output.
There's no need for this to be an nested class, it stutters, and it also
prevents Dawn from forward declaring it.
Add move assignment operator to DataMap.
Change-Id: Ibe1af03abc1a872790d20ee6ec8cf18a511ea0b4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47772
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Adds additional modes to the Renamer for renaming just HLSL or MSL
keywords. The sanitizers no longer rename reserved keywords
automatically, which isn't usually necessary anyway since Dawn renames
all symbols.
The tint executable automatically renames reserved keywords when
targeting HLSL and MSL. It now also has an option to rename
everything, which is useful for testing the renamer.
Change-Id: Idbfd53226805e851050024402be8d8f251b88707
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47960
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
type::Array was the only type with special handling in that we'd resolve
all array types first in ResolverInternal, then proceed with resolving
the AST in order of declaration. This CL removes this special handling,
and instead, we now process Arrays as we process ast::Variables of
array type. This change also allows us to pass down a Source location
for validation messages when processing Arrays.
Updated some Builder tests that weren't creating a variable of the array
type they declared.
Bug: tint:707
Change-Id: I8483b3a979bc1e5e04feb1ca4d281e96e9e654be
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47825
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Assert in each AST constructor that symbols belong to the program of the parent.
Bug: tint:709
Change-Id: I82ae9b23c88e89714a44e057a0272f0293385aaf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47624
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Assert in each AST constructor that child nodes belong to the program of the parent.
Bug: tint:709
Change-Id: Icc89b69691d099e358ff632a0ca6fd7943cb0193
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47623
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Don't end the EXPECT_THAT() with `<< ToString(p->builder(), fe.ast_body());`
This string is already emitted in the error, and calling ast_body() a second time triggers an assert as the builder is already finalized.
Change-Id: Ied6aca73937e86aa6ce2d8cff8a09f67b5082349
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47767
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
HLSL interface matching rules impose additional requirements on the
order of structure members. We now sort members such that all members
with location attributes appear first (ordered by location slot),
followed by those with builtin attributes.
Fixed: tint:710
Change-Id: I90940bcb7a5b9eeb1f50f132d406d4cf74e47ea2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47822
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Fix a test that did not have the position builtin as a vertex shader
output.
Change-Id: I8dceba59d8327938e725e7d5e62b4b9c43695710
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47820
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Pre-clone the program's symbols at the start of the transform,
otherwise the original program's symbols may get mangled. This causes
problems for Dawn when the entry point name is changed.
Change-Id: I414c798fb5f51afe44e8b97619f77452f97f0782
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47824
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
David Neto
Antonio Maiorano
Ben Clayton
James Price
Brandon Jones
Alastair F. Donaldson
Ryan Harrison
Zhaoming Jiang
Corentin Wallez