Skip to content

[MLIR] Replace getVoidPtrType with getPtrType in ConvertToLLVMPattern #145657

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
merged 1 commit into from
Jun 27, 2025

Conversation

PragmaTwice
Copy link
Contributor

@PragmaTwice PragmaTwice commented Jun 25, 2025

ConversionPattern::getVoidPtrType looks a little confusion since the opaque pointer migration is already done. Also we cannot specify address space in this method.

Maybe we can mark them as deprecated and add new method getPtrType(), as this PR did : )

@PragmaTwice PragmaTwice changed the title [MLIR] Replace get(Void/Int)PtrType with getPtrType in ConvertToLLVMPattern [MLIR] Replace getVoidPtrType with getPtrType in ConvertToLLVMPattern Jun 25, 2025
@PragmaTwice PragmaTwice force-pushed the mlir-llvm-ptr branch 3 times, most recently from e6b3e18 to 49f30a3 Compare June 25, 2025 10:40
@PragmaTwice PragmaTwice marked this pull request as ready for review June 25, 2025 11:21
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-mlir-llvm

Author: Twice (PragmaTwice)

Changes

ConversionPattern::getVoidPtrType looks a little confusion since the opaque pointer migration is already done. Also we cannot specify address space in this method.

Maybe we can mark them as deprecated and add new method getPtrType(), as this PR did : )


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

4 Files Affected:

  • (modified) mlir/include/mlir/Conversion/LLVMCommon/Pattern.h (+4)
  • (modified) mlir/lib/Conversion/LLVMCommon/Pattern.cpp (+6-3)
  • (modified) mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp (+4-7)
  • (modified) mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp (+1-1)
