[Mlir-commits] [mlir] 4614889 - [mlir][GPUToLLVM] Fix regression introduced with opaque-pointers when generate GPU launch func parameters

[Markus Böck]

Author: Markus Böck
Date: 2023-03-10T11:05:18+01:00
New Revision: 4614889a936fe7a69a4760fd765c374002c207c3

URL: https://github.com/llvm/llvm-project/commit/4614889a936fe7a69a4760fd765c374002c207c3
DIFF: https://github.com/llvm/llvm-project/commit/4614889a936fe7a69a4760fd765c374002c207c3.diff

LOG: [mlir][GPUToLLVM] Fix regression introduced with opaque-pointers when generate GPU launch func parameters

This has caused build failures when enabling opaque pointers for the GPU integration tests as could be seen here:
https://lab.llvm.org/buildbot/#/builders/220/builds/16946 and here https://lab.llvm.org/buildbot/#/builders/61/builds/40822

The gist of the issue was the use of a wrong pointer base type within a GEP. There sadly was no test coverage for either the generating of that GEP, nor is LLVM Dialects GEP verifier currently capable of catching such issues, so it went unnoticed until the integration tests actually attempted to convert it to LLVM IR.

Differential Revision: https://reviews.llvm.org/D145774

Added: 
    

Modified: 
    mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
    mlir/test/Conversion/GPUCommon/lower-launch-func-to-gpu-runtime-calls.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
index ec85b99df10ff..55a5e46839558 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
@@ -680,8 +680,7 @@ Value ConvertLaunchFuncOpToGpuRuntimeCallPattern::generateParamsArray(
   for (const auto &en : llvm::enumerate(arguments)) {
     Value fieldPtr = builder.create<LLVM::GEPOp>(
         loc, getTypeConverter()->getPointerType(argumentTypes[en.index()]),
-        argumentTypes[en.index()], structPtr,
-        ArrayRef<LLVM::GEPArg>{0, en.index()});
+        structType, structPtr, ArrayRef<LLVM::GEPArg>{0, en.index()});
     builder.create<LLVM::StoreOp>(loc, en.value(), fieldPtr);
     auto elementPtr = builder.create<LLVM::GEPOp>(
         loc, llvmPointerPointerType, llvmPointerType, arrayPtr,

diff  --git a/mlir/test/Conversion/GPUCommon/lower-launch-func-to-gpu-runtime-calls.mlir b/mlir/test/Conversion/GPUCommon/lower-launch-func-to-gpu-runtime-calls.mlir
index 1db1dc8c04b00..0bc1450edc73c 100644
--- a/mlir/test/Conversion/GPUCommon/lower-launch-func-to-gpu-runtime-calls.mlir
+++ b/mlir/test/Conversion/GPUCommon/lower-launch-func-to-gpu-runtime-calls.mlir
@@ -40,9 +40,18 @@ module attributes {gpu.container_module} {
 
   // CHECK: [[STREAM:%.*]] = llvm.call @mgpuStreamCreate
 
+  // CHECK: %[[ONE:.*]] = llvm.mlir.constant(1 : i32)
+  // CHECK: %[[MEMREF:.*]] = llvm.alloca %[[ONE]] x !llvm.struct[[STRUCT_BODY:<.*>]]
   // CHECK: [[NUM_PARAMS:%.*]] = llvm.mlir.constant(6 : i32) : i32
   // CHECK-NEXT: [[PARAMS:%.*]] = llvm.alloca [[NUM_PARAMS]] x !llvm.ptr
 
+  // CHECK: llvm.getelementptr %[[MEMREF]][0, 0] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct[[STRUCT_BODY:<.*>]]
+  // CHECK: llvm.getelementptr %[[MEMREF]][0, 1] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct[[STRUCT_BODY:<.*>]]
+  // CHECK: llvm.getelementptr %[[MEMREF]][0, 2] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct[[STRUCT_BODY:<.*>]]
+  // CHECK: llvm.getelementptr %[[MEMREF]][0, 3] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct[[STRUCT_BODY:<.*>]]
+  // CHECK: llvm.getelementptr %[[MEMREF]][0, 4] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct[[STRUCT_BODY:<.*>]]
+  // CHECK: llvm.getelementptr %[[MEMREF]][0, 5] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct[[STRUCT_BODY:<.*>]]
+
   // CHECK: [[EXTRA_PARAMS:%.*]] = llvm.mlir.null : !llvm.ptr
 
   // CHECK: llvm.call @mgpuLaunchKernel([[FUNC]], [[C8]], [[C8]], [[C8]],