[MLIR] Lowering from GPU to LLVM fails when the same kernel is called multiple times

[Alon_Lau]

Summary

See details about the Bug: https://github.com/llvm/llvm-project/issues/59503
I try to fix it, what do you think about this bugfix?
I don't have commit access.

Closes #59503.

[VitalyR]

It seems that the build environment doesn't have llvm-readelf or it's not in the PATH when testing`flang`:

Script:
--
: 'RUN: at line 5';   rm -f /var/lib/buildkite-agent/builds/llvm-project/build/tools/flang/test/Driver/Output/code-gen-rv64.f90.tmp.o
: 'RUN: at line 6';   /var/lib/buildkite-agent/builds/llvm-project/build/bin/flang-new -fc1 -triple riscv64-unknown-linux-gnu    -target-feature +d -target-feature +c -emit-obj /var/lib/buildkite-agent/builds/llvm-project/flang/test/Driver/code-gen-rv64.f90 -o /var/lib/buildkite-agent/builds/llvm-project/build/tools/flang/test/Driver/Output/code-gen-rv64.f90.tmp.o
: 'RUN: at line 8';   llvm-readelf -h /var/lib/buildkite-agent/builds/llvm-project/build/tools/flang/test/Driver/Output/code-gen-rv64.f90.tmp.o | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/flang/test/Driver/code-gen-rv64.f90
: 'RUN: at line 10';   rm -f /var/lib/buildkite-agent/builds/llvm-project/build/tools/flang/test/Driver/Output/code-gen-rv64.f90.tmp.o
: 'RUN: at line 11';   /var/lib/buildkite-agent/builds/llvm-project/build/bin/flang-new --target=riscv64-unknown-linux-gnu -c /var/lib/buildkite-agent/builds/llvm-project/flang/test/Driver/code-gen-rv64.f90 -o /var/lib/buildkite-agent/builds/llvm-project/build/tools/flang/test/Driver/Output/code-gen-rv64.f90.tmp.o
: 'RUN: at line 12';   llvm-readelf -h /var/lib/buildkite-agent/builds/llvm-project/build/tools/flang/test/Driver/Output/code-gen-rv64.f90.tmp.o | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/flang/test/Driver/code-gen-rv64.f90
--
Exit Code: 2
 
Command Output (stderr):
--
/var/lib/buildkite-agent/builds/llvm-project/build/tools/flang/test/Driver/Output/code-gen-rv64.f90.script: line 3: llvm-readelf: command not found
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/flang/test/Driver/code-gen-rv64.f90

[VitalyR]

flang tests dependencies don't include llvm-readelf: https://github.com/llvm/llvm-project/blob/43eb96cab8f734a740b86c553f48f96fa9d19edc/flang/test/CMakeLists.txt#L50.

[VitalyR]

The llvm-readelf in flang/test/Driver/code-gen-rv64.f90 should be changed to llvm-readobj, according to this commit: https://github.com/llvm/llvm-project/commit/0b958fe41182c9040499e629e24cbdd250d73bf7.

cc @sunshaoce @DavidSpickett

[ftynse]

Please add a test. The IR in the bug is a good start for the test, but can be simplified further.

[Alon_Lau]

add fir testcase

[Alon_Lau]

In D146272#4201645, @ftynse wrote:

Please add a test. The IR in the bug is a good start for the test, but can be simplified further.

The IR test has been added in this patch, I think this is the simplest testcase now.

[ftynse]

mlir/test/Conversion/GPUCommon/kernel-call-multi-times.fir

2 ↗

