Commit Graph

25 Commits

Author SHA1 Message Date
Ben Clayton 3b3cf5f2d3 ProgramBuilder: Fixup last few uses of sem::Type*
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>
2021-04-23 15:41:34 +00:00
Antonio Maiorano 3751fd2290 Move all classes from namespace `type` to namespace `sem`
Bug: tint:724
Change-Id: I4eeabab9b00b6b28f61645bd95d326fb48609bf0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48362
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
2021-04-19 22:51:23 +00:00
Ben Clayton 169512e499 ProgramBuilder: Add a WrapInStatement() overload for literals
Lets you pass AST literals to WrapInStatement() / WrapInFunction()

Change-Id: I69d8918a61f0ea25193b64df38ed6cd606c7f044
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48047
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-17 05:52:11 +00:00
Ben Clayton a0cf62f415 Fix all tests that shared AST nodes
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>
2021-04-17 00:42:41 +00:00
Antonio Maiorano 5cd71b8c0a Rename semantic to sem
* Rename namespace semantic to sem
* Rename directory src/semantic/ to src/sem/

Bug: tint:724
Change-Id: I76383b821fbca7f1037a803c497b595a706dcb06
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48120
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-04-16 19:07:51 +00:00
Ben Clayton e6995de232 Add ProgramID feed it into all ast::Nodes
This will be used to detect accidental leaks of program objects between programs.

Bug: tint:709
Change-Id: I20f784a2c673d19a04a880b3ec91dfe2eb743bdb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47622
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-04-13 23:27:27 +00:00
Ben Clayton ab26a8fd34 hlsl/writer: Transpose matrices
HLSL's matrices are declared as <type>NxM, where N is the number of
rows and M is the number of columns. Despite HLSL's matrices being
column-major by default, the index operator and constructors actually
operate on row-vectors, where as WGSL operates on column vectors.
To simplify everything we use the transpose of the matrices.

This is the same approach taken by SPIRV-Cross.

Change-Id: I98860e11ff1a68132736980f694b2f68b633ef83
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46873
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-07 15:54:11 +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 e09989ae34 Move assignment validation from Validator to Resolver
* With this change, ProgramBuilder::WrapInStatement(expr), which
produced an "expr = expr" assignment expression, would fail validation
in some cases like for call expressions. Replaced this with a
declaration of a variable with type inferred from expr.

* Moved existing validation tests to resolver\assignment_validation.cc,
and added missing tests: AssignFromPointer_Fail.

* Fixed broken tests as a result of this change.

Bug: tint:642
Change-Id: I6a738b67edb0e116a46e936d652c2dcb4389281e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46161
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-03-31 13:26:43 +00:00
Antonio Maiorano 39a65a1d1e Add type inference support to Resolver
There is still no way to spell this out in WGSL, but this adds support
for VariableDecls with an ast::Variable that has nullptr type. In this
case, the Resolver uses the type of the rhs (constructor expression),
which is stored in semantic::Variable.

Added tests for resolving inferred types from constructor, arithmetic,
and call expressions.

Bug: tint:672
Change-Id: I3dcfd18adecebc8b969373d2ac72c21891c21a87
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46160
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
2021-03-31 12:46:52 +00:00
Antonio Maiorano 03c01b5266 Move function validation from Validator to Resolver
* Fixed many tests that now failed validation. Most of the time,
functions declared that they returned a type, but with no return
statement.
* ProgramBuilder::WrapInFunction now returns the function is creates,
and std::moves its StatementList.
* ProgramBuilder: Added Return function to create ast::ReturnStatements
more easily.

Bug: tint:642
Change-Id: I3011314e66e264ebd7b89bf9271392391be6a0e5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45382
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
2021-03-19 14:04:51 +00:00
Ben Clayton 8454d824d4 ast: Replace IsValid() with TINT_ASSERT()
The readers must not produce invalid ASTs.
If readers cannot produce a valid AST, then they should error instead.
If a reader does produce an invalid AST, this change catches this bad behavior early, significantly helping identify the root of the broken logic.

IsValid() made a bit more sense in the days where the AST was mutable, and was constructed by calling setters on the nodes to build up the tree.
In order to detect bad ASTs, IsValid() would have to perform an entire AST traversal and give a yes / no answer for the entire tree. Not only was this slow, an answer of 'no' didn't tell you *where* the AST was invalid, resulting in a lot of manual debugging.
Now that the AST is fully immutable, all child nodes need to be built before their parents. The AST node constructors now become a perfect place to perform pointer sanity checking.

