This matches the term used in the WGSL spec.
Change-Id: I4603332b828450c126ef806f1064ed54f372013f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78787
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
1) Append "Shadow" to samplers representing depth textures.
2) Sampling a depth texture returns f32, not vec4<f32>
3) Sampling a depth texture requires a Dref parameter, so we must
generate one if none is provided.
4) GLSL requires Dref to be appended to the texture coordinates vector,
*unless* it's a samplerCubeArrayShadow, since this would require vec5.
In that case, it's passed as a separate parameter.
5) GLSL's textureGather() with a depth sampler always requires a refZ
parameter, so provide zero to emulate WGSL's compare-less textureGather().
6) texelFetch() does not support depth textures, so this will have to be
validated out.
7) textureOffset() does not support sampler2DArrayShadow in GLES, so this will
have to be validated out.
Bug: tint:1298
Change-Id: Idaebe89cac6c1ec97c50a361b1d3aa3b84fb6c12
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78760
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
The SPIR-V cleanup transforms were only being run if the
DecomposeStridedMatrix transform needed to be run - despite
the RemoveUnreachableStatements transform needing to be run, always.
Change-Id: I5dc28a79200ab67c96fb793980412a5632e26026
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78780
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Move builtin_to_string() and builtin_type() to
the CanonicalizeEntryPointIO transform. Use the former to
rename entry point IO variables to the gl_ names, and the latter
to cast values to the correct type.
Change-Id: Iddfad574ddd660ff1bfd89a399a001b967b6b67e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78380
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
The transform was not correctly inserting the intrinsic call after array element types.
Fixed: chromium:1290107
Change-Id: I7199d1846cb98305d789cf0bc362eb5872d9b917
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78542
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
More line() and less std::endl.
More automated indents and less manual spacing.
Put a single newline after every struct and function declaration.
Note that this does touch every test result, but only affects whitespace.
Change-Id: I7506b9029b79b91fb335911dba44369b36f09bbe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78300
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
OpenGL clips against [-w, w], while WebGPU uses [0, w]. The fix is to
modify gl_Position.z on output to match GLSL semantics. (This is the
same code used by SPIRV-Cross under the fixup_clipspace option.)
Bug: tint:1401
Change-Id: I1b1511352eee11cd9b095cd809aa8e1263c6cf4c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78261
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Change-Id: Id0c879c597e02da32350b2ee4878991fe78c470f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78221
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Bug: tint:1397
Change-Id: Ifd6870b3e7cba151c361bd21f9d3d42642ff6c26
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78060
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Stephen White <senorblanco@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>
This was made a warning in M97, and can now become a hard error.
Fixed: tint:1224
Change-Id: Ied72f6e28b3dc64a6ab832e0eac53f62ce045d40
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77700
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
Essentially, this adds GLSL to the list of backends to run.
Bug: tint:1358
Change-Id: I5075df32d935a3e08733daadbe5ac9dc1e13f03c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77220
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
This transform converts all separate texture/sampler references
in a program into combined texture/samplers. This is required for GLSL,
which does not support separate texture/samplers.
As input, the transform requires a map from the unique sampler/texture
pairs previously gathered by the Resolver to strings, which will be
used as the names of the newly-generated combined samplers. Note that
binding points are unused by GLSL, and so are set to (0, 0) with
collision detection disabled.
All function signatures containing textures or samplers are rewritten,
as well as function calls and texture intrinsic calls. For texture
intrinsic calls, a placeholder sampler is used to satisfy the subsequent
Resolver pass (GLSL texture intrinsics do not require a separate sampler,
but WGSL intrinsics do). The placeholder is also used if the shader
contains only texture references (e.g., textureLoad).
Bug: tint:1366
Change-Id: Iff8407d28fdc2a8adac5cb655707a08c8553c389
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77080
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
If a WGSL test contains no entry points, add an empty one.
In that case, do not pass its name to the generator, so we
generate code for all functions, so they aren't culled for
reachability.
Add new test results for formerly empty tests.
Bug: tint:1376
Change-Id: Ibf371b943fb273d44712dfcc9dc1b7bb4ab071db
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/76540
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Update validation error for invalid uniform array element alignment.
Update tests to either remove the @stride attribute or use a different
element type.
Bug: tint:1381
Change-Id: I50b52cd78a34d9cd162fa5f2171a5fd35dcf3b79
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77560
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Deprecate the old syntax. Migrate everything to the new syntax.
Bug: tint:1382
Change-Id: Ide12b2e927b17dc93b9714c7049090864cc568d3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77260
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: David Neto <dneto@google.com>
When these are used inside a function, we were not unwrapping the
array from the struct that we wrapped it in.
Fixed: tint:1385
Change-Id: Ide7bbd802394bf09819265be48d978ec9346adfe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77180
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Don't generate them either, which generated a lot of test churn.
Fixed: tint:1380
Change-Id: I0a7cfdd2ef0ffe8e7fda111fbc57997b36b949e0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77165
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Most of this change is fixing up the numerious tests that violated this rule.
Fixed: tint:1365
Issue: tint:1374
Change-Id: I38da27c7367277fe60857208170fec017e80bd25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/76400
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Remove the ICE check for expression behaviors always being either `{Next}` or `{Next, Discard}`. Unreachable code may be result in something else.
Add the RemoveUnreachableStatements transform to the SPIR-V writer sanitizer transform list. The writer cannot correctly handle unreachable statements.
Bug: tint:1369
Bug: chromium:1285622
Change-Id: I9fa54c6d2096b1ee633dd551b628c7dd3ba64fb5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/76300
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
All failures are now SKIP'ed.
Bug: tint:1358
Change-Id: If04d57b9e0b71dd877468b5dc277fbff82f36692
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/75220
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Constructing a null vector, matrix or array does not need to provide an explicit list of null elements. `T()` is a more efficient and readable equivalent.
Fuzzers like to generate enormous pathological composite types, which has been triggering OOM failures.
Bug: oss-fuzz:38095
Bug: oss-fuzz:39235
Bug: oss-fuzz:39246
Bug: oss-fuzz:39874
Change-Id: I910bb04bdd0e80532c09fc038be895ec37d3d380
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/75426
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Just like for divide, FXC fails with the exact same error when
performing a modulo on a value that FXC determines to be zero. We
address it in the same way as we do for divide.
This also fixes a couple of the vk-gl-cts tests for which I manually
generated expectation files for.
Bug: tint:1083
Change-Id: Ia388bf002112afded542adb791d37e88e35a77ff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/74220
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
FXC fails to compile when it determines that the rhs of an integral
division is zero with "error X4010: Unsigned integer divide by zero".
bclayton's fix (https://dawn-review.googlesource.com/c/tint/+/60500)
addressed cases for division by an integer constant 0. This CL adds the
missing support for division by integral vectors with 0 components.
FXC also fails on division by integral expressions that it can fold to
0. To handle these cases, we now emit a runtime check for 0 and replace
by 1. In the cases I've tested, FXC seems able to optimize these checks
away.
Bug: tint:1083
Change-Id: I02f08e9077882f03c1e42b62dacb742a48fa48ba
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/73580
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Change-Id: I32aaefe71ea728ae5118de08e5dd9073ddf2269e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/73740
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
FXC fails to compile code that assigns to dynamically-indexed fixed-size
arrays in structs on internal shader variables with:
error X3500: array reference cannot be used as an l-value; not natively
addressable
This CL detects this case, and transforms such assignments into copying
out the array to a local variable, assigning to that local, and then
copying the array back.
Also manually regenerate SKIPs for HLSL/FXC after this change, which
fixes 30 tests. Also exposes some "compilation aborted unexpectedly" now
that "array reference cannot be used as an l-value" has been fixed. For
tests that fail for both DXC and FXC, updating SKIPs to the DXC one to
help distinguish actual FXC bugs from valid errors.
Bug: tint:998
Bug: tint:1206
Change-Id: I09204d8d81ab27d1c257538ad702414ccc386543
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71620
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
The GLSL emitted for these was invalid, and we don't need these
structs since they're only used as the store types of buffers, which
are handled elsewhere.
Change-Id: I17c15e408b5c36e9b895e5950528a6d02d1802a6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72381
Reviewed-by: Stephen White <senorblanco@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
This attribute is now deprecated, and the warning is causing these
tests to fail.
Bug: tint:1324
Change-Id: I0665c9f3b657c8cb56e2252a23eb6184ea876075
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72420
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This has been deprecated and will soon be removed.
Bug: tint:1324
Change-Id: If5dbbc3a40d7591591fb2802dbe9c8dd5f96d299
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72087
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
These are no longer necessary and will soon be deprecated and removed.
Bug: tint:1324
Change-Id: I3fa076e7ce5eb36466d24c80fd1c83658c28c5ab
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72086
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Skipping those that are block-decorated is not correct, as
block-decorated structures can also have non-buffer usages. This is
even clearer now that WGSL has removed the block attribute.
Bug: tint:1324
Change-Id: I6484766a5c541d39e2dc08beb3ae7b889759a3fb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72083
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This fixes errors like "error X3500: array reference cannot be used as
an l-value; not natively addressable". Note that FXC treats matrices
like arrays. We still get this error for dynamically indexed arrays in
structs.
Also improved HLSL assign tests, and add missing ones for vector
indexing.
Manually removed 20 e2e skip hlsl SKIP files that are now passing with
this change.
Bug: tint:1333
Change-Id: If23881a667857a4d4ec6881e72666af0a666ef10
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71982
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
It seems we haven't run our e2e tests with FXC validation in a long time
- at least not since we've added the vk-gl-cts corpus.
Locally disabled exclusion of "/test/vk-gl-cts/" in test-runner/main.go,
and ran:
./test/test-all.sh out/build/x64-Debug/tint.exe --format hlsl --fxc --generate-skip
These will need to be manually removed once the skips aren't needed by
deleting them all, and re-running as above.
Bug: tint:940
Change-Id: I27e395e69f5e1bd7d234f3155f40396b751720d4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71981
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This new expectation file was added with a similar parent that changed the expected output.
Bug: tint:1112
Change-Id: Icb6175f68b6459385933d39e07460eb863ecb9ff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71101
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Now that I've landed this change to Dawn to disable FXC optimizations:
https://dawn-review.googlesource.com/c/dawn/+/70700,, we can reland this
change. The Tint-into-Dawn roll was failing because FXC would miscompile
certain loops into infinite loops when not unrolled.
Also reland
test/bug/fxc/gradient_in_varying_loop/1112.wgsl.expected.hlsl
Bug: tint:1112
Bug: dawn:1203
Change-Id: I641d68864b833e0fbe3b117d397b89ae96482536
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71000
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
If we encounter a global twice, the stack vector was not popped, which could trigger false cyclic-dependency errors. Because the cyclic dependency did not actually exist, later logic could break, expecting to find a global->global edge which did not exist.
Fixed: chromium:1273451
Change-Id: I9d99f1aeeaea042d9ed847a878c4717803122240
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70980
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
It's surprising that the fuzzers didn't find this, but I've confirmed that at least one of the fuzzers will fail with this case.
By adding a e2e test case, this will be used as a seed for the fuzzer corpus.
Bug: tint:1321
Change-Id: Ibf4bfa50bf376ae4cba401dfc31a2498fa3b9eec
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70982
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
RemovePhonies was transforming:
_ = f32(1)
_ = vec2<f32>(1.0, 2.0)
into:
f32(1)
vec2<f32>(1.0, 2.0)
Which the resolver gets grumpy about, as these are expressions, not statements.
Fixed: chromium:1273230
Change-Id: Ie85d3cee705fa3f792db686c021d76331e241f17
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70960
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Add transform::Unshadow to renamed shadowed symbols. Required by a
number of other transforms.
Replace Resolver symbol resolution with dep-graph.
The dependency graph now performs full symbol resolution before the
regular resolver pass.
Make use of this instead of duplicating the effort.
Simplfies code, and actually performs variable shadowing consistently.
Fixed: tint:819
Bug: tint:1266
Change-Id: I595d1812aebe1d79d2d32e724ff90de36e74cf4b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70523
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Looks like this should have been part of bc5d8f6b
Bug: tint:1112
Change-Id: I62e98bbb5f8ca29a171d5d5baa65af55b3a090d4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70664
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This reverts commit 11d09f2fe7.
Reason for revert: Failing roll of Tint to Dawn: https://dawn-review.googlesource.com/c/dawn/+/70100
Original change's description:
> HLSL: force FXC to never unroll loops
>
> Emit the "[loop]" attribute on "for" and "while" so that FXC does not
> attempt to unroll them. This is to work around an FXC bug where it fails
> to unroll loops with gradient operations.
>
> FXC ostensibly unrolls such loops because gradient operations require
> uniform control flow, and loops that have varying iterations may
> possibly not be uniform. Tint will eventually validate that control flow
> is indeed uniform, so forcing FXC to avoid unrolling in these cases
> should be fine.
>
> Bug: tint:1112
> Change-Id: I10077f8b62fbbb230a0003f3864c75a8fe0e1d18
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69880
> Kokoro: Kokoro <noreply+kokoro@google.com>
> Reviewed-by: Ben Clayton <bclayton@google.com>
> Commit-Queue: Antonio Maiorano <amaiorano@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: tint:1112
Change-Id: I8e8f3c0abfa6e6bc5d0e67af9428a46ef867d5c1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70540
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
No need for the HLSL-style repeated swizzle; GLSL allows construction
of a vector from a scalar value of the component type.
Bug: tint:1317
Change-Id: Ia0afe3012cbb56716a2d1c5c3849dd662a5ff89c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70342
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Try to lex identifiers before punctuation, and backtrack if we only
see a single underscore or something that starts with two underscores.
Rename the modf and frexp return types to start with two underscores.
Spec PR:
https://github.com/gpuweb/gpuweb/pull/2326
Change-Id: Id283af100babfe84faa183345cb8a60848140caa
Fixed: tint:1292
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70160
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Use imageSize() on images, not textureSize().
In GLSL, the LOD parameter to textureSize() is mandatory for
sampled textures, so emit a default 0 if not supplied. (Also, don't pack
the level into the coords argument; that's an HLSLism.)
GLSL returns the array size of array textures in the final component
of textureSize(); remove it for WGSL.
Write the subtype of storage images correctly (uimage*, iimage*, etc).
This required a bit of cleanup to move "writeonly" ahead of subtype
emission.
Bug: tint:1298
Change-Id: Ica1cec0f833a9b684143c8b0cf6d090fb511a7d2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70140
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>