Commit Graph

32 Commits

Author SHA1 Message Date
James Price 830b97ffa9 writer/msl: Handle texture and sampler variables
Move these module-scope variables to entry point parameters and pass
them as arguments to functions that use them. Disable entry point IO
validation for them.

Emit [[texture()]] and [[sampler()]] attributes on these entry point
parameters.

Fixed: tint:145
Change-Id: I936a80801875a5d0b6cd98a2e8f3e297a2f53509
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53961
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-06-11 12:34:26 +00:00
Ben Clayton 9b54a2e53c Implement Pointers and References
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>
2021-05-18 10:28:48 +00:00
Antonio Maiorano dc4e6c1844 Remove sem::AccessControl
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>
2021-05-14 17:51:13 +00:00
Ben Clayton a34fa0ecb7 Remove sem::Alias
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>
2021-05-10 17:38:01 +00:00
Ben Clayton 4cd5eea87e sem: Fold together sem::Array and sem::ArrayType
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>
2021-05-07 20:58:34 +00:00
Ben Clayton ba6ab5e6bd sem: Fold together sem::Struct and sem::StructType
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>
2021-05-07 14:49:34 +00:00
Ben Clayton 02ebf0dcae ast: Migrate to using ast::Type
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>
2021-05-05 09:09:41 +00:00
Ben Clayton 02ada29af2 Add Transform::CreateASTTypeFor()
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>
2021-05-04 18:13:21 +00:00
Antonio Maiorano 73fdc16c33 wgsl parser: replace almost all sem::Type* with typ::Type where possible
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>
2021-04-26 14:19:55 +00:00
Ben Clayton 856a3688f7 transform: Minor changes to cleanup Dawn usage.
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>
2021-04-16 08:35:24 +00:00
James Price fd7251bb02 Merge RenameReservedKeywords into the Renamer
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>
2021-04-16 08:28:54 +00:00
Ben Clayton 4d349b5391 transform: s/_tint_/tint_ for reserved keywords
Bug: tint:640
Change-Id: I7525ac97d8942fffa651847d703bf37a10cbc71d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47636
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
2021-04-13 23:44:57 +00:00
Ben Clayton 363a2e26e3 SymbolTable: Change behavior of anonymous symbols
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>
2021-04-13 20:07:57 +00:00
Ben Clayton 12ac6e5d80 transform::VertexPulling - use DataMap for inputs
Migrate this transform to using the transform::DataMap pattern for configuration.

Allows users to fully construct their transforms ahead of time, and pass in the configuration options for each run.

Bug: tint:389
Change-Id: Ie4a8bf80d7b09cfe7bdd4ef01287d994b6b9eb4f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47626
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
2021-04-13 18:46:47 +00:00
Ben Clayton 095cd1c255 Fix symbol collision in RenameReservedKeywords()
Fixed: tint:711
Bug: tint:712
Change-Id: I8d0b5c926772c4265d273f6420f101f48ac29b9c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47628
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
2021-04-13 17:53:09 +00:00
James Price 5c9906e307 [spirv-writer] Handle entry point IO struct types
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>
2021-03-24 17:08:06 +00:00
James Price feecbe0d83 [wgsl-reader] Allow decorations on function return types
Add a return type decoration list field to ast::Function.

Bug: tint:513
Change-Id: I41c1087f21a87731eb48ec7642997da5ae7f2baa
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44601
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-03-15 17:01:34 +00:00
Ben Clayton 5b36d2c612 Remove all unnecessary includes
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>
2021-03-09 11:11:17 +00:00
Ben Clayton 37d2b35d59 transform::Msl: Rename reserved keywords
This change begins the work to move the reserved keyword remapping out of the writer and into the sanitizer transform.

If the transform::Renamer is in use, then these symbols should never have to be remapped - however for debugging purposes it is often nice to be able to emit code that isn't entirely mangled.

The logic in the msl writer will be removed as a followup change

Bug: tint:273
Change-Id: I76af03ff80388a48d9dd80a5b5fdfe21f3c8e7a0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43982
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
2021-03-04 23:03:25 +00:00
Ben Clayton 7732ca51c4 Rename TINT_INSTANTIATE_CLASS_ID()
to TINT_INSTANTIATE_TYPEINFO()

ClassID isn't a thing any more.

Change-Id: Ie1c0d4a95e58ef7166d3cab5ef733a2dfc702345
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42921
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
2021-03-02 20:51:18 +00:00
Ben Clayton eb496d0a4d transform: API cleanup
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>
2021-02-24 15:55:24 +00:00
Ben Clayton 545c9742d5 Cloning: move arguments to create() into temporary locals
In C++ argument evaluation order is undefined. MSVC and Clang evaluate these in different orders, leading to hilarity when writing tests that expect a deterministic ordering.

Pull out all the argument expressions to create() in the clone functions so a cloned program is deterministic in its ordering between compilers.

