Skip to content

[MLIR][MemRefToLLVM] Remove last typed pointer remnants #71113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Dinistro
Copy link
Contributor

@Dinistro Dinistro commented Nov 2, 2023

This commit removes the last typed pointer remnants from the MemRef to LLVM conversions, including the transform dialect operation. Typed pointers have been deprecated for a while now and it's planned to soon remove them from the LLVM dialect.

Related PSA: https://discourse.llvm.org/t/psa-removal-of-typed-pointers-from-the-llvm-dialect/74502

This commit removes the last typed pointer remnants from the MemRef to
LLVM conversions, including the transform dialect operation. Typed
pointers have been deprecated for a while now and it's planned to soon
remove them from the LLVM dialect.

Related PSA: https://discourse.llvm.org/t/psa-removal-of-typed-pointers-from-the-llvm-dialect/74502
@llvmbot
Copy link
Member

llvmbot commented Nov 2, 2023

@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-memref

Author: Christian Ulmann (Dinistro)

Changes

This commit removes the last typed pointer remnants from the MemRef to LLVM conversions, including the transform dialect operation. Typed pointers have been deprecated for a while now and it's planned to soon remove them from the LLVM dialect.

Related PSA: https://discourse.llvm.org/t/psa-removal-of-typed-pointers-from-the-llvm-dialect/74502


Full diff: https://github.com/llvm/llvm-project/pull/71113.diff

4 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/FunctionCallUtils.h (+5-5)
  • (modified) mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td (-3)
  • (modified) mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp (+4-12)
  • (modified) mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp (-1)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/FunctionCallUtils.h b/mlir/include/mlir/Dialect/LLVMIR/FunctionCallUtils.h
index 7289c3ac6ff7e1e..8da609755f6cae6 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/FunctionCallUtils.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/FunctionCallUtils.h
@@ -50,16 +50,16 @@ LLVM::LLVMFuncOp lookupOrCreatePrintCloseFn(ModuleOp moduleOp);
 LLVM::LLVMFuncOp lookupOrCreatePrintCommaFn(ModuleOp moduleOp);
 LLVM::LLVMFuncOp lookupOrCreatePrintNewlineFn(ModuleOp moduleOp);
 LLVM::LLVMFuncOp lookupOrCreateMallocFn(ModuleOp moduleOp, Type indexType,
-                                        bool opaquePointers);
+                                        bool opaquePointers = true);
 LLVM::LLVMFuncOp lookupOrCreateAlignedAllocFn(ModuleOp moduleOp, Type indexType,
                                               bool opaquePointers = true);
 LLVM::LLVMFuncOp lookupOrCreateFreeFn(ModuleOp moduleOp,
                                       bool opaquePointers = true);
 LLVM::LLVMFuncOp lookupOrCreateGenericAllocFn(ModuleOp moduleOp, Type indexType,
-                                              bool opaquePointers);
-LLVM::LLVMFuncOp lookupOrCreateGenericAlignedAllocFn(ModuleOp moduleOp,
-                                                     Type indexType,
-                                                     bool opaquePointers);
+                                              bool opaquePointers = true);
+LLVM::LLVMFuncOp
+lookupOrCreateGenericAlignedAllocFn(ModuleOp moduleOp, Type indexType,
+                                    bool opaquePointers = true);
 LLVM::LLVMFuncOp lookupOrCreateGenericFreeFn(ModuleOp moduleOp,
                                              bool opaquePointers = true);
 LLVM::LLVMFuncOp lookupOrCreateMemRefCopyFn(ModuleOp moduleOp, Type indexType,
diff --git a/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td b/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td
index d7bd8410e360a76..76309b9b8a9640d 100644
--- a/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td
+++ b/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td
@@ -30,8 +30,6 @@ def MemrefToLLVMTypeConverterOp : Op<Transform_Dialect,
       machine word.
     - `use_generic_functions`: Use generic allocation and deallocation functions
       instead of the classic "malloc", "aligned_alloc" and "free" functions.
-    - `use_opaque_pointers`: Generate LLVM IR using opaque pointers instead of
-      typed pointers.
     // TODO: the following two options don't really make sense for 
     // memref_to_llvm_type_converter specifically.
     // We should have a single to_llvm_type_converter.
@@ -45,7 +43,6 @@ def MemrefToLLVMTypeConverterOp : Op<Transform_Dialect,
       DefaultValuedOptionalAttr<BoolAttr, "false">:$use_aligned_alloc,
       DefaultValuedOptionalAttr<I64Attr, "64">:$index_bitwidth,
       DefaultValuedOptionalAttr<BoolAttr, "false">:$use_generic_functions,
-      DefaultValuedOptionalAttr<BoolAttr, "false">:$use_opaque_pointers,
       DefaultValuedOptionalAttr<BoolAttr, "false">:$use_bare_ptr_call_conv,
       OptionalAttr<StrAttr>:$data_layout);
   let assemblyFormat = "attr-dict";
