[PATCH] D74549: [mlir][spirv] Add pass ConvertGpuLaunchFuncToVulkanCallsPass.

[Lei Zhang via Phabricator via llvm-commits]

antiagainst added inline comments.


================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:157
+  std::vector<char> shaderName(name.begin(), name.end());
+  shaderName.push_back('\0');
+
----------------
denis13 wrote:
> antiagainst wrote:
> > Could you document why we are putting a mysterious `\0` in the middle here?
> 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
> https://llvm.org/doxygen/classllvm_1_1ConstantDataArray.html#a3edef3fa47c611d3d10606591213e57b
> , so we need to explicitly append `\0` to align with C style string. Please fix me I'm wrong.
> 
> ```
>  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
> ```
Yeah I was just saying that we should document along the code. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74549/new/

https://reviews.llvm.org/D74549