Removes the need to pass the storage class to the SPIR-V reader
builtin mapping function.
Added a deprecation warning for sample_mask_{in,out}.
Bug: tint:715
Change-Id: I948ff2de2d5de7bd05e1c6ff45bd721c856900e3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47743
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Use the variable storage class to determine the correct builtin to use
in the SPIR-V generator.
Added a deprecation warning for frag_coord.
Bug: tint:714
Change-Id: I5ad4956f9345e2f39f4af16e84668dec345ac82e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47742
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>
SymbolTable::New() used to build and return a symbol without a registered name. When you asked for the name of the symbol it would return tint_symbol_N, where N is the numerical identifier for the symbol.
This approach was a major tripping hazzard for transforms that liked to fetch the source program name, and register it in the new program (in this situation, you should always use `CloneContext::Clone(Symbol)`).
Without special casing for unnamed symbols, you could end up promoting the unnamed symbol to a named symbol, and then colliding against a new unnamed symbol.
This is exactly what happened in tint:711.
Instead, with this change:
* The concept of unnamed symbols has been removed. All symbols now have a name.
* The signature of `SymbolTable::New()` has been changed to take a name parameter (which defaults to 'tint_symbol'). This can be used to create a new, unique named symbol (possibly with a suffix), which will not collide with any existing symbols. Note these symbols may still collide if `SymbolTable::Register()` is called with the same name. All Transforms that currently use `SymbolTable::Register()` will be fixed in another change.
* The CloneContext has been updated to use `SymbolTable::New()` instead of `Register()`. This means that any symbols defined before a clone will not collide.
* `CloneContext::CloneSymbols()` has been added which allows a transform to pre-clone all the symbols from the source program. This can be used to avoid the authored identifiers being suffixed with a number, in the case a transform calls New() before the symbol is cloned.
* `Symbol::to_str()` has been changed to return `$<id>` instead of `tint_symbol_N`. This is to avoid any confusion between the actual name and the symbol ID.
Bug: tint:711
Bug: tint:712
Change-Id: I526e4b49b7027545613859de487e6a275686107a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47631
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Registering a new Symbol with the NameFor() of the source symbol creates
a new *named* symbol. When mixing these with unnamed symbols we can have
collisions.
Update CloneContext::Clone(Symbol) to properly clone unnamed symbols.
Update (most) the transforms to ctx.Clone() the symbols instead of
registering the names directly.
Fix up the tests where the symbol IDs have changed.
Note: We can still have symbol collisions if a program is authored with
identifiers like 'tint_symbol_3'. This will be fixed up in a later
change.
Change-Id: I0ce559644da3d60e1060f2eef185fa55ae284521
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46866
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Checks the following things:
- Non-struct entry point parameters must have pipeline IO attributes
- Non-struct entry point return type must have a pipeline IO attribute
- Structs used as entry point parameters and return values much have
pipeline IO attributes on every member
- Structs used as entry point parameters and return values cannot have
runtime array or nested struct members
- Multiple pipeline IO attributes on a parameter, return type, or
struct member is not allowed
- Any given builtin and location attribute can only appear once for
the return type, and across all parameters
Removed tests for nested structs from the SPIR-V transform/backend.
Fixed a couple of other tests with missing pipeline IO attributes.
Fixed: tint:512
Change-Id: I4c48fe24099333c8c7fcd45934c09baa6830883c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46701
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
The SPIR-V and HLSL sanitizing transforms add an empty one if
necessary.
Fixed: tint:679
Change-Id: Ic98ff3109d7381b1fbc2de68d95d57e15c7a67c0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46700
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
The backend was wrongly generating OpLoad instructions for function parameter
accesses since it thought they were pointers.
Run SPIR-V validation for the entry point IO tests.
Bug: tint:509
Change-Id: Ic816a22973fb87c7ede26bad8fb595764a333250
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45941
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
Recursively hoist struct members out to module-scope variables, and
redeclare the structs without entry point IO decorations. Generate a
function for storing entry point outputs to the corresponding
module-scope variables and replace return statements with calls to
this function.
Fixed: tint:509
Change-Id: I8977f384b3c7425f844e9346dbbde33b750ea920
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45821
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Generate a global variable for the return value and replace return
statements with assignments to this variable.
Add a list of return statements to semantic::Function.
Bug: tint:509
Change-Id: I6bc08fcac7858b48f0eff62199d5011665284220
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44804
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Use it for entry point IO sanitizing transforms to fix cases where structures were being inserted before type aliases that they reference.
Also fixes up some ordering issues with the FirstIndexOffset
transform.
Change-Id: I50d472ccb844b388f69914dcecbc0fcda1a579ed
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45000
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Add a sanitizing transform to hoist entry point parameters out as
global variables.
Bug: tint:509
Change-Id: Ic18f69386a58d82ee11571fa9ec0c54cb5bdf2cf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44083
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
All includes from .cc to .h are preserved, even when transitively included.
It's clear that there are far too many includes in header files, and we should be more aggressive with forward declarations. tint:532 will continue to track this work.
There are, however, plenty of includes that have accumulated over time which are no longer required directly or transitively, so this change starts with a clean slate of *required* includes.
Bug: tint:532
Change-Id: Ie1718dad565f8309fa180ef91bcf3920e76dba18
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44042
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Transforms are meant to be reusable. Those that hold state cannot be used concurrently.
State leakage between runs is dangerous.
To fix this:
* Add transform::Data - A new base class for extra information emitted by transforms.
* Add transform::DataMap - A container of Data, keyed by type.
* Add a transform::DataMap field to Transform::Output.
* Have FirstIndexOffset emit a FirstIndexOffset::Data.
* Deprecate the getters on the transform.
Mutability of the transform config is also dangerous as setters can be called while a transform is actively running on another thread.
To fix:
* Expose a VertexPulling::Config structure and add a constructor that accepts this.
* Deprecate the setters on VertexPulling.
Also deprecate Transform::Output::diagnostics.
Put all the transform diagnostics into the returned Program. Reduces error handling of the client.
Change-Id: Ibd228dc2fbf004ede4720e2d6019c024bc5934d1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42264
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Use a sanitizing transform to convert scalar `sample_mask_{in,out}`
variables to single element arrays.
Add the `SampleRateShading` capability if the `sample_index` builtin
is used.
Bug: tint:372
Change-Id: Id7280e3ddb21e0a098d83587d123c97e3c34fa1b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41662
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
James Price
Ben Clayton