Implement a pass to convert gpu.launch_func op into a sequence of
Vulkan runtime calls. The Vulkan runtime API surface is huge so currently we
don't expose separate external functions in IR for each of them, instead we
expose a few external functions to wrapper libraries which manages
Vulkan runtime.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nice, thanks Denis! Reviews already happened in D72696 so I just have two more nits here.
mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp | ||
---|---|---|
104 | Move this into initializeCachedTypes or put all of them into the constructor of GpuLaunchFuncToVulkanCalssPass? | |
158 | Could you document why we are putting a mysterious \0 in the middle here? |
Addressed review comments.
mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp | ||
---|---|---|
158 | I was looking how llvm.mlir globals lowering to llvm ir, and as far as I undertood in the current approach the AddNull is set to false if (op.getValueOrNull()) { // String attributes are treated separately because they cannot appear as // in-function constants and are thus not supported by getLLVMConstant. if (auto strAttr = op.getValueOrNull().dyn_cast_or_null<StringAttr>()) { cst = llvm::ConstantDataArray::getString( llvmModule->getContext(), strAttr.getValue(), /*AddNull=*/false); type = cst->getType(); } else { cst = getLLVMConstant(type, op.getValueOrNull(), op.getLoc()); } } else if (Block *initializer = op.getInitialize |
mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp | ||
---|---|---|
158 | Yeah I was just saying that we should document along the code. :) |
mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp | ||
---|---|---|
83 | SmallVectorImpl | |
156 | SmallString ? That would remove the explicit StringRef conversion below. | |
162 | Seems easier to just write (name + "_spv_entry_point_name").str() | |
175 | Return the error directly it auto converts to failure. | |
179 | Drop all of the trivial braces. | |
197 | Same here. | |
210 | /// |
SmallVectorImpl