The argument for attempting to catch and handle invalid ASTs is not a compelling one.
Invalid ASTs are invalid compiler behavior, not something that should ever happen with a correctly functioning compiler.
If this were to happen in production, the user would be utterly clueless to _why_ the program is invalid, or _how_ to fix it.
Attempting to handle invalid ASTs is just masking a much larger problem.

Let's just let the fuzzers do their job to catch any of these cases early.

Fixed: chromium:1185569
Change-Id: I6496426a3a9da9d42627d2c1ca23917bfd04cc5c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44048
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
2021-03-10 11:41:49 +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 c7ca7668cc Fix TypesBuilder::builder pointer initialization
When copying or moving a ProgramBuilder, we always want the ProgramBuilder::ty to point back to the owner.
We were previously std::move()'ing the ty field, which is not correct - this will result in the TypesBuilder pointing to the wrong ProgramBuilder.

I'm not sure why we've not seen any issues with this using clang, but running under MSVC immediately highlighted this brokenness.

Change-Id: I4293bb00ac4fbdfa66d12b1504a7bd060e014cd6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41861
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-17 16:23:52 +00:00
Ben Clayton 46d78d731b ProgramBuilder: Add Symbol overloads of Var(), Const(), Global(), GlobalConst()
Useful if you have a symbol already

Change-Id: Ib9e15ea761f58ee67dc3cc722d9129cd5369d92b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41481
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-10 21:34:26 +00:00
James Price 558385357f [ast] Capture global declaration order
Adds a vector<CastableBase*> to ast::Module which stores the list of
global variables, functions, and types, in the order that they were
declared.

This will be used to fix validation and backend issues around name
uniqueness.

Change-Id: I14491f6ebc0fc7341bd3fb3b3f408faa234a91f7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41301
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-02-09 21:39:10 +00:00
Ben Clayton 401b96b9bb Simplify usage of the TypeDeterminer in tests
Make private all TypeDeterminer::DetermineXXX() methods, forcing all tests to use the root-level TypeDeterminer::Determine() method.
Remove TypeDeterminer::RegisterVariableForTesting().

The main use for calling the TypeDeterminer::DetermineXXX() methods was to perform type determination on a partial AST.
This was messy and often resulting in multiple calls into TypeDeterminer. Most tests already perform a full TypeDeterminer::Determine() call when the program is built, so many of these were redundant.
The exposure of these internal methods for testing also makes refactoring the TypeDeterminer extremely difficult.

Add a number of ProgramBuilder helper methods for attaching the partial AST in these tests to the root of the AST, greatly simplifying the use of the TypeDeterminer:
* ProgramBuilder::Global() and ProgramBuilder::GlobalConst() are helpers that register the variable returned by ProgramBuilder::Var() and ProgramBuilder::Const(), respectively.
* ProgramBuilder::WrapInFunction() is a variadic function that accepts variables, expressions and statements, attaching these to the root of the AST via a dummy function.

Most test classes now no longer use their own TypeDeterminer, and instead properly depend on the automatic type determination performed at Program build time.

Bug: tint:390
Change-Id: Ie901890420c5de170cdf2a7aaef9b96fc3bebd60
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40062
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
2021-02-03 17:19:59 +00:00
Ben Clayton 3335254c1c Migrate to using semantic::Expression
Remove the mutable `result_type` from the ast::Expression.
Replace this with the use of semantic::Expression.

Bug: tint:390
Change-Id: I1f0eaf0dce8fde46fefe50bf2c5fe5b2e4d2d2df
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39007
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-29 16:43:41 +00:00
Ben Clayton 7fdfff1de4 Add semantic::Node
Rename Program(Builder)::Nodes to ASTNodes, and add SemNodes.

Bug: tint:390
Change-Id: If501ea5f5d1cbb4bc2673b303aa8ebce7195e2c3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39005
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-29 15:17:30 +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 dd1b6fca9f Introduce semantic::Info
Will hold the mutable fields that currently reside in the otherwise immutable-AST.

Change the AST string methods to accept a `const semantic::Info&`. This is required as some nodes include type-resolved information in their output strings.

Bug: tint:390
Change-Id: Iba494a9c5645ce2096da0a8cfe63a4309a9d9c3c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39003
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2021-01-29 10:55:40 +00:00
Ben Clayton 844217fa34 Add Diagnostics to Program, reader::[wgsl,spirv]::Parse()
By putting diagnostics into the program, we can hold all the diagnostic messages for parsing and type determination in one place.
This also means that we can simplify the public WGSL and SPIR-V Parser interfaces to a single function.

Change-Id: Ib6ab5fa180addd45c4aaf0c6b192d47182ffb50a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38920
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
2021-01-27 18:49:05 +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