[Mlir-commits] [mlir] [mlir][gpu][nvptx] Remove null terminator when outputting PTX (PR #133019)

[llvmlistbot at llvm.org]

modiking wrote:

> I dug a bit more to see how things are fitting.
> 
> The `moduleToObject` interface is returning a "bag of bytes" (as materialized by the use of `SmallVector<char, 0>` and not any `*String*` class), as such there shouldn't be an expectation of null termination in the output of `moduleToObject`.
> 
> When the output is embedded in the IR, we will always add a null terminator since we store it as a `StringAttr`, but with the correct size (that is not including the null terminator): https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Support/StorageUniquer.h#L112-L114
> 
> However we currently have also a null terminator within the string:
> 
> ```
>   gpu.binary @kernel_module1  [#gpu.object<#nvvm.target<chip = "sm_70">, properties = {O = 2 : i32}, assembly = "//\0A// Generated by NVIDIA NVVM Compiler\0A//\0A// Compiler Build ID: UNKNOWN\0A// UNKNOWN\0A// Based on LLVM 20.0.0git\0A//\0A\0A.version 6.0\0A.target sm_70\0A.address_size 64\0A\0A\09// .globl\09kernel\0A\0A.visible .func kernel()\0A{\0A\0A\0A\09ret;\0A\0A}\0A\00">]
> ```
> 
> See the `\00` at the end of the string.
> 
> So this change LGTM.

Thanks for taking a look!

I'm double checking that the integration tests in JIT mode pass with the change and I'm noticing that:
```
/data/mmo/llvm-project/mlir/test/Integration/Dialect/SparseTensor/GPU/CUDA$ /data/mmo/llvm-project/build-jit-mlir/bin/mlir-runner    --shared-libs=/data/mmo/llvm-project/build-jit-mlir/lib/libmlir_cuda_runtime.so    --shared-libs=/data/mmo/llvm-project/build-jit-mlir/lib/libmlir_c_runner_utils.so    --e main --entry-point-result
=void out.ll --dump-object-file
JIT compilation failed with: 'ptxas application ptx input, line 147; fatal   : Parsing error near 'kernel0': syntax error
ptxas fatal   : Ptx assembly aborted due to errors'
```

Checking out the dumped object file I'm seeing that when the assembly is embedded into the JIT program it doesn't automatically get a null character at the end:
```
/data/mmo/llvm-project/mlir/test/Integration/Dialect/SparseTensor/GPU/CUDA$ readelf -x .lrodata out.ll.o | tai
l
  0x00000f50 09257264 36382c20 25726436 382c2031 .%rd68, %rd68, 1
  0x00000f60 363b0a09 6164642e 73363420 09257264 6;..add.s64 .%rd
  0x00000f70 36372c20 25726436 372c2031 363b0a09 67, %rd67, 16;..
  0x00000f80 73657470 2e6c742e 73363420 09257036 setp.lt.s64 .%p6
  0x00000f90 2c202572 6436392c 20257264 31333b0a , %rd69, %rd13;.
  0x00000fa0 09402570 36206272 61200924 4c5f5f42 .@%p6 bra .$L__B
  0x00000fb0 42305f37 3b0a0962 72612e75 6e692009 B0_7;..bra.uni .
  0x00000fc0 244c5f5f 4242305f 383b0a24 4c5f5f42 $L__BB0_8;.$L__B
  0x00000fd0 42305f39 3a0a0972 65743b0a 0a7d0a   B0_9:..ret;..}.
 ```
 So when it gets embedded in the binary we don't get a null character. Fortunately when this gets embedded as a `llvm::ConstantDataArray` we can specify whether we want it null-terminated or not so I made a change there. I *think* it's not any more invasive than currently always embedding it but take a look and LMK.

https://github.com/llvm/llvm-project/pull/133019