diff --git a/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp b/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
index 6286b32b3704279..7e3fb9e95bc2cd9 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
@@ -23,11 +23,9 @@ LLVM::LLVMFuncOp getNotalignedAllocFn(const LLVMTypeConverter *typeConverter,
   bool useGenericFn = typeConverter->getOptions().useGenericFunctions;
 
   if (useGenericFn)
-    return LLVM::lookupOrCreateGenericAllocFn(
-        module, indexType, typeConverter->useOpaquePointers());
+    return LLVM::lookupOrCreateGenericAllocFn(module, indexType);
 
-  return LLVM::lookupOrCreateMallocFn(module, indexType,
-                                      typeConverter->useOpaquePointers());
+  return LLVM::lookupOrCreateMallocFn(module, indexType);
 }
 
 LLVM::LLVMFuncOp getAlignedAllocFn(const LLVMTypeConverter *typeConverter,
@@ -35,11 +33,9 @@ LLVM::LLVMFuncOp getAlignedAllocFn(const LLVMTypeConverter *typeConverter,
   bool useGenericFn = typeConverter->getOptions().useGenericFunctions;
 
   if (useGenericFn)
-    return LLVM::lookupOrCreateGenericAlignedAllocFn(
-        module, indexType, typeConverter->useOpaquePointers());
+    return LLVM::lookupOrCreateGenericAlignedAllocFn(module, indexType);
 
-  return LLVM::lookupOrCreateAlignedAllocFn(module, indexType,
-                                            typeConverter->useOpaquePointers());
+  return LLVM::lookupOrCreateAlignedAllocFn(module, indexType);
 }
 
 } // end namespace
@@ -70,10 +66,6 @@ static Value castAllocFuncResult(ConversionPatternRewriter &rewriter,
         typeConverter.getPointerType(allocatedPtrTy.getElementType(),
                                      memrefAddrSpace),
         allocatedPtr);
-
-  if (!typeConverter.useOpaquePointers())
-    allocatedPtr =
-        rewriter.create<LLVM::BitcastOp>(loc, elementPtrType, allocatedPtr);
   return allocatedPtr;
 }
 
diff --git a/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp b/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
index eed29efcaaada88..8932d6164182772 100644
--- a/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
+++ b/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
@@ -42,7 +42,6 @@ transform::MemrefToLLVMTypeConverterOp::getTypeConverter() {
       (getUseAlignedAlloc() ? LowerToLLVMOptions::AllocLowering::AlignedAlloc
                             : LowerToLLVMOptions::AllocLowering::Malloc);
   options.useGenericFunctions = getUseGenericFunctions();
-  options.useOpaquePointers = getUseOpaquePointers();
 
   if (getIndexBitwidth() != kDeriveIndexBitwidthFromDataLayout)
     options.overrideIndexBitwidth(getIndexBitwidth());

@@ -50,16 +50,16 @@ LLVM::LLVMFuncOp lookupOrCreatePrintCloseFn(ModuleOp moduleOp);
LLVM::LLVMFuncOp lookupOrCreatePrintCommaFn(ModuleOp moduleOp);
LLVM::LLVMFuncOp lookupOrCreatePrintNewlineFn(ModuleOp moduleOp);
LLVM::LLVMFuncOp lookupOrCreateMallocFn(ModuleOp moduleOp, Type indexType,
bool opaquePointers);
bool opaquePointers = true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not drop this argument entirely?

Copy link
Contributor Author

@Dinistro Dinistro Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a followup that removes all of these in one go, but I cannot stack PRs that use branches of my fork. The functions in FunctionCallUtils.h are also used in the LLVM utils helpers from LLVMCommon.

Once this and #71075 have been landed, I'll open the other PR that removes all of this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just hoped these were truly the last remnants :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are only the last remnants in the MemRefToLLVM lowering :)

@Dinistro Dinistro merged commit da5b382 into llvm:main Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants