[PATCH] D75192: [mlir][vulkan-runner] Update mlir-vulkan-runner execution driver.
River Riddle via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 10 13:39:53 PDT 2020
rriddle added inline comments.
================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertGPULaunchFuncToVulkanLaunchFunc.cpp:36
+
+// A pass to convert gpu launch op to vulkan launch call op, by creating a
+// SPIR-V binary shader from `spirv::ModuleOp` using `spirv::serialize`
----------------
Use /// for top level comments.
================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertGPULaunchFuncToVulkanLaunchFunc.cpp:119
+ SmallVector<uint32_t, 0> binary;
+ for (auto spirvModule : module.getOps<spirv::ModuleOp>()) {
+ if (done)
----------------
Why not check the range for `empty() || hasSingleElement()` before doing anything? If there can only be one, we don't need a loop.
================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:131
+ // 4. Stride in dim 0.
+ return {operands[numConfigOps + index * loweredMemRefNumOps1D],
+ operands[numConfigOps + index * loweredMemRefNumOps1D + 3]};
----------------
This breaks on GCC5:
https://buildkite.com/mlir/mlir-core/builds/3336#3ba0b41b-c8d6-4ce6-9788-dc12c51a4024
================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:161
+ LLVM::CallOp vulkanLaunchCallOp, Value vulkanRuntime) {
+ if (vulkanLaunchCallOp.getNumOperands() == 6)
+ return;
----------------
Can we have a comment? Why 6?
================
Comment at: mlir/tools/mlir-vulkan-runner/VulkanRuntime.h:25
-#include <vulkan/vulkan.h>
+#include <vulkan/vulkan.h> // NOLINT
----------------
Let's remove this.
================
Comment at: mlir/tools/mlir-vulkan-runner/vulkan-runtime-wrappers.cpp:66
extern "C" {
-/// Fills the given memref with the given value.
+// Initializes `VulkanRuntimeManager` and returns a pointer to it.
+void *initVulkan() { return new VulkanRuntimeManager(); }
----------------
Use /// for top-level comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75192/new/
https://reviews.llvm.org/D75192
More information about the llvm-commits
mailing list