Commit Graph

1731 Commits

Author SHA1 Message Date
David Neto 8f9de8a509 spirv-reader: Add method to be used in tests later
Bug: tint:508
Change-Id: Ifd83bcfe168a2be0a11e8a81e392ba20101fc586
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48865
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-04-26 09:56:05 +00:00
James Price 12353bc5fd Remove duplicate using directive
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>
2021-04-23 22:11:14 +00:00
Ben Clayton 6052d41d51 sem::Alias::type_name(): Optimize complexity
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>
2021-04-23 18:21:34 +00:00
Ben Clayton b205b872ae reader/spirv: Don't create disjoint AST nodes
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>
2021-04-23 18:19:54 +00:00
Antonio Maiorano ab215981fe Fix linux dawn roll
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>
2021-04-23 18:09:34 +00:00
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
Ryan Harrison 099ceb2050 Remove use of UnwrapAll()
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>
2021-04-23 08:36:33 +00:00
Antonio Maiorano 0b9ea91fcf Make ast::StructMember accept typ::Type
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>
2021-04-23 08:35:13 +00:00
Antonio Maiorano 05abdf5096 wgsl parser: make type_decl() return typ::Type
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>
2021-04-23 08:19:33 +00:00
Brandon Jones 145f865fb1 Add texture_external parsing and intrinsic overloads
Adds texture_external to ParserImpl. Adds texture_external overloads to
TextureSample, TextureLoad, and TextureDimensions to the intrisic table.
Adds corresponding tests.

