36 Commits

Author SHA1 Message Date
Ben Clayton
13ef87caab Validate that Symbols are all part of the same program
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>
2021-04-15 18:20:03 +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
dac5418819 type::Type: fix weird formatting
Change-Id: I12c6836316f30b0924c2bb5a62ee6c168749fa30
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46871
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
2021-04-07 12:48:01 +00:00
Ben Clayton
90f43cf87f Replace all uses of assert() with TINT_ICE macros
Use TINT_ICE() where we have diagnostics, TINT_ASSERT() where we do not.

Change-Id: Ic6e842a7afdd957654c3461e5d03ecec7332e6f9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46444
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
2021-03-31 20:43:26 +00:00
Antonio Maiorano
be0fc4e929 Validate binary operations
This change validates that the operand types and result type of every
binary operation is valid.

* Added two unit tests which test all valid and invalid param combos. I
also removed the old tests, many of which failed once I added this
validation, and the rest are obviated by the new tests.

* Fixed VertexPulling transform, as well as many tests, that were using
invalid operand types for binary operations.

Fixed: tint:354
Change-Id: Ia3f48384256993da61b341f17ba5583741011819
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44341
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
2021-03-16 13:26:03 +00:00
Antonio Maiorano
5106a0653c Add CastableBase::To() overload with predicate
This allows for a more optimal way to filter the result of To(). Updated
Type query functions to make use of it. Added tests.

Change-Id: If3a65259345fbe6b92c6d367ab01fa718bb7cfee
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44463
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
2021-03-15 19:55:02 +00:00
Antonio Maiorano
a089a56c73 Add Castsable::IsAnyOf<T1, T2, ...>()
This makes it a little easier to check if an object is one of any of the
types provided. Updated Type query functions to make use of IsAnyOf.
Added tests.

Change-Id: I12ea62b32042b6675d998ab85b86f2fe15861330
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44462
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-03-15 17:29:03 +00:00
Ben Clayton
d614dd5d12 Implement Default Struct Layout
Implements https://github.com/gpuweb/gpuweb/pull/1447

SPIR-V Reader is still TODO, but continues to function as the offset
decoration is still supported.

Bug: tint:626
Bug: tint:629
Change-Id: Id574eb3a5c6729559382812de37b23f0c68fd406
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43640
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
2021-03-15 10:43:11 +00:00
James Price
95d4077648 Flatten ast::Decoration class hierarchy
Remove the decoration groupings (Array, Function, Struct,
StructMember, Type, Variable), such that all *Decoration classes now
subclass ast::Decoration directly. This allows for decorations to be
used in multiple places; for example, builtin decorations are now
valid for both variables and struct members.

Checking that decoration lists only contain decorations that are valid
for the node that they are attached to is now done inside the
validator.

Change-Id: Ie8c0e53e5730a7dedea50a1dec8f26f9e7b00e8d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44320
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
2021-03-11 17:39:32 +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
5f0ea11365 Rename TypeDeterminer to Resolver
Move out of the src root and into its own subdirectory
Rename methods to remove the 'Determine' prefix.

Fixed: tint:529
Change-Id: Idf89d647780f8a2e7495c1c9e6c402e00ad45b7c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44041
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
2021-03-09 10:54:37 +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
Ryan Harrison
13dbbc6797 Remove references 1D Array texture dimension
WGSL recently removed this concept, since it didn't exist in WebGPU,
so excising it from the code.

BUG=tint:515

Change-Id: Ibbca6bd643fd96c2fb10bd33f471c9e9e58de535
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42320
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
2021-02-24 05:05:21 +00:00
Ben Clayton
52296de528 Add tint::Cloneable base class
The CloneContext was previously dealing with pointers to CastableBase, which has no guarantees that the object was actually cloneable.
Add a Cloneable base class that CloneContext can use instead.

Improves readability and produces cleaner compiler errors if you try to clone a non-cloneable object.

Change-Id: I4352fc5dab3da434e4ab160a54c4c82d50e427b4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41722
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-17 13:17:39 +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
81da283b2c Add type::Type::UnwrapAliasIfNeeded()
Unwraps aliased types until reaching a non-alias.

Change-Id: I6546d60b7cbe07d4c8cc5a0b439329af8b468ca9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41500
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-10 21:40:15 +00:00
Ben Clayton
3e8060b4e3 Add type::Type::FriendlyName()
Gives a WGSL-like string for the given type.

Also cleans up some code in IntrinsicTable.

Change-Id: I89a2fadb5291b49dcbf43371bb970eef74670e2c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40605
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-09 18:52:34 +00:00
Ben Clayton
b57c19d450 Change StorageTexture::SubTypeFor() signature
So that it takes a type::Manager instead of a ProgramBuilder.
Makes this callable from places that has the former and not the latter.

Change-Id: Ie968617ae944cc6621c17467a4f7caadacba548f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40505
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-08 20:35:01 +00:00
James Price
1f2d38c826 [wgsl-writer] Omit the storage class for handle types
These types have an implicit storage class of UniformConstant.

Bug: tint:332
Change-Id: I78c5b2a085e543ebba7d5e92b8f11550d0cd4d49
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40400
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
2021-02-04 18:35:40 +00:00
Ben Clayton
48b384168c type::StorageTexture: Remove set_type()
The subtype of a storage texture is statically determinisic from the ImageFormat, and does not need to be late-set by the TypeDeterminer.

Add StorageTexture::SubtypeFor() helper for returning the subtype for a given ImageFormat, and add the subtype as another immutable constructor parameter.

