[mlir][ptr] Switch LogicalResult to bool in MemorySpaceAttrInterrface
#137513
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…face` This patch switches the return type in `MemorySpaceAttrInterface` methods from `LogicalResult` to `bool`. As `is*` methods are predicates. Users of the `MemorySpaceAttrInterface` API must note that, if `emitError` is non-null and the result of a `is*` method is `false`, then an error was likely emitted. To avoid the emission of an error the user can pass a default constructed `emitError`.
Member
|
@llvm/pr-subscribers-mlir Author: Fabian Mora (fabianmcg) ChangesThis patch switches the return type in Users of the Full diff: https://github.com/llvm/llvm-project/pull/137513.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Dialect/Ptr/IR/MemorySpaceInterfaces.td b/mlir/include/mlir/Dialect/Ptr/IR/MemorySpaceInterfaces.td
index 3e9f99e0e1d6a..54efeb0bc0f9e 100644
--- a/mlir/include/mlir/Dialect/Ptr/IR/MemorySpaceInterfaces.td
+++ b/mlir/include/mlir/Dialect/Ptr/IR/MemorySpaceInterfaces.td
@@ -35,8 +35,10 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
This method checks if it's valid to load a value from the memory space
with a specific type, alignment, and atomic ordering.
If `emitError` is non-null then the method is allowed to emit errors.
+ Furthermore, if `emitError` is non-null and the result is `false` an
+ error must have been emitted.
}],
- /*returnType=*/ "::mlir::LogicalResult",
+ /*returnType=*/ "bool",
/*methodName=*/ "isValidLoad",
/*args=*/ (ins "::mlir::Type":$type,
"::mlir::ptr::AtomicOrdering":$ordering,
@@ -48,8 +50,10 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
This method checks if it's valid to store a value in the memory space
with a specific type, alignment, and atomic ordering.
If `emitError` is non-null then the method is allowed to emit errors.
+ Furthermore, if `emitError` is non-null and the result is `false` an
+ error must have been emitted.
}],
- /*returnType=*/ "::mlir::LogicalResult",
+ /*returnType=*/ "bool",
/*methodName=*/ "isValidStore",
/*args=*/ (ins "::mlir::Type":$type,
"::mlir::ptr::AtomicOrdering":$ordering,
@@ -61,8 +65,10 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
This method checks if it's valid to perform an atomic operation in the
memory space with a specific type, alignment, and atomic ordering.
If `emitError` is non-null then the method is allowed to emit errors.
+ Furthermore, if `emitError` is non-null and the result is `false` an
+ error must have been emitted.
}],
- /*returnType=*/ "::mlir::LogicalResult",
+ /*returnType=*/ "bool",
/*methodName=*/ "isValidAtomicOp",
/*args=*/ (ins "::mlir::ptr::AtomicBinOp":$op,
"::mlir::Type":$type,
@@ -76,8 +82,10 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
in the memory space with a specific type, alignment, and atomic
orderings.
If `emitError` is non-null then the method is allowed to emit errors.
+ Furthermore, if `emitError` is non-null and the result is `false` an
+ error must have been emitted.
}],
- /*returnType=*/ "::mlir::LogicalResult",
+ /*returnType=*/ "bool",
/*methodName=*/ "isValidAtomicXchg",
/*args=*/ (ins "::mlir::Type":$type,
"::mlir::ptr::AtomicOrdering":$successOrdering,
@@ -90,8 +98,10 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
This method checks if it's valid to perform an `addrspacecast` op
in the memory space.
If `emitError` is non-null then the method is allowed to emit errors.
+ Furthermore, if `emitError` is non-null and the result is `false` an
+ error must have been emitted.
}],
- /*returnType=*/ "::mlir::LogicalResult",
+ /*returnType=*/ "bool",
/*methodName=*/ "isValidAddrSpaceCast",
/*args=*/ (ins "::mlir::Type":$tgt,
"::mlir::Type":$src,
@@ -101,11 +111,13 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
/*desc=*/ [{
This method checks if it's valid to perform a `ptrtoint` or `inttoptr`
op in the memory space.
- The first type is expected to be integer-like, while the second must be a
- ptr-like type.
+ The first type is expected to be integer-like, while the second must be
+ a ptr-like type.
If `emitError` is non-null then the method is allowed to emit errors.
+ Furthermore, if `emitError` is non-null and the result is `false` an
+ error must have been emitted.
}],
- /*returnType=*/ "::mlir::LogicalResult",
+ /*returnType=*/ "bool",
/*methodName=*/ "isValidPtrIntCast",
/*args=*/ (ins "::mlir::Type":$intLikeTy,
"::mlir::Type":$ptrLikeTy,
diff --git a/mlir/lib/Dialect/Ptr/IR/PtrAttrs.cpp b/mlir/lib/Dialect/Ptr/IR/PtrAttrs.cpp
index 1770e4febf099..b819f56841852 100644
--- a/mlir/lib/Dialect/Ptr/IR/PtrAttrs.cpp
+++ b/mlir/lib/Dialect/Ptr/IR/PtrAttrs.cpp
@@ -23,45 +23,45 @@ constexpr const static unsigned kBitsInByte = 8;
// GenericSpaceAttr
//===----------------------------------------------------------------------===//
-LogicalResult GenericSpaceAttr::isValidLoad(
+bool GenericSpaceAttr::isValidLoad(
Type type, ptr::AtomicOrdering ordering, IntegerAttr alignment,
function_ref<InFlightDiagnostic()> emitError) const {
- return success();
+ return true;
}
-LogicalResult GenericSpaceAttr::isValidStore(
+bool GenericSpaceAttr::isValidStore(
Type type, ptr::AtomicOrdering ordering, IntegerAttr alignment,
function_ref<InFlightDiagnostic()> emitError) const {
- return success();
+ return true;
}
-LogicalResult GenericSpaceAttr::isValidAtomicOp(
+bool GenericSpaceAttr::isValidAtomicOp(
ptr::AtomicBinOp op, Type type, ptr::AtomicOrdering ordering,
IntegerAttr alignment, function_ref<InFlightDiagnostic()> emitError) const {
- return success();
+ return true;
}
-LogicalResult GenericSpaceAttr::isValidAtomicXchg(
+bool GenericSpaceAttr::isValidAtomicXchg(
Type type, ptr::AtomicOrdering successOrdering,
ptr::AtomicOrdering failureOrdering, IntegerAttr alignment,
function_ref<InFlightDiagnostic()> emitError) const {
- return success();
+ return true;
}
-LogicalResult GenericSpaceAttr::isValidAddrSpaceCast(
+bool GenericSpaceAttr::isValidAddrSpaceCast(
Type tgt, Type src, function_ref<InFlightDiagnostic()> emitError) const {
// TODO: update this method once the `addrspace_cast` op is added to the
// dialect.
assert(false && "unimplemented, see TODO in the source.");
- return failure();
+ return false;
}
-LogicalResult GenericSpaceAttr::isValidPtrIntCast(
+bool GenericSpaceAttr::isValidPtrIntCast(
Type intLikeTy, Type ptrLikeTy,
function_ref<InFlightDiagnostic()> emitError) const {
// TODO: update this method once the int-cast ops are added to the dialect.
assert(false && "unimplemented, see TODO in the source.");
- return failure();
+ return false;
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/lib/Dialect/Test/TestAttributes.cpp b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
index 057d9fb4a215f..3524e85c2d234 100644
--- a/mlir/test/lib/Dialect/Test/TestAttributes.cpp
+++ b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
@@ -331,43 +331,44 @@ TestOpAsmAttrInterfaceAttr::getAlias(::llvm::raw_ostream &os) const {
// TestConstMemorySpaceAttr
//===----------------------------------------------------------------------===//
-LogicalResult TestConstMemorySpaceAttr::isValidLoad(
+bool TestConstMemorySpaceAttr::isValidLoad(
Type type, mlir::ptr::AtomicOrdering ordering, IntegerAttr alignment,
function_ref<InFlightDiagnostic()> emitError) const {
- return success();
+ return true;
}
-LogicalResult TestConstMemorySpaceAttr::isValidStore(
+bool TestConstMemorySpaceAttr::isValidStore(
Type type, mlir::ptr::AtomicOrdering ordering, IntegerAttr alignment,
function_ref<InFlightDiagnostic()> emitError) const {
- return emitError ? (emitError() << "memory space is read-only") : failure();
+ return emitError ? failed(emitError() << "memory space is read-only") : false;
}
-LogicalResult TestConstMemorySpaceAttr::isValidAtomicOp(
+bool TestConstMemorySpaceAttr::isValidAtomicOp(
mlir::ptr::AtomicBinOp binOp, Type type, mlir::ptr::AtomicOrdering ordering,
IntegerAttr alignment, function_ref<InFlightDiagnostic()> emitError) const {
- return emitError ? (emitError() << "memory space is read-only") : failure();
+ return emitError ? failed(emitError() << "memory space is read-only") : false;
}
-LogicalResult TestConstMemorySpaceAttr::isValidAtomicXchg(
+bool TestConstMemorySpaceAttr::isValidAtomicXchg(
Type type, mlir::ptr::AtomicOrdering successOrdering,
mlir::ptr::AtomicOrdering failureOrdering, IntegerAttr alignment,
function_ref<InFlightDiagnostic()> emitError) const {
- return emitError ? (emitError() << "memory space is read-only") : failure();
+ return emitError ? failed(emitError() << "memory space is read-only") : false;
}
-LogicalResult TestConstMemorySpaceAttr::isValidAddrSpaceCast(
+bool TestConstMemorySpaceAttr::isValidAddrSpaceCast(
Type tgt, Type src, function_ref<InFlightDiagnostic()> emitError) const {
- return emitError
- ? (emitError() << "memory space doesn't allow addrspace casts")
- : failure();
+ return emitError ? failed(emitError()
+ << "memory space doesn't allow addrspace casts")
+ : false;
}
-LogicalResult TestConstMemorySpaceAttr::isValidPtrIntCast(
+bool TestConstMemorySpaceAttr::isValidPtrIntCast(
Type intLikeTy, Type ptrLikeTy,
function_ref<InFlightDiagnostic()> emitError) const {
- return emitError ? (emitError() << "memory space doesn't allow int-ptr casts")
- : failure();
+ return emitError
+ ? failed(emitError() << "memory space doesn't allow int-ptr casts")
+ : false;
}
//===----------------------------------------------------------------------===//
|
|
|
joker-eph
reviewed
Apr 27, 2025
joker-eph
reviewed
Apr 27, 2025
joker-eph
reviewed
Apr 27, 2025
joker-eph
reviewed
Apr 27, 2025
joker-eph
reviewed
Apr 27, 2025
joker-eph
approved these changes
Apr 27, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/35/builds/9584 Here is the relevant piece of the build log for the reference |
Contributor
Author
|
Could this be product of a dirty cache? However, this patch changed both to |
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…rface` (llvm#137513) This patch switches the return type in `MemorySpaceAttrInterface` methods from `LogicalResult` to `bool`. As `is*` methods are predicates. Users of the `MemorySpaceAttrInterface` API must note that, if `emitError` is non-null and the result of a `is*` method is `false`, then an error was likely emitted. To avoid the emission of an error the user can pass a default constructed `emitError`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch switches the return type in
MemorySpaceAttrInterfacemethods fromLogicalResulttobool. Asis*methods are predicates.Users of the
MemorySpaceAttrInterfaceAPI must note that, ifemitErroris non-null and the result of ais*method isfalse, then an error was likely emitted. To avoid the emission of an error the user can pass a default constructedemitError.