(On Diff #506478)

There is no reason why this file should be suffixed with ".fir". Please rename.

29 ↗

(On Diff #506478)

Nit: Please add a newline.

[Alon_Lau]

update fix-patch

[Alon_Lau]

In D146272#4201645, @ftynse wrote:

Please add a test. The IR in the bug is a good start for the test, but can be simplified further.

The gpu testcase is based on the pass-pipeline of cubin，the current compiler is not supported.

[ftynse]

The gpu testcase is based on the pass-pipeline of cubin，the current compiler is not supported.

I'm sorry, I don't quite follow this. We have the -test-gpu-to-cubin pass specifically for testing the lowering without having the target-specific compiler. It should be possible to run that, followed by gpu-to-llvm lowering and drop everything else. The gpu-to-llvm lowering would create the single constant, we can actually check that the single constant was created. I don't mind the integration test, but it should be an addition to a unit test.

[Alon_Lau]

update testcase by using test-gpu-to-cubin.

[Alon_Lau]

In D146272#4209314, @ftynse wrote:

The gpu testcase is based on the pass-pipeline of cubin，the current compiler is not supported.

I'm sorry, I don't quite follow this. We have the -test-gpu-to-cubin pass specifically for testing the lowering without having the target-specific compiler. It should be possible to run that, followed by gpu-to-llvm lowering and drop everything else. The gpu-to-llvm lowering would create the single constant, we can actually check that the single constant was created. I don't mind the integration test, but it should be an addition to a unit test.

I have upated the testcase by using '--test-gpu-to-cubin', but the other two testcases failed. I don't have the flangFeatureList.so. I can't fix the error, and analyze it.

[ftynse]

This looks like an unrelated flang failure

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

   OpBuilder moduleBuilder(module.getBodyRegion(), builder.getListener());
   MLIRContext *ctx = builder.getContext();
   auto type = LLVM::LLVMArrayType::get(IntegerType::get(ctx, 8), value.size());
-  auto global = moduleBuilder.create<LLVM::GlobalOp>(
-      loc, type, /*isConstant=*/true, linkage, name,
-      builder.getStringAttr(value), /*alignment=*/0);
+  auto symbol = module.lookupSymbol<LLVM::GlobalOp>(name.str());
+  auto global = symbol ? symbol
+                       : moduleBuilder.create<LLVM::GlobalOp>(
+                             loc, type, /*isConstant=*/true, linkage, name,
+                             builder.getStringAttr(value), /*alignment=*/0);
 
   LLVMPointerType resultType;
   LLVMPointerType charPtr;

mlir/test/Integration/GPU/CUDA/kernel-call-multi-times.mlir

+// RUN: mlir-opt %s \
+// RUN: -pass-pipeline='builtin.module(gpu.module(strip-debuginfo,convert-gpu-to-nvvm,test-gpu-to-cubin))' \
+// RUN: | mlir-opt -gpu-to-llvm \
+// RUN: | FileCheck %s
+
+// CHECK: llvm.mlir.global internal constant @main_kernel_empty_kernel_kernel_name
+// CHECK: llvm.mlir.global internal constant @main_kernel_gpubin_cst("CUBIN")
+module attributes {gpu.container_module} {
+  // CHECK-LABEL: llvm.func @main
+  func.func @main() {
+    %one = arith.constant 1 : index
+    %sx = arith.constant 12 : index
+    // CHECK: llvm.mlir.addressof @main_kernel_gpubin_cst
+    // CHECK: llvm.mlir.addressof @main_kernel_empty_kernel_kernel_name
+    gpu.launch_func @main_kernel::@empty_kernel blocks in (%one, %one, %one) threads in (%sx, %one, %one) args()
+    %token_1 = gpu.wait async
+
+    // CHECK: llvm.mlir.addressof @main_kernel_gpubin_cst
+    // CHECK: llvm.mlir.addressof @main_kernel_empty_kernel_kernel_name
+    gpu.launch_func @main_kernel::@empty_kernel blocks in (%one, %one, %one) threads in (%sx, %one, %one) args()
+    %token_2 = gpu.wait async
+
+    return
+  }
+
+ gpu.module @main_kernel attributes {gpu.binary = "CUBIN"}{
+    gpu.func @empty_kernel() kernel {
+      gpu.return
+    }
+  }
+}