Bug: tint:390
Change-Id: Ibe732293e3142064b60f4e666a7eb39ae8db50e7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40064
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
2021-02-03 20:28:26 +00:00
Alan Baker
2e6a1bb396 Remove special case type determination
* Affects any, all, derivatives, dot and select
* Add validation for those builtins

Change-Id: I029b5acf92ddb2239c4f50d2e179bdb63d09aafd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39801
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Alan Baker <alanbaker@google.com>
2021-02-01 16:03:03 +00:00
Alan Baker
e809fb3ae5 Refactor unnecessary builtin checks
* Move unnecessary builtin checks out of type determination and into
  validation
  * Type determination now uses a bare minimum of information for most
    builtins
  * Validation now does majority of checking of builtins
* Added const qualifier to type accessors

Change-Id: Id11b739770af904a9b7afe0b1c2de50e1428a165
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39540
Commit-Queue: Alan Baker <alanbaker@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-01 15:33:13 +00:00
Ben Clayton
3d20d49211 src/types/bool_type.h: Undefine Bool if defined
X11's use of #defines strike again.

Fixes tint -> dawn autoroller

Change-Id: Iccac4f2ac3a548b490dafc54e024a858419080e6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39760
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
2021-02-01 14:55:13 +00:00
Ben Clayton
e43c830b14 Add ProgramBuilder::Wrap()
Follows the basic shallow-clone pattern of type::Manager::Wrap(),
which this new method uses.

Use ProgramBuilder::Wrap() in the writers, where we were previously
using type::Manager::Wrap(). This simplifies some of the logic
(especially around AppendVector()).

This is now required as semantic information is generated inside the
writers for newly constructed AST nodes, and we need to register
this semantic info so it can be fetched later.

Change-Id: Ia362204a1f13054e50545baeb7d269f210a36826
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39004
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-29 11:59:32 +00:00
Ben Clayton
708dc2d040 Simplify calls to ast::Node::[to_]str()
Add helpers on Program and ProgramBuilder that significantly simplify
usage.
Also demangle - this also reduces a bunch of copy-pasta code.

Change-Id: I6215c346e7f6e49c20aced058a6150603253ed93
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39342
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-29 11:22:40 +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
1f7e18bbc0 ast::Builder: Add shortcuts to the Program methods
This builder will be merged into ProgramBuilder, where these will become methods.
To breakup this change, perform the refactoring as a separate change.

Bug: tint:390
Change-Id: I2c9151cd9f198e99d88eaf296dd994293df6c425
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38720
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-26 16:57:10 +00:00
Ben Clayton
8d391f7a10 ast::TypesBuilder: Change const fields to getters
This is required in order to support move operators for TypesBuilder.

Bug: tint:390
Change-Id: I9667bda5f5be267df092f5cd94dc40db053ae6e2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38555
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-26 16:57:10 +00:00
Ben Clayton
a458c57662 type::Manager: Add Wrap()
Wrap returns a new Manager created with the types of `inner`.

The Manager returned by Wrap is intended to temporarily extend the types of an existing immutable Manager.

Bug: tint:390
Change-Id: I46bebf8b83cf7987ddcf2513c54f9f885a028c60
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38551
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-26 16:57:10 +00:00
Ben Clayton
44bec80e5f Demangler: Change signature of primary Demangle() function
Have this take a SymbolTable instead of a Program.
Program will be split into Program (immutable) and ProgramBuilder (mutable). We'll need Demangler to support both.

Bug: tint:390
Change-Id: I6447dd9674919d4867ed8ba126880cdfd9bf7128
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38550
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-26 16:57:10 +00:00
Ben Clayton
cf757e3a89 Program: Remove deprecated types and nodes methods
Fixup all usages

Bug: tint:390
Change-Id: I14c9a0420be7da2d26bf21bce96ca0ded0978711
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38548
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
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
281b602f59 type::Manager: Simplify interface and use BlockAllocator
Internally use BlockAllocator to allocate the types.
When we optimize the allocation patterns of BlockAllocator, this will now benefit both AST nodes and types.

Remove Reset(). It was not used.

Remove type::Manager::Get(std::unique_ptr<type::Type>) - this was used (via Module::unique_type) in one place, which has easily been migrated to using the standard Module::create<>.

Replace all remaining uses of std::unique_ptr<> of types in tests with the standard create<> so we can guarantee uniqueness of the types.

Change-Id: Ib0e1fe94e492b31816450df5de0c839a0aefcb9e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38362
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-21 16:35:10 +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
Ben Clayton
207b5e2de1 Move tint::ast::type to tint::type
Despite `tint::ast::type::Type` being in the AST namespace, these classes are clearly not AST nodes:
* They don't derive from ast::Node
* They're deduplicated by the type manager
* None of the types have an Source - they have no lexical declaration point
* The fact we have `ast::Struct` and `ast::type::Struct` clearly demonstrates what is an AST node, and what is a type.
* We have code scattered in the codebase (TypeDeterminer, writers, etc) that create new types after parsing - so clearly not part of the original syntax tree.

Types in tint are closer to being semantic info, but due to the parse-time generation of types, and tight dependency of ast::Nodes to types, I'd be reluctant to class these as semantic info. Instead, put these into a separate root level `tint::type` namespace and `src/tint` directory.

The fact that types exist in the ast::Module has already caused bugs (https://dawn-review.googlesource.com/c/tint/+/37261). This is a first step in separating out types from the ast::Module.

Bug: tint:390
Change-Id: I8349bbbd1b19597b8e6d51d5cda0890de46ecaec
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38002
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-21 15:42:10 +00:00