[mlir][ptr] Switch LogicalResult to bool in MemorySpaceAttrInterrface (#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`.

Author: fabianmcg

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,

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;
 }
 
 //===----------------------------------------------------------------------===//

mlir/test/lib/Dialect/Test/TestAttributes.cpp

@@ -331,43 +331,50 @@ 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();
+  if (emitError)
+    emitError() << "memory space is read-only";
+  return 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();
+  if (emitError)
+    emitError() << "memory space is read-only";
+  return 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();
+  if (emitError)
+    emitError() << "memory space is read-only";
+  return 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();
+  if (emitError)
+    emitError() << "memory space doesn't allow addrspace casts";
+  return 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();
+  if (emitError)
+    emitError() << "memory space doesn't allow int-ptr casts";
+  return false;
 }
 
 //===----------------------------------------------------------------------===//