[Mlir-commits] [mlir] [MLIR][GPU-LLVM] Add in-pass signature update option for opencl kernels (PR #105664)

[Petr Kurapov]

================
@@ -306,6 +310,36 @@ struct GPUShuffleConversion final : ConvertOpToLLVMPattern<gpu::ShuffleOp> {
   }
 };
 
+class MemorySpaceToOpenCLMemorySpaceConverter final : public TypeConverter {
+public:
+  MemorySpaceToOpenCLMemorySpaceConverter() {
+    addConversion([](Type t) { return t; });
+    addConversion([this](BaseMemRefType memRefType) -> std::optional<Type> {
+      // Attach global addr space attribute to memrefs with no addr space attr
+      Attribute memSpaceAttr = memRefType.getMemorySpace();
+      if (memSpaceAttr)
+        return std::nullopt;
+
+      auto addrSpaceAttr = gpu::AddressSpaceAttr::get(
+          memRefType.getContext(), gpu::AddressSpace::Global);
+      if (auto rankedType = dyn_cast<MemRefType>(memRefType)) {
+        return MemRefType::get(memRefType.getShape(),
+                               memRefType.getElementType(),
+                               rankedType.getLayout(), addrSpaceAttr);
+      }
+      return UnrankedMemRefType::get(memRefType.getElementType(),
+                                     addrSpaceAttr);
+    });
+    addConversion([this](FunctionType type) {
+      auto inputs = llvm::map_to_vector(
+          type.getInputs(), [this](Type ty) { return convertType(ty); });
+      auto results = llvm::map_to_vector(
+          type.getResults(), [this](Type ty) { return convertType(ty); });
+      return FunctionType::get(type.getContext(), inputs, results);
+    });
+  }
+};
----------------
kurapov-peter wrote:

Yeah, this does not work as mentioned in https://github.com/llvm/llvm-project/pull/105664#discussion_r1754224480. So first, you don't get the last conversion to ever be triggered since the underlying pass dials with the signature separately. The way to deal with that in this approach would be, for example, to make `convertFunctionSignature` protected in `LLVMTypeConverter` and override it here. When I tried that it dragged the implementation out making the converter interface wider for no good reason.

Second, the conversion in the middle would need to be rewritten to produce a legal output. A memref isn't legal since this now inherits from `LLVMTypeConverter`. That would duplicate memref lowering in the converter.

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