Bug: dawn:728
Change-Id: I5e5557a10327f8c3d3044319decd748f812ecf3e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48722
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
2021-04-22 22:47:03 +00:00
Ryan Harrison add325bb61 Add check for access controls on Storage Textures
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>
2021-04-22 19:57:33 +00:00
Antonio Maiorano 26fa9927e8 Constify Type* constructor args for ast/sem classes
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>
2021-04-22 15:34:13 +00:00
Ben Clayton e204f27f86 ProgramBuilder: Migrate any remaining types to typ::*
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>
2021-04-22 14:40:23 +00:00
Ben Clayton 7241a504f0 ProgramBuilder: Migrate arrays to typ::TypePair
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>
2021-04-22 14:32:53 +00:00
Ben Clayton fdb91fd85e ProgramBuilder: Migrate matrices to typ::TypePair
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>
2021-04-22 14:30:53 +00:00
Ben Clayton e30ffeb02a ProgramBuilder: Migrate vectors to typ::TypePair
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>
2021-04-22 14:24:43 +00:00
Ben Clayton 43073d8aa3 tests: Use ProgramBuilder helpers where we can
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>
2021-04-22 13:50:53 +00:00
David Neto 261642e4e3 Update spirv-reader readme
Change-Id: I933cab554b3bbd278a23b892535cba4992649002
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48760
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
2021-04-22 12:54:43 +00:00
David Neto 943ded79d2 spirv-reader: support OpTranspose
Change-Id: If338b22b703257e863e511579cfd3abcaa0bdfe7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48761
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
2021-04-22 12:29:45 +00:00
Ben Clayton d5ceafee16 resolver: Temporarily disable vertex position validation
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>
2021-04-21 22:55:42 +00:00
Ben Clayton 42708348b7 resolver: Validate uniform buffer types
Fixed: tint:210
Change-Id: I7763ca23a5dce09755a1ca71d35f24897a875ac0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48604
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-21 17:55:12 +00:00
Ben Clayton 8db818840e Resolver: Validate array el is not [[block]] struct
Fixed: tint:90
Change-Id: I9500a8c7fad9acf5f797dd2903217821c953acb6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48421
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-21 16:45:02 +00:00
James Price 2dd393729c Require vertex shaders to return builtin(position)
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>
2021-04-21 16:13:42 +00:00
Ben Clayton fec63b7aa6 Resolver: Add ast -> sem type resolving
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>
2021-04-21 13:47:12 +00:00
Ben Clayton a922e65d72 ProgramBuilder: Generate type pairs for primitives
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>
2021-04-21 13:37:22 +00:00
Ben Clayton 440636c15c Add typepair.h
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>
2021-04-21 13:32:22 +00:00
Corentin Wallez c0810e7e6e Fix gn check in Chromium
- 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>
2021-04-21 09:18:52 +00:00
Ryan Harrison de8f1332f6 Validate Storage Textures and update existing tests
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>
2021-04-20 20:47:51 +00:00
Ben Clayton 5b3d88b748 resolver: Fix for ICE: node not reached by the resolver
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>
2021-04-20 18:11:51 +00:00
Ryan Harrison 83116d298c Add validation rules for multisampled textures
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>
2021-04-20 16:09:21 +00:00
Ryan Harrison a32e4f66e5 Remove 2d Array Multisampled Textures
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>
2021-04-20 16:00:21 +00:00
Ben Clayton 913a2f4b2a sem::StructType remove symbol()
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>
2021-04-20 15:21:21 +00:00
Ben Clayton 8a8d26bbd9 ast: Add type nodes
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>
2021-04-20 15:04:21 +00:00
David Neto ba6f260629 spirv-reader: Remove stale TODO
Change-Id: Icd9bf067160c7b79b3c5d8c3123ac94a8f29376c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48400
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
2021-04-19 23:16:23 +00:00
Antonio Maiorano aea9c68de9 Move type/* files to sem/ directory
Bug: tint:724
Change-Id: I45d83d32dbce1fbee265810615c18219ef879b20
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48363
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
2021-04-19 22:54:43 +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
Antonio Maiorano 3aa226138e Rename type::Struct to type::StructType
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>
2021-04-19 21:31:43 +00:00
Antonio Maiorano cf4057be01 Rename type::Array to type::ArrayType
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>
2021-04-19 21:13:12 +00:00
Brandon Jones e94237dcdd Add ExternalTexture Type
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>
2021-04-19 20:35:13 +00:00
Ben Clayton cf28b5aafe resolver: Call ValidateVariable() for globals
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>
2021-04-19 20:23:12 +00:00
Ben Clayton 85bfea6edd resolver: Validate storage buffers have the [[block]] decoration
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>
2021-04-19 20:20:33 +00:00
David Neto 8b50537767 spriv-reader: handle loop-header with internal divergence
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>
2021-04-19 19:54:32 +00:00
Antonio Maiorano ff7c293141 Fix clang-cl build
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>
2021-04-19 19:41:02 +00:00
Ben Clayton 6fcefe4c25 Resolver: Check that every AST node is reached once
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>
2021-04-19 19:16:12 +00:00
Ben Clayton b8ea59149e writer/msl: Fix all tests that had unreachable AST nodes
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>
2021-04-19 16:52:42 +00:00
Ben Clayton 917b14b626 CloneContext: Assert objects are owned by the program
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>
2021-04-19 16:50:23 +00:00
Ben Clayton b895ba1cea writer/hlsl: Fix all tests that had unreachable AST nodes
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>
2021-04-19 16:06:42 +00:00
Antonio Maiorano 3244bcdb67 Rename sem_*.cc to *.cc
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>
2021-04-19 15:20:03 +00:00
Ben Clayton 08031f326c writer/hlsl: Implement generation of module-scope vars
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>
2021-04-19 15:11:22 +00:00
Ben Clayton f4d3949c78 writer/wgsl: Fix all tests that had unreachable AST nodes
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>
2021-04-19 14:40:49 +00:00
Ben Clayton 8198d57ff2 writer/wgsl: Handle inferred-type variable declarations
Fixes a null-pointer deference crash.

Bug: tint:672
Change-Id: Icfcb1c8754950ea0e074a0b392d0b34748a13827
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48225
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
2021-04-19 14:37:49 +00:00
Ben Clayton 238de88266 writer/spirv: Fix all tests that had unreachable AST nodes
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>
2021-04-19 14:26:00 +00:00
Ben Clayton b696607338 writer/msl: Add TINT_UNIMPLEMENTED() for module-scope vars
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>
2021-04-19 14:12:29 +00:00
Ben Clayton 4649377d05 transform: Don't create detached AST nodes
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>
2021-04-17 06:02:21 +00:00
Ben Clayton f32719ff85 reader/spirv: Don't create detatched AST nodes
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>
2021-04-17 06:01:41 +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 49668bbe17 ProgramBuilder: Tweak Return() signature
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>
2021-04-17 05:32:01 +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
Ben Clayton 0ecea57a8a Add ShareableCloneable, dervies from Cloneable
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>
2021-04-16 20:30:21 +00:00
James Price 395b48825a Add support for [[builtin(workgroup_id)]]
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>
2021-04-16 19:57:34 +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 26cd48603d Castable: Add FLAGS template argument for Is,As
[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>
2021-04-16 15:05:44 +00:00
Ben Clayton 7687ec1fa3 Kokoro: Test that the _other.cc files also build
Change-Id: I908b9250de839435c3b355fb89b0bb72ab821a54
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48043
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
2021-04-16 14:43:34 +00:00
Ben Clayton fe70c75bc5 *_other.cc: Remove unused parameters
Change-Id: I966461fdf70a8d23c05f4e5daa5795b7c54e186b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48042
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
2021-04-16 11:40:14 +00:00
Ben Clayton a2596bb696 Fix tmpfile_other.cc
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>
2021-04-16 11:27:44 +00:00
Ben Clayton 316dcd4166 UintLiteral::to_str() suffix with 'u'
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>
2021-04-16 10:44:54 +00:00
Ben Clayton 98b084e4a4 spirv/reader: Implement derivative intrinsics
Fixed: tint:719
Change-Id: Iaa5e44a574b2843f4f6f9264b8baa8d199acd70f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47770
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
2021-04-16 10:43:34 +00:00
Ben Clayton 648b05e4e2 Fix AttemptTintPadSymbolCollision test
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>
2021-04-16 10:37:34 +00:00
James Price 8cb5a23afb Automatically clone all symbols in CloneContext()
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>
2021-04-16 10:29:54 +00:00
Ben Clayton ed8332a0d1 writer/wgsl: Omit the newline after [[access()]] decorations
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>
2021-04-16 10:09:44 +00:00
Ben Clayton bfae10525f writer/msl: Drop leading _ for _tint_pad_N fields
Bug: tint:640
Change-Id: I4415dcd2c46bff07bef302620321acc3847a6399
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47771
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
2021-04-16 10:03:04 +00:00
Ben Clayton 1773535e1c Resolver: Validate <storage> var types
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>
2021-04-16 09:26:14 +00:00
Ben Clayton 4db10dd496 Resolver: Handle AccessControl in Canonical()
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>
2021-04-16 08:47:14 +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
Antonio Maiorano fc03a46516 Validate array stride
Fix: tint:707
Change-Id: I4439b10f173d8753bd1d407629954a7dad61a679
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47826
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
2021-04-16 02:36:44 +00:00
Antonio Maiorano 2e0bd4acb0 Resolver: improve array handling
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>
2021-04-16 01:15:43 +00:00
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 f0c816a757 ast: Validate that ASTs are all part of the same program
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>
2021-04-15 17:47:23 +00:00
Ben Clayton 31804e09ed reader/spirv: Fix ExpectEq() tests
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>
2021-04-15 15:28:13 +00:00
Ben Clayton cc46e393e4 Add NOLINT to the new long test names
Suppresses cpplint warnings.

Change-Id: I2c8865709037b67f7af96e737dad8b6c179c5110
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47760
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
2021-04-15 00:07:38 +00:00
James Price 0d96c1d644 transform/CanonicalizeEntryPointIO: Sort struct members
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>
2021-04-14 22:51:18 +00:00
James Price 172215e118 writer/hlsl: Fix mapping for [[builtin(front_facing)]]
Change-Id: I9493b8599e306113d4ede7b109adf5d119fbe4d6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47821
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
2021-04-14 22:15:58 +00:00
James Price 0ba48eeb52 writer/msl: Enable validation for entry point tests
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>
2021-04-14 22:14:08 +00:00
James Price 43b84d69dd transform/CanonicalizeEntryPointIO: Fix symbol renaming
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>
2021-04-14 22:08:48 +00:00
James Price edd4d3cc3b Rename builtin(sample_mask_{in,out}) to builtin(sample_mask)
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>
2021-04-14 16:44:38 +00:00
James Price 88d7edcf7a Rename builtin(frag_coord) to builtin(position)
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>
2021-04-14 16:37:18 +00:00
Arman Uguray d13982ff2e [msl-writer] Support MSL compiler validation in unit tests
The MSL writer GTest harness (TestHelper) now provides a function to
invoke the XCode SDK Metal compiler for the MSL output of a given
tint::Program.

The tint_unittests binary now provides the `--validate-msl` and
`--xcrun-path` command-line flags to optionally enable MSL validation
and to configure its path.

The MSL validation logic itself is conditionally compiled based on the
TINT_BUILD_MSL_WRITER define. The TINT_BUILD_* flags were previously
not propagated to the GTest binary which this CL addresses by linking
the common/public tint configs when building the tint_unittests_main
target.

Bug: tint:535
Fixed: tint:696
Change-Id: I08b1c36ba59c606ef6cffa5fa5454fd8cf8b035d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45800
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Arman Uguray <armansito@chromium.org>
2021-04-14 15:36:58 +00:00
Ben Clayton f8313e5a6e transform/BoundArrayAccessors: Fix symbol renaming
We need to pre-clone the source program's symbols before creating the symbols `min` and `arrayLength`, otherwise the original program's symbols will be suffixed with a number to make them unique.

Fixes Dawn tests that triggered this issue.

Bug: tint:712
Change-Id: Ie1cf6cbcf2050a2ce1a94acf0ae131a06f635820
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47761
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-14 14:29:28 +00:00
Ben Clayton 3dd4fae775 writer/hlsl: Use Symbols().New()
Instead of rolling another implementation inside GeneratorImpl.

Bug: tint:712
Change-Id: I26af0d68f6529c0c6dc45f51233f4618389edb55
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47638
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
2021-04-13 23:48:07 +00:00
Ben Clayton 5c34060a4e transform/hlsl: Use Symbols().New() for empty entry point
And drop the leading understore, it's no longer needed.

Bug tint:712

Change-Id: Ic0ad304119ceb148984d2fa0a5e9e61f2c3a89fd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47637
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
2021-04-13 23:46:17 +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 a5a31d35f6 transform/EmitVertexPointSize: Use SymbolTable::New()
And clean up some code in the process.

Avoids potential symbol collisions. Simplifies the logic.

Bug: tint:711
Bug: tint:712
Change-Id: I7c41dff81e1fb2abfd3f5d3fecf625a27c42f14d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47635
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
2021-04-13 23:30:57 +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 f01ed13a25 transform/VertexPulling: s/_tint/tint
Don't use a leading underscore in identifiers. Tint cannot parse these, they're illegal in MSL.

Bug: tint:640
Change-Id: I6e923cf3317a646cc5f4f2a10a7a522036000c70
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47634
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-13 23:08:37 +00:00
Ben Clayton 93cd23c01b transform/VertexPulling: Use SymbolTable::New()
And clean up some code in the process.

Avoids potential symbol collisions. Simplifies the logic.

Bug: tint:712
Change-Id: Ibce5ccbd4c7fd45d5bf29906b5a83b3637b6cdcc
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47633
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-13 23:04:07 +00:00
Ben Clayton bd1597a545 transform/Renamer: Use SymbolTable::New()
Avoids potential symbol collisions. Simplifies the logic.

Bug: tint:712
Change-Id: I4416307b10f2dbe38964b6fd9342042c7e5505ec
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47632
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
2021-04-13 22:58:07 +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
David Neto 46bb2bc935 spirv-reader: handle break and continue from if-selection header
Fixed: tint:243, tint:494
Change-Id: I6baf3360b44042b52f510b8f761376e1daab878f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47540
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
2021-04-13 19:52:08 +00:00
Arman Uguray 66a8efc6ae Support optional file extension in TmpFile
TmpFile can now be supplied an optional file extension. This change
was motivated by validation work using the XCode SDK Metal compiler,
which expects a source file to end with the ".metal" extension.

See
https://developer.apple.com/documentation/metal/libraries/understanding_the_metal_shading_language_filename_extension

Bug: tint:535
Change-Id: I5d44baa20ba350530ace46569e238c4627135e51
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45720
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Arman Uguray <armansito@chromium.org>
2021-04-13 19:07:47 +00:00