[Mlir-commits] [mlir] [MLIR][LLVM] Improve atomic verifier to properly support larger types (PR #92120)
Christian Ulmann
llvmlistbot at llvm.org
Tue May 14 08:17:22 PDT 2024
https://github.com/Dinistro updated https://github.com/llvm/llvm-project/pull/92120
>From 158c0219cf9dfa37fd821071a98dda1dad0a7314 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Tue, 14 May 2024 13:55:04 +0000
Subject: [PATCH 1/2] [MLIR][LLVM] Improve atomic verifier to properly support
larger types
This commit extends the verifier for atomics to properly verify larger
types. Beforehand, the verifier strictly rejected larger integer types,
while it now consults the data layout to determine if their bitsize is a
power of two. This behavior reflects what LLVM's verifier is checking
for.
---
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 39 ++++++++++------------
mlir/test/Dialect/LLVMIR/invalid.mlir | 14 ++++++++
mlir/test/Dialect/LLVMIR/roundtrip.mlir | 6 +++-
3 files changed, 36 insertions(+), 23 deletions(-)
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 7d33d05feb650..23db9f4b52049 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -839,25 +839,19 @@ void LoadOp::getEffects(
}
/// Returns true if the given type is supported by atomic operations. All
-/// integer and float types with limited bit width are supported. Additionally,
-/// depending on the operation pointers may be supported as well.
-static bool isTypeCompatibleWithAtomicOp(Type type, bool isPointerTypeAllowed) {
- if (llvm::isa<LLVMPointerType>(type))
- return isPointerTypeAllowed;
-
- std::optional<unsigned> bitWidth;
- if (auto floatType = llvm::dyn_cast<FloatType>(type)) {
- if (!isCompatibleFloatingPointType(type))
+/// integer, float, and pointer types with a power-of-two bitsize and a minimal
+/// size of 8 bits are supported.
+static bool isTypeCompatibleWithAtomicOp(Type type,
+ const DataLayout &dataLayout) {
+ if (!isa<IntegerType, LLVMPointerType>(type))
+ if (!isa<FloatType>(type) || !isCompatibleFloatingPointType(type))
return false;
- bitWidth = floatType.getWidth();
- }
- if (auto integerType = llvm::dyn_cast<IntegerType>(type))
- bitWidth = integerType.getWidth();
- // The type is neither an integer, float, or pointer type.
- if (!bitWidth)
+
+ llvm::TypeSize bitWidth = dataLayout.getTypeSizeInBits(type);
+ if (bitWidth.isScalable())
return false;
- return *bitWidth == 8 || *bitWidth == 16 || *bitWidth == 32 ||
- *bitWidth == 64;
+ // Needs to be at least 8 bits and a power of two.
+ return bitWidth >= 8 && (bitWidth & (bitWidth - 1)) == 0;
}
/// Verifies the attributes and the type of atomic memory access operations.
@@ -865,8 +859,8 @@ template <typename OpTy>
LogicalResult verifyAtomicMemOp(OpTy memOp, Type valueType,
ArrayRef<AtomicOrdering> unsupportedOrderings) {
if (memOp.getOrdering() != AtomicOrdering::not_atomic) {
- if (!isTypeCompatibleWithAtomicOp(valueType,
- /*isPointerTypeAllowed=*/true))
+ DataLayout dataLayout = DataLayout::closest(memOp);
+ if (!isTypeCompatibleWithAtomicOp(valueType, dataLayout))
return memOp.emitOpError("unsupported type ")
<< valueType << " for atomic access";
if (llvm::is_contained(unsupportedOrderings, memOp.getOrdering()))
@@ -2694,7 +2688,8 @@ LogicalResult AtomicRMWOp::verify() {
if (!mlir::LLVM::isCompatibleFloatingPointType(valType))
return emitOpError("expected LLVM IR floating point type");
} else if (getBinOp() == AtomicBinOp::xchg) {
- if (!isTypeCompatibleWithAtomicOp(valType, /*isPointerTypeAllowed=*/true))
+ DataLayout dataLayout = DataLayout::closest(*this);
+ if (!isTypeCompatibleWithAtomicOp(valType, dataLayout))
return emitOpError("unexpected LLVM IR type for 'xchg' bin_op");
} else {
auto intType = llvm::dyn_cast<IntegerType>(valType);
@@ -2741,8 +2736,8 @@ LogicalResult AtomicCmpXchgOp::verify() {
if (!ptrType)
return emitOpError("expected LLVM IR pointer type for operand #0");
auto valType = getVal().getType();
- if (!isTypeCompatibleWithAtomicOp(valType,
- /*isPointerTypeAllowed=*/true))
+ DataLayout dataLayout = DataLayout::closest(*this);
+ if (!isTypeCompatibleWithAtomicOp(valType, dataLayout))
return emitOpError("unexpected LLVM IR type");
if (getSuccessOrdering() < AtomicOrdering::monotonic ||
getFailureOrdering() < AtomicOrdering::monotonic)
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index 0914f00232102..a1d3409109484 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -160,6 +160,13 @@ func.func @load_unsupported_type(%ptr : !llvm.ptr) {
// -----
+func.func @load_unsupported_type(%ptr : !llvm.ptr) {
+ // expected-error at below {{unsupported type 'i33' for atomic access}}
+ %1 = llvm.load %ptr atomic monotonic {alignment = 16 : i64} : !llvm.ptr -> i33
+}
+
+// -----
+
func.func @load_unaligned_atomic(%ptr : !llvm.ptr) {
// expected-error at below {{expected alignment for atomic access}}
%1 = llvm.load %ptr atomic monotonic : !llvm.ptr -> f32
@@ -195,6 +202,13 @@ func.func @store_unsupported_type(%val : i1, %ptr : !llvm.ptr) {
// -----
+func.func @store_unsupported_type(%val : i48, %ptr : !llvm.ptr) {
+ // expected-error at below {{unsupported type 'i48' for atomic access}}
+ llvm.store %val, %ptr atomic monotonic {alignment = 16 : i64} : i48, !llvm.ptr
+}
+
+// -----
+
func.func @store_unaligned_atomic(%val : f32, %ptr : !llvm.ptr) {
// expected-error at below {{expected alignment for atomic access}}
llvm.store %val, %ptr atomic monotonic : f32, !llvm.ptr
diff --git a/mlir/test/Dialect/LLVMIR/roundtrip.mlir b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
index 410122df1c14d..2386dde19301e 100644
--- a/mlir/test/Dialect/LLVMIR/roundtrip.mlir
+++ b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
@@ -385,15 +385,19 @@ func.func @atomic_load(%ptr : !llvm.ptr) {
%0 = llvm.load %ptr atomic monotonic {alignment = 4 : i64} : !llvm.ptr -> f32
// CHECK: llvm.load volatile %{{.*}} atomic syncscope("singlethread") monotonic {alignment = 16 : i64} : !llvm.ptr -> f32
%1 = llvm.load volatile %ptr atomic syncscope("singlethread") monotonic {alignment = 16 : i64} : !llvm.ptr -> f32
+ // CHECK: llvm.load %{{.*}} atomic monotonic {alignment = 4 : i64} : !llvm.ptr -> i128
+ %2 = llvm.load %ptr atomic monotonic {alignment = 4 : i64} : !llvm.ptr -> i128
llvm.return
}
// CHECK-LABEL: @atomic_store
-func.func @atomic_store(%val : f32, %ptr : !llvm.ptr) {
+func.func @atomic_store(%val : f32, %large_val : i256, %ptr : !llvm.ptr) {
// CHECK: llvm.store %{{.*}}, %{{.*}} atomic monotonic {alignment = 4 : i64} : f32, !llvm.ptr
llvm.store %val, %ptr atomic monotonic {alignment = 4 : i64} : f32, !llvm.ptr
// CHECK: llvm.store volatile %{{.*}}, %{{.*}} atomic syncscope("singlethread") monotonic {alignment = 16 : i64} : f32, !llvm.ptr
llvm.store volatile %val, %ptr atomic syncscope("singlethread") monotonic {alignment = 16 : i64} : f32, !llvm.ptr
+ // CHECK: llvm.store %{{.*}}, %{{.*}} atomic monotonic {alignment = 4 : i64} : i256, !llvm.ptr
+ llvm.store %large_val, %ptr atomic monotonic {alignment = 4 : i64} : i256, !llvm.ptr
llvm.return
}
>From 6b8607d5669fa543dbcfffeb83b9bfd3be1f34a1 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Tue, 14 May 2024 15:17:07 +0000
Subject: [PATCH 2/2] address review comments
---
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 23db9f4b52049..dcf3f3b52a606 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -844,7 +844,7 @@ void LoadOp::getEffects(
static bool isTypeCompatibleWithAtomicOp(Type type,
const DataLayout &dataLayout) {
if (!isa<IntegerType, LLVMPointerType>(type))
- if (!isa<FloatType>(type) || !isCompatibleFloatingPointType(type))
+ if (!isCompatibleFloatingPointType(type))
return false;
llvm::TypeSize bitWidth = dataLayout.getTypeSizeInBits(type);
More information about the Mlir-commits
mailing list