Previously when comparing a "bestType" that's device-local with a
"current" that's non-device-local, the logic to favor device-local
memory would be skipped and the types compared on their size. This lead
to incorrect memory types selection on some configurations (where the
small device local heap was selected).
Bug: dawn:659
Change-Id: Ie764815082ebeef845b70077fe630df05bcdb92b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/39500
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Vulkan requires that linear and opaque resources be placed in different
"pages" of bufferImageGranularity size, as some hardware uses the page
table to contain some compression bits or other stuff. Make Dawn honor
this limit by aligning all allocations to bufferImageGranularity. This
is pretty bad and should be improved later.
Also does some cleanups:
- Add kMappableBufferUsage to represent all mappable usages.
- Remove the proxy function for resource management from
vulkan::Device and call ResourceMemoryAllocator directly.
- Use an enum to make the difference between mappable, linear and
opaque resources.
This issue was found while doing a change of the memory type selection
in Vulkan, that started failing some unrelated tests on Nvidia. Without
knowing the details of the HW or the driver it is really hard to write
tests, except by copy-pasting a failing test. This is why there is no
test added in this CL, and instead will rely on tests not failing with
the follow-up CL.
Bug: dawn:659
Change-Id: Ib7c1f3f1949457e04ca8e23d212dc60af7046213
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/52920
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
This reverts commit 055901b3cd406c6f3d35a331813f2674a604839a.
Reason for revert: Is causing a *lot* of test failures in the tint -> dawn roll
Original change's description:
> inspector: Remove legacy shader IO support
>
> Bug: tint:697
> Change-Id: Ifa01fa459daa5c2f4b47833cd14ea1b303455c34
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55402
> Reviewed-by: Ben Clayton <bclayton@google.com>
> Commit-Queue: James Price <jrprice@google.com>
> Kokoro: Kokoro <noreply+kokoro@google.com>
> Auto-Submit: James Price <jrprice@google.com>
TBR=rharrison@chromium.org,bclayton@google.com,jrprice@google.com,noreply+kokoro@google.com,tint-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I692a09eeda8e9e7271f032aa6d4cf0630d54c26a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: tint:697
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55580
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
VectorInsertDynamic normaly inserts a temporary variable.
But if the source vector is already a hoisted variable, then
reuse that instead. This avoids defining the same name twice.
Bug: tint:804
Change-Id: I69c20c11d462c148261bb0646db698dc7850495d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55362
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Use a DisableValidationDecoration to allow these storage classes only
for variables generated by the SPIR-V sanitizer.
Fix or delete all of the tests that were wrongly using these storage
classes.
Bug: tint:697
Change-Id: Ife1154f687b18529cfcc7a0ed93407fd25c9868e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55404
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Composite insert normally inserts a temporary variable.
But if the composite value is already a hoisted variable,
then reuse that instead. This avoids defining the same name twice.
Also add AddressOfIfNeeded and use it when processing the operand
of an OpCopyObject or when making a let-declaration. Only take the
address in these cases when the corresponding SPIR-V type is a
pointer.
Bug: tint:804
Change-Id: I44f4289a557db919d8f1805e187a9b2a95c1efe0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55361
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>
After adding some e2e tests for 3D texture T2T copy on non-zero mip
levels, it turns out that there is a bug in the e2e test itself.
The 3D texture copy splitter works well.
Bug: dawn:547
Change-Id: I51aef7aaca4caf9c86bfb0590eb0288d17731ba5
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/55144
Commit-Queue: Yunchao He <yunchao.he@intel.com>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Tint now matches the behavior of spirv-cross for shaders to use the
arrayLength builtin, and so needs a uniform buffer containing storage
buffer lengths to be passed to shaders that use this builtin.
Fixed: tint:256
Change-Id: Ib51cc4a3c6f7c2cdea867a23ed868d9d3740d734
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/55181
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Also remove the unreachanble constructor logic in EmitHandleVariable.
Variables of the handle storage class cannot have initializers.
Fixed: tint:173
Change-Id: I7c997a8b6a70308ff9b5c42fa1198810ee365bac
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55258
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
We shouldn't be calculating array strides with the packed element size.
Bug: tint:180
Bug: tint:649
Bug: tint:898
Change-Id: Ifa23517480435610b20c1597c2fc2c2837f4280a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55257
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Remove special case handling of pointers and values related
to builtins SampleId, VertexIndex, and InstanceIndex.
These map to private variables with store type matching the
type stated in the SPIR-V code. There is no need to generate
special case code for user-written functions accessing those variables.
Therefore:
- Remove SkipReason enums associated with those builtin inputs
- Remove newly unreachable code.
Bug: tint:508
Change-Id: I22ea86d49e14f171a92863d9f02145606ad37683
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55321
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: David Neto <dneto@google.com>
Varaibles can infer types now, in which case the type_ field is null.
Fixed: chromium:1221120
Change-Id: I0cb2a6a2e8128c56625f48940cf73cf4cadb22ce
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55252
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This CL adds the support ofwgpu::CopyTextureForBrowserOptions::alphaOps
for CopyTextureForBrowser(), which allows the user to specify the alpha
operations (DontChange, Premultiply, Unpremultiply) when calling
CopyTextureForBrowser().
BUG=chromium:1217153
Change-Id: Idb5dd4482b33f2ada9aafc24380afeb48f2e26bc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/54800
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Shaobo Yan <shaobo.yan@intel.com>
Inline the `continuing` block in the places where `continue` is called.
Simplifies the emission, and fixes emission of `let` statements in the loop.
This fix matches the same approach in writer/hlsl.
See: https://dawn-review.googlesource.com/c/tint/+/51784
Fixed: tint:833
Fixed: tint:914
Change-Id: If4d8cde62dfaf8efa24272854ca7ff5edc0a8234
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55341
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
ShaderRobustnessPerf:
SPIRV-Cross apparently generates incorrect code for MSL and OpenGL, when
attempting to initialize workgroup memory. Disable these tests when not
running with use_tint_generator.
D3D12DescriptorHeapTest:
The UBO layout for a mat2x2 was incorrect, but the test relied upon this.
Temporarily disable the test so we can roll the fix.
Bug: dawn:945
Bug: dawn:946
Change-Id: I810249b26a8a0b583796c0ba63b8a703eb9540e4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/55248
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Mostly just deleting unneeded code, and a few additional cleanups as a
result.
Bug: tint:697
Change-Id: I31ceea93feb34994f51a1b6d294a35cf0c127447
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55282
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Generate a uniform buffer that will receive the lengths of all storage
buffers, and use this to implement calls to arrayLength(). The
transform is provided with a set of mappings from storage buffer
binding points to the corresponding index into the array of buffer
lengths. The transform reports whether it generated the uniform
buffers or not.
Use this transform from the MSL sanitizer, using the binding number as
the index into the array. This matches the behavior of spirv-cross,
and so works with how Dawn already produces this uniform buffer.
Bug: tint:256
Change-Id: I2682d2d024e8daa30f78270b8cfb6bbb32632133
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54480
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Zero the workgroup memory for all backends.
We can probably disable this for the backends that support workgroup zeroing, but that's an optimization we can perform later.
Fixed: tint:280
Change-Id: I9cad919ba3a15b8cedfe6939317d1f6b95425453
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55244
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Zero initializes all referenced workgroup storage classed variables used by each entry point.
Bug: tint:280
Fixed: tint:911
Change-Id: I3fca26a10f015f08fedef404720bbe6fd7b343a9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55243
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Instead of a ConstantBuffer.
HLSL requires that each structure field in a UBO is 16 byte aligned.
WGSL has much looser constraints with its UBO field alignment rules.
Instead generate an array of uint4 vectors, and index into this, much
like we index into [RW]ByteAddressBuffers for SSBOs.
Extend the DecomposeStorageAccess transform to support uniforms too.
This has been renamed to DecomposeMemoryAccess.
Change-Id: I3868ff80af1ab3b3dddfbf5b969724cb87ef0744
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55246
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
If during clone, we register a type, function or global declaration, we could end up with the declaration held twice by the AST Module.
AST nodes must only be referenced once.
Change-Id: I5c517699ea80422800639088b97a50ba9ac27b70
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55245
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Removing parsing support for the 'in' and 'out' storage classes is
enough to prevent anyone from using the old syntax. The Input and
Output storage classes will remain in the AST for now, as the SPIR-V
reader still has codepaths that use them, and the SPIR-V writer
currently still relies on them.
Bug: tint:697
Change-Id: Ifef9eda8bcbf2f243b1e1d8d4fab25784cd3f80e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54841
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Storage buffers are emitted as `ByteAddressBuffer`s in HLSL, so we have to jump through hoops to support atomic ops on storage buffer atomics.
Workgroup atomics are far more conventional, but very little code can be shared between these two code paths.
Bug: tint:892
Change-Id: If10ea866e3b67a093e87aca689d34065fd49b705
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54651
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Refer the TODO in ShaderModuleBase::InjectCompilationMessages to
issue dawn:944 for further tracking.
Bug: dawn:753
Change-Id: I2ff09ba5914f2df0182bf26e9001a66782811f06
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/55223
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Zhaoming Jiang <zhaoming.jiang@intel.com>
After implementing validation and fairly exhaustive tests, discovered
that conversion of scalar vector to bool vector did not work in the
spir-v backend. For module scope variables, we use and rely on the
FoldConstants transform to ensure no conversion needs to take place.
This is necessary because we cannot easily introduce temporary values
and refer to them when casting at module scope. Note that for the same
reason, module-level conversions are always constant foldable, so this
works. For function-level conversions, implemented support to emit a
comparison against a zero value, and store the result in the bool
vector.
Bug: tint:865
Change-Id: I0528045e803f176e03428bc7eac31ae06920bbd7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54744
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This is required for implementing module-level conversions in the spir-v
backend (upcoming CL).
Bug: tint:865
Change-Id: I7fd38c6b1628c791851917165991bc247fc113c2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54740
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Add the formatted tint messages in OwnedCompilationMessages, which will be emit after creating the shader module succeeds of fails.
This patch also change the compilation messages handling in creating shader modules. Now the compilation messages are separated from sparseResult, and should be handled manually. A new method, ShaderModuleBase::InjectCompilationMessages, is introduced to move a given OwnedCompilationMessages into the shader module, and emit the formatted tint errors and warnings. This method should be called explicitly on a valid or error shader module.
Bug: dawn:753
Change-Id: I5825186c6d9c4aa7725aebd0c302bfce5e1f37cf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/53890
Commit-Queue: Zhaoming Jiang <zhaoming.jiang@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This will be relied on by the upcoming arrayLength transform.
Update test expectations.
Change-Id: Ib74b647abcd6f4393f9899ce40bbf06f6e53e7f4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55180
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
The Resolver already has this information, so just propagate it to the
semantic variable.
Change-Id: Id9fc58d3fc706a1433aba7cb3845b4f53f3fc386
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55120
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>
Also tighten previous suppressions to only apply when using the Tint
generator.
TBR=bclayton@google.com
Bug: tint:904
Change-Id: I22210227c428eb38901904966ec2c3bd70ce607b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/55200
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Copies between 3D and 2DArray textures need to be done depth/array
slice by slice via CopyTextureRegion() API on D3D12 because this
API can copy one single subresource every time.
However, if both src and dst textures are 3D texture, we can copy
all depth slices in one shot, which is a fast path for this
copy scenario.
Bug: dawn:547
Change-Id: I950ed58319fb0f30dfc8a2de3e57e8a64406f7e4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/55140
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Allows for reflection of the specific textures that a sampler has
sampled.
BUG=tint:699
Change-Id: Iba47baf5c99c4d03671caf2c01747bd6ad12b1e2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54901
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
The WorkgroupSize builtin decoration applies to a composite constant.
Because WGSL does not yet support specializable constants for this,
use the *default* values for that SPIR-V spec constant.
Update end-to-end test expectations.
Fixed: tint:503
Change-Id: I012b316d13544ab9282e3276b58906327adab133
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41960
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
