This reverts commit 56f1678437.
Reason for revert: crbug.com/1059205
Bug: chromium:1059205
Original change's description:
> Vulkan: Enforce fixed subgroup size for compute shaders.
>
> This CL ensures that, on architectures with a varying subgroup size,
> compute shaders are always compiled with a fixed subgroup size to
> avoid consistency issues when one shader writes data in a subgroup-size
> dependent layout to GPU memory, to be read by another shader in a
> future dispatch.
>
> At the moment, only Intel ICDs are known to implement this [1],
> and the code uses a heuristics to chose the size of 16, which seems to
> be the sweet spot according to Intel engineers.
>
> + Update the PNextChainBuilder class to deal with the fact that
> VkComputePipelineCreateInfo::pNext is defined as a const void*,
> which created compiler errors in the previous implementation.
>
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=108875
>
> Change-Id: I332faa53b9f854a8abe43a7271f30d8c5deb2142
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16021
> Commit-Queue: Corentin Wallez <cwallez@chromium.org>
> Reviewed-by: Corentin Wallez <cwallez@chromium.org>
TBR=cwallez@google.com,cwallez@chromium.org,enga@chromium.org,enga@google.com,david.turner.dev@gmail.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I922eccc310505da4b4a9fc853335733ca4900fc8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16521
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This matches Chromium's logic and make it less likely to break developer
workflow in the subsequent roll of Chromium's buildtools.
Bug: dawn:339
Change-Id: Ic42553827be125985a3d16b4f5d003b6662ead39
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16520
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This CL ensures that, on architectures with a varying subgroup size,
compute shaders are always compiled with a fixed subgroup size to
avoid consistency issues when one shader writes data in a subgroup-size
dependent layout to GPU memory, to be read by another shader in a
future dispatch.
At the moment, only Intel ICDs are known to implement this [1],
and the code uses a heuristics to chose the size of 16, which seems to
be the sweet spot according to Intel engineers.
+ Update the PNextChainBuilder class to deal with the fact that
VkComputePipelineCreateInfo::pNext is defined as a const void*,
which created compiler errors in the previous implementation.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=108875
Change-Id: I332faa53b9f854a8abe43a7271f30d8c5deb2142
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16021
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Certain Vulkan ICDs (Intel ones notably) will compile SPIR-V
shaders with an liberal, compiler-selected, subgroup size (i.e.
either 8, 16 or 32). For more context, see [1].
This can be a problem for compute, when one shader stores data
in device memory using a subgroup-size dependent layout, to be
consumed by a another shader. Problems arise when the compiler
decides to compile both shaders with different subgroup sizes.
To work-around this, the VK_EXT_subgroup_size_control device
extension was introduced recently: it allows the device to
report the min/max subgroup sizes it provides, and allows
the Vulkan program to control the subgroup size precisely
if it wants to.
This patch adds support to the Vulkan backend to report and
enable the extension if it is available. Note that:
- The corresponding VkStructureType enum values and
struct types are not rolled to the third-party Vulkan
headers used by Dawn yet, so vulkan_platform.h has been
modified to define them if necessary. This can be
removed in the future when the Vulkan-Headers are
updated in a different patch.
- This modifies VulkanDeviceInfo::GatherDeviceInfo() to
use VkGetPhysicalDevice{Properties2,Features2} if the
VK_KHR_get_device_properties2 instance extension is
available. Otherwise, the Vulkan 1.0 APIs
VkGetPhysicalDevice{Properties,Features} are used instead
(and it is assumed that no subgroup size control is
possible).
- This changes the definition of VulkanDeviceKnobs to
make room for the required pNext-linked chains of
extensions.
- A helper class, PNextChainBuilder is also provided in
UtilsVulkan.h to make it easy to build pNext-linked
extension struct chains at runtime, as required when
probing device propertires/features, or when
creating a new VkDevice handle.
Apart from that, there is no change in behaviour in this CL.
I.e. a later CL might force a specific subgroup size for
consistency, or introduce a new API to let Dawn clients
select a fixed subgroup size.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=108875
Change-Id: I524af6ff3479f25b0a8bb139a062fe632c826893
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16020
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
A Chromium's LinkedList class to Dawn. Implementation and header are
a direct copy/paste. This is to be used to implement an LRU Cache
for the ResidencyManager class.
Bug: dawn:193
Change-Id: I7cb02649590be4db0fe54c9d80557ac49efc34de
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16380
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Instead of tracking RTV/DSVs before the start of the pass to allocate
memory for CPU descriptors, allocate them at the start of the pass,
removing the need to loop through the entire command buffer each
Submit().
BUG=dawn:256
Change-Id: I72faff8951095c6a45207bfe5b12936715c58abf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16261
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Some dawn_unittests crash on some configurations because the
uninitialized |label| member crashed string serialization.
Default initialize all descriptors to avoid this problem.
Bug: none
Change-Id: I6ea1851ebb6f54690a28ba396e0beaa85d8670cc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16260
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
This is so we can run the end2end_tests using Swiftshader. We still
prefer the discrete, then integrated GPUs so that normal testing uses
the real GPU.
Bug: dawn:283
Change-Id: I17a1ffd8aa88ddeaafa019feb67deeb25cdd2da0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16220
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Mark Henderson <mehe@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Instead of counting descriptors to be allocated for the entire command
buffer in a pre-pass, the bindgroup state tracker is used to allocate
only dirty bindgroups upon recording draw/dispatch. If the heap has no
more room and must be changed, bindgroups will be re-created according
to the BGL.
A future change will address the CPU descriptors and removal of the
pre-pass.
BUG=dawn:256,dawn:307
Change-Id: I6603de17cfda713bd4512c46e1c93618ca01bb7b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/13400
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
These are not supported on some older OpenGL, OpenGL ES, and iOS
devices.
Bug: dawn:343
Change-Id: I70def749ae57fcfe2895f8556674dd241941d3d3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16163
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This Toggle is set if MTLFeatureSet_iOS_GPUFamily3_v1 is not supported.
Bug: dawn:342
Change-Id: Ia5f43e87fdd2c13eaffe9557cb0ce9a06dec3b29
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16180
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
BUG=dawn:344
Change-Id: Ifa9e1e3167ecfe7d38c16f393cec0443ea1589f2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16164
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
If these extern variables are initialized after DAWN_INSTANTIATE_TEST,
they will be zero. Change them to be function calls instead.
Since they're function calls, fold in arguments from ForceToggles to
enable/disable toggles.
Bug: dawn:341
Change-Id: I1aeaa1e535a0a003977e8ce7ab3d5278c5d81281
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16162
Reviewed-by: Mark Henderson <mehe@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Roll third_party/SPIRV-Tools/ 4a80497a8..8910ea5f1 (4 commits)
4a80497a88..8910ea5f1c
$ git log 4a80497a8..8910ea5f1 --date=short --no-merges --format='%ad %ae %s'
2020-02-23 nicolasweber Fix Wrange-loop-analysis warnings in SPIRV-Tools. (#3201)
2020-02-21 geofflang Add missing dependencies when generating spvtools_core_tables (#3199)
2020-02-21 afdx Brief guide to writing a spirv-fuzz fuzzer pass (#3190)
2020-02-21 47594367+rg3igalia Fix ignored const qualifier warning in static_cast (#3197)
Roll third_party/glslang/ c12493ff6..07e1a0a67 (1 commit)
c12493ff69..07e1a0a67a
$ git log c12493ff6..07e1a0a67 --date=short --no-merges --format='%ad %ae %s'
2020-02-22 rex.xu Fix an issue of SPV generation for imageAtomicStore.
Roll third_party/shaderc/ 738f1655a..1059f43a3 (1 commit)
$ git log 738f1655a..1059f43a3 --date=short --no-merges --format='%ad %ae %s'
2020-02-21 rharrison Rolling 6 dependencies and update expectations (#987)
Roll third_party/spirv-cross/ f19fdb94d..c5f7b5575 (5 commits)
f19fdb94d7..c5f7b55756
$ git log f19fdb94d..c5f7b5575 --date=short --no-merges --format='%ad %ae %s'
2020-02-24 post MSL: Add C API for force native arrays.
2020-02-24 post MSL: Add native array test for composite array initialization.
2020-02-24 post MSL: Reintroduce workaround for constant arrays being passed by value.
2020-02-24 post MSL: Reinstate workaround for returning arrays.
2020-02-24 post MSL: Add a workaround path to force native arrays for everything.
Created with:
roll-dep third_party/SPIRV-Tools third_party/glslang third_party/shaderc third_party/spirv-cross third_party/spirv-headers
Change-Id: Ic9f459c21a9656093b5ec27e19f0744182734c7c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16120
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Previously we would always assume that if the driver supported a Vulkan
version it would also support extensions that were promoted in that
version. This is not a spec requirement, so instead try to load the core
entrypoints, and only if the version is not available, load the
extension entrypoints.
Also renames VulkanFunction members that are from promoted extension to
not have a vendor prefix.
Also tag the promoted extensions that are the same in a core version as
available when that core version is available. This simplifies checking
for features in the Vulkan backend.
Bug:
Change-Id: I0817c01b8838ba26070858abb0cbed030e3291df
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16040
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: David Turner <digit@google.com>
This relands commit 0bbfec1f7f which
was reverted in 21e5074dcd.
The original CL broke the Chromium roll because drm/drm_fourcc.h
could not be found. Still not sure why this is the case since it
seems to be present on all of my CrOS test machines, but at the end
of the day, I realized that I don't even need this header in the
first place.
This CL removes the header and relands the rest of the original CL.
Bug: chromium:996470
Change-Id: I77d6b1692094b7798f3c5d9c2b50219e674c8a8c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16060
Commit-Queue: Brian Ho <hob@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This reverts commit 0bbfec1f7f.
Reason for revert: Makes the roll into Chromium fail with the following:
[314/21578] CXX obj/third_party/dawn/dawn_white_box_tests_sources/VulkanImageWrappingTestsDmaBuf.o
FAILED: obj/third_party/dawn/dawn_white_box_tests_sources/VulkanImageWrappingTestsDmaBuf.o
../../build/toolchain/clang_code_coverage_wrapper.py --target-os=chromeos --files-to-instrument=../....(too long)
../../third_party/dawn/src/tests/white_box/VulkanImageWrappingTestsDmaBuf.cpp:28:10: fatal error: 'drm/drm_fourcc.h' file not found
#include <drm/drm_fourcc.h>
^~~~~~~~~~~~~~~~~~
1 error generated.
Original change's description:
> Create VulkanImageWrappingTests for dma-buf images
>
> This CL branches the existing VulkanImageWrappingTests to separate
> tests for OpaqueFD-backed amd DmaBuf-backed external images. On
> Chrome OS of Dawn, we no longer interop using opaque FDs, so these
> tests were failing in the end2end test suite.
>
> The new VulkanImageWrappingTestsDmaBuf tests are essentially 1:1
> mappings of their counterparts in the Opaque FD version. The only
> difference is that we allocate memory directly on the device using
> GBM instead of creating a VkImage (which will likely call some GBM
> methods under the hood) and then extracting the FD using a Vulkan
> extension. We then communicate this to Dawn via the DmaBuf
> ExternalImageDescriptor.
>
> Also, this fixes VulkanImageWrappingUsageTests::LargeImage on AMD
> devices (assuming the extension is implemented) as we can now
> specify DRM modifiers.
>
> Bug: chromium:996470
> Change-Id: I2b3c57d7f5ff14131d415e99a09d32d2f16b3e54
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15800
> Commit-Queue: Brian Ho <hob@chromium.org>
> Reviewed-by: Austin Eng <enga@chromium.org>
TBR=cwallez@chromium.org,enga@chromium.org,hob@chromium.org
Change-Id: Idb45586c608ce20432142834a4f14d42c76d3b3b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:996470
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16001
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This CL branches the existing VulkanImageWrappingTests to separate
tests for OpaqueFD-backed amd DmaBuf-backed external images. On
Chrome OS of Dawn, we no longer interop using opaque FDs, so these
tests were failing in the end2end test suite.
The new VulkanImageWrappingTestsDmaBuf tests are essentially 1:1
mappings of their counterparts in the Opaque FD version. The only
difference is that we allocate memory directly on the device using
GBM instead of creating a VkImage (which will likely call some GBM
methods under the hood) and then extracting the FD using a Vulkan
extension. We then communicate this to Dawn via the DmaBuf
ExternalImageDescriptor.
Also, this fixes VulkanImageWrappingUsageTests::LargeImage on AMD
devices (assuming the extension is implemented) as we can now
specify DRM modifiers.
Bug: chromium:996470
Change-Id: I2b3c57d7f5ff14131d415e99a09d32d2f16b3e54
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15800
Commit-Queue: Brian Ho <hob@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Also fixes some warnings when compiling with GCC
Bug: dawn:333
Change-Id: Ib597bb3b950476279a1e20e3556765ec9f1db697
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15960
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Headers only INTERFACE library with generated headers don't work in CMake
because the GENERATED property is local to a directory. Instead we make a
STATIC library with a Dummy cpp file.
INTERFACE libraries can only have INTERFACE sources so the sources get added
to the dependant's list of sources. If these dependents are in another
directory, they don't see the GENERATED property and fail to configure
because the file doesn't exist on disk.
Use this trick for both dawn_headers and dawncpp_headers that are header
only libraries with generated headers.
Bug: dawn:333
Change-Id: Ib0d6dcc5f351a638d1c5360214c0ce14a28fee3e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15921
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
The symbol was not marked as static and would cause linking errors
because it would be defined in multiple translation units. Replace it
with a more traditional C-style #define.
Bug:
Change-Id: I19151884b7e8e171f829ffa47b1d119aff12ff99
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15740
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This also changes the name to be consistent with the option being set
in the spvc API.
BUG=dawn:335
Change-Id: I6f7431095493874e1fef0856e563f7f1225cfc21
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15780
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
There appears to be issues with the CTS + spvc that are preventing
chromium to roll, so I am reverting spvc by default, until the CTS
issues are resolved.
BUG=dawn:337
Change-Id: I171ee5325b9afbf5d240a469009433105caf6ddb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15840
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Adds CMake support for:
- Generating the Dawn headers and C++ wrappers
- libdawn_wire
- libdawn_native with the Metal backend for now
- All the examples.
Bug: dawn:333
Change-Id: I6ffbe090b0bd21d6a805c03a507ad51fda0275ca
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15720
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
This reverts commit 97c3be2699.
Reason for revert: Breaks linking on MSVC
Original change's description:
> Use libshaderc_spvc as a source_set, so complete_static_lib applies properly
>
> Brings the list of dependencies for a standalone Dawn app down from:
>
> obj/libdawn_native.a
> obj/libdawn_utils.a
> obj/src/dawn/libdawn_proc.a
> obj/third_party/shaderc/libshaderc_spvc/spvc.o
> obj/third_party/shaderc/libshaderc_spvc/spvc_private.o
> obj/third_party/shaderc/libshaderc_spvc/spvcir_pass.o
> obj/third_party/SPIRV-Tools/libspvtools_opt.a
>
> to
>
> obj/libdawn_native.a
> obj/src/dawn/libdawn_proc.a
>
> Bug: dawn:327
> Change-Id: I74654b304a9cb5f2aff19e72aa6a8bf1eb708c15
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15481
> Reviewed-by: Kai Ninomiya <kainino@chromium.org>
> Reviewed-by: Ryan Harrison <rharrison@chromium.org>
> Commit-Queue: Kai Ninomiya <kainino@chromium.org>
TBR=kainino@chromium.org,rharrison@chromium.org
Change-Id: Ib6d17eccebf371b71e74f1857b50bb3cfd67595a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: dawn:327
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15722
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Falling-back to direct allocation ensures allocation failure returns OOM.
If no OOM, the resource could be left then used while in an invalid state.
BUG=chromium:1045811,chromium:1047220,chromium:1047048
Change-Id: I927962b1dc6a7422a7d6eac114d82f28a42794a2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15600
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Brings the list of dependencies for a standalone Dawn app down from:
obj/libdawn_native.a
obj/libdawn_utils.a
obj/src/dawn/libdawn_proc.a
obj/third_party/shaderc/libshaderc_spvc/spvc.o
obj/third_party/shaderc/libshaderc_spvc/spvc_private.o
obj/third_party/shaderc/libshaderc_spvc/spvcir_pass.o
obj/third_party/SPIRV-Tools/libspvtools_opt.a
to
obj/libdawn_native.a
obj/src/dawn/libdawn_proc.a
Bug: dawn:327
Change-Id: I74654b304a9cb5f2aff19e72aa6a8bf1eb708c15
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15481
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
The SampleUtils uses CreateSwapChain with a nullptr surface. This is
currently valid with implementation-based swapchains so the argument
should be tagged as optional.
Bug: dawn:269
Change-Id: Ic00d5a67fb038d2771174bb36f99b66b84f1a252
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15680
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Overriding VK_DEFINE_NON_DISPATCHABLE_HANDLE changes the function
signatures of Vulkan functions, changing their ABI and making us
incompatible with real drivers. This removes that magic, and replaces it
with an explicit wrapper, VkHandle, which has much of the same
functionality as the original VkNonDispatchableHandle.
It adds definitions for dawn_native::vulkan::VkBuffer et al, which
shadow the native ::VkBuffer et al. This retains type safety throughout
the Vulkan backend without changing every single usage.
Notably, the following things had to change:
- An explicit conversion from VkBuffer* to ::VkBuffer* is needed for
arrays. This is implemented as a reinterpret_cast, which is still
safe as the new VkHandle still has the same memory layout properties
as VkNonDispatchableHandle did.
- When pointing to a VkHandle as an output pointer, it's now necessary
to explicitly get the native ::VkBuffer (via operator*) and point to
it.
Previously reviewed on:
https://dawn-review.googlesource.com/c/dawn/+/15580
Bug: chromium:1046362
Change-Id: I7d34ec38a805025f92165ea9a7ee07ae5c182076
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15641
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>