[Mlir-commits] [mlir] Fix some AffineOps to properly declare their inherent affinemap Attribute (PR #66050)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Tue Sep 12 00:27:41 PDT 2023


llvmbot wrote:

@llvm/pr-subscribers-mlir

<details>
<summary>Changes</summary>

This makes it more consistent with properties.
--
Full diff: https://github.com/llvm/llvm-project/pull/66050.diff

6 Files Affected:

- (modified) mlir/include/mlir/Dialect/Affine/IR/AffineOps.h (+4-4) 
- (modified) mlir/include/mlir/Dialect/Affine/IR/AffineOps.td (+20-15) 
- (modified) mlir/include/mlir/IR/SymbolInterfaces.td (+1-1) 
- (modified) mlir/lib/Dialect/Affine/IR/AffineOps.cpp (+1-7) 
- (modified) mlir/test/Dialect/Affine/invalid.mlir (+1-1) 
- (modified) mlir/test/Dialect/Affine/load-store-invalid.mlir (-29) 


<pre>
diff --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
index 153878a41a043c2..704c2704536d20a 100644
--- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
+++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
@@ -117,7 +117,7 @@ class AffineDmaStartOp
   /// Returns the affine map used to access the source memref.
   AffineMap getSrcMap() { return getSrcMapAttr().getValue(); }
   AffineMapAttr getSrcMapAttr() {
-    return cast<AffineMapAttr>((*this)->getAttr(getSrcMapAttrStrName()));
+    return cast<AffineMapAttr>(*(*this)->getInherentAttr(getSrcMapAttrStrName()));
   }
 
   /// Returns the source memref affine map indices for this DMA operation.
@@ -156,7 +156,7 @@ class AffineDmaStartOp
   /// Returns the affine map used to access the destination memref.
   AffineMap getDstMap() { return getDstMapAttr().getValue(); }
   AffineMapAttr getDstMapAttr() {
-    return cast<AffineMapAttr>((*this)->getAttr(getDstMapAttrStrName()));
+    return cast<AffineMapAttr>(*(*this)->getInherentAttr(getDstMapAttrStrName()));
   }
 
   /// Returns the destination memref indices for this DMA operation.
@@ -185,7 +185,7 @@ class AffineDmaStartOp
   /// Returns the affine map used to access the tag memref.
   AffineMap getTagMap() { return getTagMapAttr().getValue(); }
   AffineMapAttr getTagMapAttr() {
-    return cast<AffineMapAttr>((*this)->getAttr(getTagMapAttrStrName()));
+    return cast<AffineMapAttr>(*(*this)->getInherentAttr(getTagMapAttrStrName()));
   }
 
   /// Returns the tag memref indices for this DMA operation.
@@ -307,7 +307,7 @@ class AffineDmaWaitOp
   /// Returns the affine map used to access the tag memref.
   AffineMap getTagMap() { return getTagMapAttr().getValue(); }
   AffineMapAttr getTagMapAttr() {
-    return cast<AffineMapAttr>((*this)->getAttr(getTagMapAttrStrName()));
+    return cast<AffineMapAttr>(*(*this)->getInherentAttr(getTagMapAttrStrName()));
   }
 
   /// Returns the tag memref index for this DMA operation.
diff --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
index 3ec52fc7a4dfe0b..69798905f7a15fa 100644
--- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
+++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
@@ -284,18 +284,18 @@ def AffineForOp : Affine_Op<"for",
 
     /// Returns loop step.
     int64_t getStep() {
-      return ::llvm::cast<IntegerAttr>((*this)->getAttr(getStepAttrStrName())).getInt();
+      return ::llvm::cast<IntegerAttr>(*(*this)->getInherentAttr(getStepAttrStrName())).getInt();
     }
 
     /// Returns affine map for the lower bound.
     AffineMap getLowerBoundMap() { return getLowerBoundMapAttr().getValue(); }
     AffineMapAttr getLowerBoundMapAttr() {
-      return ::llvm::cast<AffineMapAttr>((*this)->getAttr(getLowerBoundAttrStrName()));
+      return ::llvm::cast<AffineMapAttr>(*(*this)->getInherentAttr(getLowerBoundAttrStrName()));
     }
     /// Returns affine map for the upper bound. The upper bound is exclusive.
     AffineMap getUpperBoundMap() { return getUpperBoundMapAttr().getValue(); }
     AffineMapAttr getUpperBoundMapAttr() {
-      return ::llvm::cast<AffineMapAttr>((*this)->getAttr(getUpperBoundAttrStrName()));
+      return ::llvm::cast<AffineMapAttr>(*(*this)->getInherentAttr(getUpperBoundAttrStrName()));
     }
 
     /// Set lower bound. The new bound must have the same number of operands as
@@ -497,7 +497,8 @@ class AffineLoadOpBase<string mnemonic, list<Trait> traits = []> :
         MemRefsNormalizable])> {
   let arguments = (ins Arg<AnyMemRef, "the reference to load from",
       [MemRead]>:$memref,
-      Variadic<Index>:$indices);
+      Variadic<Index>:$indices,
+      AffineMapAttr:$map);
 
   code extraClassDeclarationBase = [{
     /// Returns the operand index of the memref.
@@ -507,7 +508,7 @@ class AffineLoadOpBase<string mnemonic, list<Trait> traits = []> :
 
     /// Returns the affine map used to index the memref for this operation.
     AffineMapAttr getAffineMapAttr() {
-      return ::llvm::cast<AffineMapAttr>((*this)->getAttr(getMapAttrStrName()));
+      return getProperties().map;
     }
 
     static StringRef getMapAttrStrName() { return "map"; }
@@ -801,7 +802,8 @@ def AffinePrefetchOp : Affine_Op<"prefetch",
                    BoolAttr:$isWrite,
                    ConfinedAttr<I32Attr, [IntMinValue<0>,
                      IntMaxValue<3>]>:$localityHint,
-                   BoolAttr:$isDataCache);
+                   BoolAttr:$isDataCache,
+                   AffineMapAttr:$map);
 
   let builders = [
     OpBuilder<(ins "Value":$memref, "AffineMap":$map,
@@ -814,11 +816,12 @@ def AffinePrefetchOp : Affine_Op<"prefetch",
       auto isWriteAttr = $_builder.getBoolAttr(isWrite);
       auto isDataCacheAttr = $_builder.getBoolAttr(isDataCache);
       $_state.addOperands(memref);
-      $_state.addAttribute(getMapAttrStrName(), AffineMapAttr::get(map));
       $_state.addOperands(mapOperands);
-      $_state.addAttribute(getLocalityHintAttrStrName(), localityHintAttr);
-      $_state.addAttribute(getIsWriteAttrStrName(), isWriteAttr);
-      $_state.addAttribute(getIsDataCacheAttrStrName(), isDataCacheAttr);
+      Properties &prop = $_state.getOrAddProperties<Properties>();
+      prop.map = AffineMapAttr::get(map);
+      prop.localityHint = localityHintAttr;
+      prop.isWrite = isWriteAttr;
+      prop.isDataCache = isDataCacheAttr;
     }]>];
 
   let extraClassDeclaration = [{
@@ -829,10 +832,10 @@ def AffinePrefetchOp : Affine_Op<"prefetch",
     /// Returns the affine map used to index the memref for this operation.
     AffineMap getAffineMap() { return getAffineMapAttr().getValue(); }
     AffineMapAttr getAffineMapAttr() {
-      return ::llvm::cast<AffineMapAttr>((*this)->getAttr(getMapAttrStrName()));
+      return getProperties().map;
     }
 
-    /// Impelements the AffineMapAccessInterface.
+    /// Implements the AffineMapAccessInterface.
     /// Returns the AffineMapAttr associated with 'memref'.
     NamedAttribute getAffineMapAttrForMemRef(Value mref) {
       assert(mref == getMemref() &&
@@ -874,7 +877,7 @@ class AffineStoreOpBase<string mnemonic, list<Trait> traits = []> :
 
     /// Returns the affine map used to index the memref for this operation.
     AffineMapAttr getAffineMapAttr() {
-      return ::llvm::cast<AffineMapAttr>((*this)->getAttr(getMapAttrStrName()));
+      return getProperties().map;
     }
 
     static StringRef getMapAttrStrName() { return "map"; }
@@ -912,7 +915,8 @@ def AffineStoreOp : AffineStoreOpBase<"store"> {
   let arguments = (ins AnyType:$value,
       Arg<AnyMemRef, "the reference to store to",
       [MemWrite]>:$memref,
-      Variadic<Index>:$indices);
+      Variadic<Index>:$indices,
+      AffineMapAttr:$map);
 
   let skipDefaultBuilders = 1;
   let builders = [
@@ -1065,7 +1069,8 @@ def AffineVectorStoreOp : AffineStoreOpBase<"vector_store"> {
   let arguments = (ins AnyVector:$value,
       Arg<AnyMemRef, "the reference to store to",
       [MemWrite]>:$memref,
-      Variadic<Index>:$indices);
+      Variadic<Index>:$indices,
+      AffineMapAttr:$map);
 
   let skipDefaultBuilders = 1;
   let builders = [
diff --git a/mlir/include/mlir/IR/SymbolInterfaces.td b/mlir/include/mlir/IR/SymbolInterfaces.td
index 3da9dedeb8bdcf0..844601f8f6837c4 100644
--- a/mlir/include/mlir/IR/SymbolInterfaces.td
+++ b/mlir/include/mlir/IR/SymbolInterfaces.td
@@ -163,7 +163,7 @@ def Symbol : OpInterface<"SymbolOpInterface"> {
     // If this is an optional symbol, bail out early if possible.
     auto concreteOp = cast<ConcreteOp>($_op);
     if (concreteOp.isOptionalSymbol()) {
-      if(!concreteOp->getAttr(::mlir::SymbolTable::getSymbolAttrName()))
+      if(!concreteOp->getInherentAttr(::mlir::SymbolTable::getSymbolAttrName()).value_or(Attribute{}))
         return success();
     }
     if (::mlir::failed(::mlir::detail::verifySymbol($_op)))
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 9d262c7c204e6d0..4455cc88e65e55d 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -3135,17 +3135,11 @@ static LogicalResult
 verifyMemoryOpIndexing(Operation *op, AffineMapAttr mapAttr,
                        Operation::operand_range mapOperands,
                        MemRefType memrefType, unsigned numIndexOperands) {
-  if (mapAttr) {
     AffineMap map = mapAttr.getValue();
     if (map.getNumResults() != memrefType.getRank())
       return op->emitOpError("affine map num results must equal memref rank");
     if (map.getNumInputs() != numIndexOperands)
       return op->emitOpError("expects as many subscripts as affine map inputs");
-  } else {
-    if (memrefType.getRank() != numIndexOperands)
-      return op->emitOpError(
-          "expects the number of subscripts to be equal to memref rank");
-  }
 
   Region *scope = getAffineScope(op);
   for (auto idx : mapOperands) {
@@ -3224,7 +3218,7 @@ void AffineStoreOp::build(OpBuilder &builder, OperationState &result,
   result.addOperands(valueToStore);
   result.addOperands(memref);
   result.addOperands(mapOperands);
-  result.addAttribute(getMapAttrStrName(), AffineMapAttr::get(map));
+  result.getOrAddProperties<Properties>().map = AffineMapAttr::get(map);
 }
 
 // Use identity map.
diff --git a/mlir/test/Dialect/Affine/invalid.mlir b/mlir/test/Dialect/Affine/invalid.mlir
index 964e281b625379b..fd7d7df9d8735f5 100644
--- a/mlir/test/Dialect/Affine/invalid.mlir
+++ b/mlir/test/Dialect/Affine/invalid.mlir
@@ -145,7 +145,7 @@ func.func @affine_store_missing_l_square(%C: memref<4096x4096xf32>) {
 func.func @affine_store_wrong_value_type(%C: memref<f32>) {
   %c0 = arith.constant 0 : i32
   // expected-error at +1 {{value to store must have the same type as memref element type}}
-  "affine.store"(%c0, %C) : (i32, memref<f32>) -> ()
+  "affine.store"(%c0, %C) <{map = affine_map<(i) -> (i)>}> : (i32, memref<f32>) -> ()
   return
 }
 
diff --git a/mlir/test/Dialect/Affine/load-store-invalid.mlir b/mlir/test/Dialect/Affine/load-store-invalid.mlir
index 4be34d56e443f82..482d2f35e094923 100644
--- a/mlir/test/Dialect/Affine/load-store-invalid.mlir
+++ b/mlir/test/Dialect/Affine/load-store-invalid.mlir
@@ -1,12 +1,5 @@
 // RUN: mlir-opt %s -split-input-file -verify-diagnostics
 
-func.func @load_too_many_subscripts(%arg0: memref<?x?xf32>, %arg1: index, %arg2: index, %arg3: index) {
-  // expected-error at +1 {{expects the number of subscripts to be equal to memref rank}}
-  "affine.load"(%arg0, %arg1, %arg2, %arg3) : (memref<?x?xf32>, index, index, index) -> f32
-}
-
-// -----
-
 func.func @load_too_many_subscripts_map(%arg0: memref<?x?xf32>, %arg1: index, %arg2: index, %arg3: index) {
   // expected-error at +1 {{op expects as many subscripts as affine map inputs}}
   "affine.load"(%arg0, %arg1, %arg2, %arg3)
@@ -15,13 +8,6 @@ func.func @load_too_many_subscripts_map(%arg0: memref<?x?xf32>, %arg1: index, %a
 
 // -----
 
-func.func @load_too_few_subscripts(%arg0: memref<?x?xf32>, %arg1: index) {
-  // expected-error at +1 {{expects the number of subscripts to be equal to memref rank}}
-  "affine.load"(%arg0, %arg1) : (memref<?x?xf32>, index) -> f32
-}
-
-// -----
-
 func.func @load_too_few_subscripts_map(%arg0: memref<?x?xf32>, %arg1: index) {
   // expected-error at +1 {{op expects as many subscripts as affine map inputs}}
   "affine.load"(%arg0, %arg1)
@@ -30,14 +16,6 @@ func.func @load_too_few_subscripts_map(%arg0: memref<?x?xf32>, %arg1: index) {
 
 // -----
 
-func.func @store_too_many_subscripts(%arg0: memref<?x?xf32>, %arg1: index, %arg2: index,
-                                %arg3: index, %val: f32) {
-  // expected-error at +1 {{expects the number of subscripts to be equal to memref rank}}
-  "affine.store"(%val, %arg0, %arg1, %arg2, %arg3) : (f32, memref<?x?xf32>, index, index, index) -> ()
-}
-
-// -----
-
 func.func @store_too_many_subscripts_map(%arg0: memref<?x?xf32>, %arg1: index, %arg2: index,
                                     %arg3: index, %val: f32) {
   // expected-error at +1 {{op expects as many subscripts as affine map inputs}}
@@ -47,13 +25,6 @@ func.func @store_too_many_subscripts_map(%arg0: memref<?x?xf32>, %arg1: index, %
 
 // -----
 
-func.func @store_too_few_subscripts(%arg0: memref<?x?xf32>, %arg1: index, %val: f32) {
-  // expected-error at +1 {{expects the number of subscripts to be equal to memref rank}}
-  "affine.store"(%val, %arg0, %arg1) : (f32, memref<?x?xf32>, index) -> ()
-}
-
-// -----
-
 func.func @store_too_few_subscripts_map(%arg0: memref<?x?xf32>, %arg1: index, %val: f32) {
   // expected-error at +1 {{op expects as many subscripts as affine map inputs}}
   "affine.store"(%val, %arg0, %arg1)
</pre>

</details>

https://github.com/llvm/llvm-project/pull/66050


More information about the Mlir-commits mailing list