[Mlir-commits] [mlir] Fix crash when using when using --finalize-memref-to-llvm (PR #112433)
Siddhesh Deodhar
llvmlistbot at llvm.org
Thu Nov 7 21:14:06 PST 2024
https://github.com/siddhesh195 updated https://github.com/llvm/llvm-project/pull/112433
>From b8780fbccd3b39a0018e8c17330175730c0fc11d Mon Sep 17 00:00:00 2001
From: Siddhesh Deodhar <siddheshdeodhar95 at gmail.com>
Date: Tue, 15 Oct 2024 16:03:47 -0400
Subject: [PATCH 1/5] Fix crash when using when using --finalize-memref-to-llvm
Fix crash when attempting to convert uint to int address space during finalize-memref-to-llvm
---
mlir/lib/Conversion/LLVMCommon/Pattern.cpp | 8 +++++++-
mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp | 6 ++++--
mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir | 10 ++++++++++
3 files changed, 21 insertions(+), 3 deletions(-)
create mode 100644 mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir
diff --git a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
index d551506485a454..d502b4f49b00b9 100644
--- a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
@@ -106,8 +106,14 @@ bool ConvertToLLVMPattern::isConvertibleAndHasIdentityMaps(
Type ConvertToLLVMPattern::getElementPtrType(MemRefType type) const {
auto addressSpace = getTypeConverter()->getMemRefAddressSpace(type);
- if (failed(addressSpace))
+ if (failed(addressSpace)) {
+ emitError(UnknownLoc::get(type.getContext()),
+ "conversion of memref memory space ")
+ << type.getMemorySpace()
+ << " to integer address space "
+ "failed. Consider adding memory space conversions.";
return {};
+ }
return LLVM::LLVMPointerType::get(type.getContext(), *addressSpace);
}
diff --git a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
index 5a92fa839e9847..8693a60ad6e1b0 100644
--- a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
@@ -527,8 +527,10 @@ LLVMTypeConverter::getMemRefAddressSpace(BaseMemRefType type) const {
return failure();
if (!(*converted)) // Conversion to default is 0.
return 0;
- if (auto explicitSpace = llvm::dyn_cast_if_present<IntegerAttr>(*converted))
- return explicitSpace.getInt();
+ if (auto explicitSpace = llvm::dyn_cast_if_present<IntegerAttr>(*converted)) {
+ if (explicitSpace.getType().isSignedInteger())
+ return explicitSpace.getInt();
+ }
return failure();
}
diff --git a/mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir b/mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir
new file mode 100644
index 00000000000000..adad1cb0922337
--- /dev/null
+++ b/mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir
@@ -0,0 +1,10 @@
+// RUN: mlir-opt %s -finalize-memref-to-llvm 2>&1 | FileCheck %s
+// Since the error is at an unknown location, we use FileCheck instead of
+// -veri-y-diagnostics here
+
+// CHECK: conversion of memref memory space 1 : ui64 to integer address space failed. Consider adding memory space conversions.
+// CHECK-LABEL: @invalid_int_conversion
+func.func @invalid_int_conversion() {
+ %alloc_0 = memref.alloc() {alignment = 64 : i64} : memref<10xf32, 1 : ui64>
+ return
+}
>From b3eba807493df21497cd574e5af7d1eff36ce09f Mon Sep 17 00:00:00 2001
From: Siddhesh Deodhar <siddheshdeodhar95 at gmail.com>
Date: Tue, 15 Oct 2024 20:06:14 -0400
Subject: [PATCH 2/5] Resolve test failure due to incorrect usage of getInt()
Resolve test failure by checking if an IntegerAttr is int
---
mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
index 8693a60ad6e1b0..b81497dbbd5298 100644
--- a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
@@ -528,7 +528,8 @@ LLVMTypeConverter::getMemRefAddressSpace(BaseMemRefType type) const {
if (!(*converted)) // Conversion to default is 0.
return 0;
if (auto explicitSpace = llvm::dyn_cast_if_present<IntegerAttr>(*converted)) {
- if (explicitSpace.getType().isSignedInteger())
+ if (explicitSpace.getType().isIndex() ||
+ explicitSpace.getType().isSignlessInteger())
return explicitSpace.getInt();
}
return failure();
>From 709f0ce43633f07c951123942f10eed935e6617d Mon Sep 17 00:00:00 2001
From: Siddhesh Deodhar <153800103+siddhesh195 at users.noreply.github.com>
Date: Fri, 18 Oct 2024 09:46:01 -0400
Subject: [PATCH 3/5] Update mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
Co-authored-by: Christian Ulmann <christianulmann at gmail.com>
---
mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
index b81497dbbd5298..78f1960fc8569b 100644
--- a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
@@ -527,7 +527,7 @@ LLVMTypeConverter::getMemRefAddressSpace(BaseMemRefType type) const {
return failure();
if (!(*converted)) // Conversion to default is 0.
return 0;
- if (auto explicitSpace = llvm::dyn_cast_if_present<IntegerAttr>(*converted)) {
+ if (auto explicitSpace = dyn_cast_if_present<IntegerAttr>(*converted)) {
if (explicitSpace.getType().isIndex() ||
explicitSpace.getType().isSignlessInteger())
return explicitSpace.getInt();
>From 419342b23d1a6016b4c8fe593bcb70c581f77401 Mon Sep 17 00:00:00 2001
From: Siddhesh Deodhar <siddheshdeodhar95 at gmail.com>
Date: Fri, 18 Oct 2024 09:47:59 -0400
Subject: [PATCH 4/5] Emit error message on the operation
Emit type conversion error message on the operation
---
mlir/lib/Conversion/LLVMCommon/Pattern.cpp | 8 +-------
mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp | 6 ++++++
mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir | 8 +++-----
3 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
index d502b4f49b00b9..d551506485a454 100644
--- a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
@@ -106,14 +106,8 @@ bool ConvertToLLVMPattern::isConvertibleAndHasIdentityMaps(
Type ConvertToLLVMPattern::getElementPtrType(MemRefType type) const {
auto addressSpace = getTypeConverter()->getMemRefAddressSpace(type);
- if (failed(addressSpace)) {
- emitError(UnknownLoc::get(type.getContext()),
- "conversion of memref memory space ")
- << type.getMemorySpace()
- << " to integer address space "
- "failed. Consider adding memory space conversions.";
+ if (failed(addressSpace))
return {};
- }
return LLVM::LLVMPointerType::get(type.getContext(), *addressSpace);
}
diff --git a/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp b/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
index e48ca5180b706f..b9cd1dda55cbaf 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
@@ -74,6 +74,12 @@ std::tuple<Value, Value> AllocationOpLLVMLowering::allocateBufferManuallyAlign(
MemRefType memRefType = getMemRefResultType(op);
// Allocate the underlying buffer.
Type elementPtrType = this->getElementPtrType(memRefType);
+ if (!elementPtrType) {
+ emitError(loc,"conversion of memref memory space ")
+ << memRefType.getMemorySpace()
+ << " to integer address space "
+ "failed. Consider adding memory space conversions.";
+ }
LLVM::LLVMFuncOp allocFuncOp = getNotalignedAllocFn(
getTypeConverter(), op->getParentWithTrait<OpTrait::SymbolTable>(),
getIndexType());
diff --git a/mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir b/mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir
index adad1cb0922337..7e94677ebbdd7e 100644
--- a/mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir
+++ b/mlir/test/Conversion/MemRefToLLVM/invalid-uint.mlir
@@ -1,10 +1,8 @@
-// RUN: mlir-opt %s -finalize-memref-to-llvm 2>&1 | FileCheck %s
-// Since the error is at an unknown location, we use FileCheck instead of
-// -veri-y-diagnostics here
+// RUN: mlir-opt %s -finalize-memref-to-llvm -verify-diagnostics
-// CHECK: conversion of memref memory space 1 : ui64 to integer address space failed. Consider adding memory space conversions.
// CHECK-LABEL: @invalid_int_conversion
func.func @invalid_int_conversion() {
- %alloc_0 = memref.alloc() {alignment = 64 : i64} : memref<10xf32, 1 : ui64>
+ // expected-error at +1 {{conversion of memref memory space 1 : ui64 to integer address space failed. Consider adding memory space conversions.}}
+ %alloc = memref.alloc() {alignment = 64 : i64} : memref<10xf32, 1 : ui64>
return
}
>From b039d7acde7b230ffbaa6437bf0e09c1a89e9e53 Mon Sep 17 00:00:00 2001
From: Siddhesh Deodhar <siddheshdeodhar95 at gmail.com>
Date: Tue, 29 Oct 2024 22:38:57 -0400
Subject: [PATCH 5/5] Fix clang-format error
---
mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp b/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
index b9cd1dda55cbaf..a6408391b1330c 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/AllocLikeConversion.cpp
@@ -75,7 +75,7 @@ std::tuple<Value, Value> AllocationOpLLVMLowering::allocateBufferManuallyAlign(
// Allocate the underlying buffer.
Type elementPtrType = this->getElementPtrType(memRefType);
if (!elementPtrType) {
- emitError(loc,"conversion of memref memory space ")
+ emitError(loc, "conversion of memref memory space ")
<< memRefType.getMemorySpace()
<< " to integer address space "
"failed. Consider adding memory space conversions.";
More information about the Mlir-commits
mailing list