diff --git a/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h b/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h
index 503a2a7e6f0cd..969154abe8830 100644
--- a/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h
+++ b/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h
@@ -113,8 +113,12 @@ class ConvertToLLVMPattern : public ConversionPattern {
   Type getVoidType() const;
 
   /// Get the MLIR type wrapping the LLVM i8* type.
+  [[deprecated("Use getPtrType() instead!")]]
   Type getVoidPtrType() const;
 
+  /// Get the MLIR type wrapping the LLVM ptr type.
+  Type getPtrType(unsigned addressSpace = 0) const;
+
   /// Create a constant Op producing a value of `resultType` from an index-typed
   /// integer attribute.
   static Value createIndexAttrConstant(OpBuilder &builder, Location loc,
diff --git a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
index 86fb9166b7223..c5f72f7e10b8c 100644
--- a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
@@ -46,10 +46,13 @@ Type ConvertToLLVMPattern::getVoidType() const {
   return LLVM::LLVMVoidType::get(&getTypeConverter()->getContext());
 }
 
-Type ConvertToLLVMPattern::getVoidPtrType() const {
-  return LLVM::LLVMPointerType::get(&getTypeConverter()->getContext());
+Type ConvertToLLVMPattern::getPtrType(unsigned addressSpace) const {
+  return LLVM::LLVMPointerType::get(&getTypeConverter()->getContext(),
+                                    addressSpace);
 }
 
+Type ConvertToLLVMPattern::getVoidPtrType() const { return getPtrType(); }
+
 Value ConvertToLLVMPattern::createIndexAttrConstant(OpBuilder &builder,
                                                     Location loc,
                                                     Type resultType,
@@ -273,7 +276,7 @@ LogicalResult ConvertToLLVMPattern::copyUnrankedDescriptors(
             ? builder
                   .create<LLVM::CallOp>(loc, mallocFunc.value(), allocationSize)
                   .getResult()
-            : builder.create<LLVM::AllocaOp>(loc, getVoidPtrType(),
+            : builder.create<LLVM::AllocaOp>(loc, getPtrType(),
                                              IntegerType::get(getContext(), 8),
                                              allocationSize,
                                              /*alignment=*/0);
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index 4b7b2cc224ce2..7484e4b07390e 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -408,8 +408,7 @@ struct AllocaScopeOpLowering
 
     // Save stack and then branch into the body of the region.
     rewriter.setInsertionPointToEnd(currentBlock);
-    auto stackSaveOp =
-        rewriter.create<LLVM::StackSaveOp>(loc, getVoidPtrType());
+    auto stackSaveOp = rewriter.create<LLVM::StackSaveOp>(loc, getPtrType());
     rewriter.create<LLVM::BrOp>(loc, ValueRange(), beforeBody);
 
     // Replace the alloca_scope return with a branch that jumps out of the body.
@@ -1122,8 +1121,7 @@ class MemRefCopyOpLowering : public ConvertOpToLLVMPattern<memref::CopyOp> {
     };
 
     // Save stack position before promoting descriptors
-    auto stackSaveOp =
-        rewriter.create<LLVM::StackSaveOp>(loc, getVoidPtrType());
+    auto stackSaveOp = rewriter.create<LLVM::StackSaveOp>(loc, getPtrType());
 
     auto srcMemRefType = dyn_cast<MemRefType>(srcType);
     Value unrankedSource =
@@ -1249,7 +1247,7 @@ struct MemorySpaceCastOpLowering
                                              result, resultAddrSpace, sizes);
       Value resultUnderlyingSize = sizes.front();
       Value resultUnderlyingDesc = rewriter.create<LLVM::AllocaOp>(
-          loc, getVoidPtrType(), rewriter.getI8Type(), resultUnderlyingSize);
+          loc, getPtrType(), rewriter.getI8Type(), resultUnderlyingSize);
       result.setMemRefDescPtr(rewriter, loc, resultUnderlyingDesc);
 
       // Copy pointers, performing address space casts.
@@ -1530,8 +1528,7 @@ struct MemRefReshapeOpLowering
     UnrankedMemRefDescriptor::computeSizes(rewriter, loc, *getTypeConverter(),
                                            targetDesc, addressSpace, sizes);
     Value underlyingDescPtr = rewriter.create<LLVM::AllocaOp>(
-        loc, getVoidPtrType(), IntegerType::get(getContext(), 8),
-        sizes.front());
+        loc, getPtrType(), IntegerType::get(getContext(), 8), sizes.front());
     targetDesc.setMemRefDescPtr(rewriter, loc, underlyingDescPtr);
 
     // Extract pointers and offset from the source memref.
diff --git a/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp b/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp
index f878a262512ee..3b29867cc3263 100644
--- a/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp
+++ b/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp
@@ -28,7 +28,7 @@ class TestTypeProducerOpConverter
   LogicalResult
   matchAndRewrite(test::TestTypeProducerOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
-    rewriter.replaceOpWithNewOp<LLVM::ZeroOp>(op, getVoidPtrType());
+    rewriter.replaceOpWithNewOp<LLVM::ZeroOp>(op, getPtrType());
     return success();
   }
 };

@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-mlir

Author: Twice (PragmaTwice)

Changes

ConversionPattern::getVoidPtrType looks a little confusion since the opaque pointer migration is already done. Also we cannot specify address space in this method.

Maybe we can mark them as deprecated and add new method getPtrType(), as this PR did : )


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

4 Files Affected:

  • (modified) mlir/include/mlir/Conversion/LLVMCommon/Pattern.h (+4)
  • (modified) mlir/lib/Conversion/LLVMCommon/Pattern.cpp (+6-3)
  • (modified) mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp (+4-7)
  • (modified) mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp (+1-1)
diff --git a/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h b/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h
index 503a2a7e6f0cd..969154abe8830 100644
--- a/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h
+++ b/mlir/include/mlir/Conversion/LLVMCommon/Pattern.h
@@ -113,8 +113,12 @@ class ConvertToLLVMPattern : public ConversionPattern {
   Type getVoidType() const;
 
   /// Get the MLIR type wrapping the LLVM i8* type.
+  [[deprecated("Use getPtrType() instead!")]]
   Type getVoidPtrType() const;
 
+  /// Get the MLIR type wrapping the LLVM ptr type.
+  Type getPtrType(unsigned addressSpace = 0) const;
+
   /// Create a constant Op producing a value of `resultType` from an index-typed
   /// integer attribute.
   static Value createIndexAttrConstant(OpBuilder &builder, Location loc,
diff --git a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
index 86fb9166b7223..c5f72f7e10b8c 100644
--- a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
@@ -46,10 +46,13 @@ Type ConvertToLLVMPattern::getVoidType() const {
   return LLVM::LLVMVoidType::get(&getTypeConverter()->getContext());
 }
 
-Type ConvertToLLVMPattern::getVoidPtrType() const {
-  return LLVM::LLVMPointerType::get(&getTypeConverter()->getContext());
+Type ConvertToLLVMPattern::getPtrType(unsigned addressSpace) const {
+  return LLVM::LLVMPointerType::get(&getTypeConverter()->getContext(),
+                                    addressSpace);
 }
 
+Type ConvertToLLVMPattern::getVoidPtrType() const { return getPtrType(); }
+
 Value ConvertToLLVMPattern::createIndexAttrConstant(OpBuilder &builder,
                                                     Location loc,
                                                     Type resultType,
@@ -273,7 +276,7 @@ LogicalResult ConvertToLLVMPattern::copyUnrankedDescriptors(
             ? builder
                   .create<LLVM::CallOp>(loc, mallocFunc.value(), allocationSize)
                   .getResult()
-            : builder.create<LLVM::AllocaOp>(loc, getVoidPtrType(),
+            : builder.create<LLVM::AllocaOp>(loc, getPtrType(),
                                              IntegerType::get(getContext(), 8),
                                              allocationSize,
                                              /*alignment=*/0);
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index 4b7b2cc224ce2..7484e4b07390e 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -408,8 +408,7 @@ struct AllocaScopeOpLowering
 
     // Save stack and then branch into the body of the region.
     rewriter.setInsertionPointToEnd(currentBlock);
-    auto stackSaveOp =
-        rewriter.create<LLVM::StackSaveOp>(loc, getVoidPtrType());
+    auto stackSaveOp = rewriter.create<LLVM::StackSaveOp>(loc, getPtrType());
     rewriter.create<LLVM::BrOp>(loc, ValueRange(), beforeBody);
 
     // Replace the alloca_scope return with a branch that jumps out of the body.
@@ -1122,8 +1121,7 @@ class MemRefCopyOpLowering : public ConvertOpToLLVMPattern<memref::CopyOp> {
     };
 
     // Save stack position before promoting descriptors
-    auto stackSaveOp =
-        rewriter.create<LLVM::StackSaveOp>(loc, getVoidPtrType());
+    auto stackSaveOp = rewriter.create<LLVM::StackSaveOp>(loc, getPtrType());
 
     auto srcMemRefType = dyn_cast<MemRefType>(srcType);
     Value unrankedSource =
@@ -1249,7 +1247,7 @@ struct MemorySpaceCastOpLowering
                                              result, resultAddrSpace, sizes);
       Value resultUnderlyingSize = sizes.front();
       Value resultUnderlyingDesc = rewriter.create<LLVM::AllocaOp>(
-          loc, getVoidPtrType(), rewriter.getI8Type(), resultUnderlyingSize);
+          loc, getPtrType(), rewriter.getI8Type(), resultUnderlyingSize);
       result.setMemRefDescPtr(rewriter, loc, resultUnderlyingDesc);
 
       // Copy pointers, performing address space casts.
@@ -1530,8 +1528,7 @@ struct MemRefReshapeOpLowering
     UnrankedMemRefDescriptor::computeSizes(rewriter, loc, *getTypeConverter(),
                                            targetDesc, addressSpace, sizes);
     Value underlyingDescPtr = rewriter.create<LLVM::AllocaOp>(
-        loc, getVoidPtrType(), IntegerType::get(getContext(), 8),
-        sizes.front());
+        loc, getPtrType(), IntegerType::get(getContext(), 8), sizes.front());
     targetDesc.setMemRefDescPtr(rewriter, loc, underlyingDescPtr);
 
     // Extract pointers and offset from the source memref.
diff --git a/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp b/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp
index f878a262512ee..3b29867cc3263 100644
--- a/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp
+++ b/mlir/test/lib/Conversion/FuncToLLVM/TestConvertCallOp.cpp
@@ -28,7 +28,7 @@ class TestTypeProducerOpConverter
   LogicalResult
   matchAndRewrite(test::TestTypeProducerOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
-    rewriter.replaceOpWithNewOp<LLVM::ZeroOp>(op, getVoidPtrType());
+    rewriter.replaceOpWithNewOp<LLVM::ZeroOp>(op, getPtrType());
     return success();
   }
 };

@ftynse ftynse merged commit c3e08c9 into llvm:main Jun 27, 2025
11 checks passed
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
…llvm#145657)

`ConversionPattern::getVoidPtrType` looks a little confusion since the
opaque pointer migration is already done. Also we cannot specify address
space in this method.

Maybe we can mark them as deprecated and add new method `getPtrType()`,
as this PR did : )
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.

3 participants