Change-Id: I8e2de31398960c480ce7ee1dfaac4f67652d2dbc
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41544
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Auto-Submit: Ben Clayton <bclayton@google.com>
2021-02-11 20:27:14 +00:00
Ben Clayton a6b9a8eb2f Split Program into Program and ProgramBuilder
Program is now immutable*, and remains part of the public Tint
interface.

ProgramBuilder is the mutable builder for Programs, and is not part of
the public Tint interface. ast::Builder has been folded into
ProgramBuilder.

Immutable Programs can be cloned into a mutable ProgramBuilder with
Program::CloneAsBuilder().

Mutable ProgramBuilders can be moved into immutable Programs.

* - mostly immutable. It still has a move constructor and move
  assignment operator - required for practical usage - and the
  semantic information on AST nodes is still mutable.

Bug: tint:390
Change-Id: Ia856c50b1880c2f95c91467a9eef5024cbc380c6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38240
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-26 16:57:10 +00:00
Ben Clayton c40f627bea Migrate from using ast::Module to Program
Enforce all places where Dawn passes in or returns a ast::Module, now takes a `const Program* ` or returns a `Program`.

As the end goal of all this is to have immutable Programs, all Program inputs take a pointer instead of moving the actual object.

As consumers of a Program are now all const, we have to const_cast to work around all the places we've been incorrectly mutating a ast::Module.
These const_casts are temporary, and will be fixed in the next set of changes.

Depends on https://dawn-review.googlesource.com/c/dawn/+/38522

Bug: tint:390
Change-Id: Ie05b112b16134937d1b601e9b713ea4ec4e1c677
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38541
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-26 16:57:10 +00:00
Ben Clayton d59cedb5a5 Add tint::Program as a wrapper of tint::ast::Module.
`tint::Program` will become the new public API object for a parsed shader program.
For now, have Program be a simple wrapper around ast::Module so we can migrate Dawn's use of the public tint API.

Add new Program variants of public APIs for places that returned or took a Module.

Remove Reset() methods from Generators, they aren't used, and make the migration harder.

Change-Id: Ic5bee46ceb109ea591ba7fec33685220b244a1ae
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38540
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
2021-01-25 18:14:08 +00:00
Ben Clayton 5c243f824c Rename CloneContext::mod to CloneContext::dst
In the future, CloneContext will be operating on `Program`s so a field called `mod` is poorly named.
CloneContext has a `src` member, so rename to `dst` to keep symmetry.

Bug: tint:390
Change-Id: Ic724f8a18b46ef719790394cdc810f7eb3681234
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38364
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
2021-01-22 13:41:06 +00:00
Ben Clayton 1e29f4beb0 Move CloneContext and Traits from src/ast to src/
CloneContext clones the AST, types, symbols and in the future semantic info.
3/4 of these are non-ast, so promote these up to the root.

Bug: tint:390
Change-Id: I49619796e6f81f9ab64f79413a12c87312cb1901
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38361
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-21 16:20:40 +00:00
dan sinclair c8c31560de Remove name fields
This CL removes the name fields from the various AST nodes now that the
symbols are used everywhere.

Change-Id: I73e8fa8958aa6e6f0159b12b63176b12c418f525
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/36762
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
2021-01-11 16:24:32 +00:00
dan sinclair 4ac6568821 Remove Function::name().
This CL removes the function name accessor and changes all usages to use
the symbol.

Change-Id: I19b92bf1bc557ba14e68ef8cb381487a4ad1f7ee
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/36821
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-01-11 16:24:32 +00:00
Ben Clayton 0b930237ec ast: Clone symbols instead of ever-growing
The interface for cloning a module was made significantly more complex in https://dawn-review.googlesource.com/c/tint/+/35502/ as some of the transforms required constructing symbols before the clone. This was temporary solution in 35502 was to copy the symbol table, then construct the new types, then perform the clone. This lead to a really messy callback interface, that was extremely error prone (e.g. lamda-capturing stack variables from the initializer callback that had been unwound).

Instead, clone the symbols as they're encountered. This may produce an entirely different set of identifiers, but no longer ever-grows the symbol list, and keeps the interface clean.

Bug: tint:396
Bug: tint:390
Change-Id: I54affd68ac3b730b649af9b47eba685c8a1d784a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35663
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-12-15 12:32:18 +00:00
Ben Clayton d408f2465a Remove BlockStatement::insert()
Bug: tint:396
Bug: tint:390
Change-Id: I719b84804164fa801ded505ed56717948f06c7a7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35502
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-12-14 20:31:17 +00:00
Ben Clayton 00b77a80ab Rework all transforms to transform-on-copy
instead of transform-in-place.

This is a public API breaking change, so I've added the `DAWN_USE_NEW_TINT_TRANSFORM_API` define which is used by Dawn to know which API to use.

As we're going to have to go through the effort of an API breaking change, use this as an opportunity to rename Transformer to Transform, and remove 'Transform' from each of the transforms themselves (they're already in the transform namespace).

Bug: tint:390
Bug: tint:389
Change-Id: I1017507524b76bb4ffd26b95e550ef53ddc891c9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34800
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
2020-12-04 09:06:09 +00:00