[Mlir-commits] [mlir] 98bd025 - [mlir][ODS] Migrate generated and test attribute access to use split inherent/properties API (#218872)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Wed Aug 26 03:13:43 PDT 2026


Author: Mehdi Amini
Date: 2026-08-26T10:13:37Z
New Revision: 98bd02564ead9b39d964b69d62c55cc2d3827fa9

URL: https://github.com/llvm/llvm-project/commit/98bd02564ead9b39d964b69d62c55cc2d3827fa9
DIFF: https://github.com/llvm/llvm-project/commit/98bd02564ead9b39d964b69d62c55cc2d3827fa9.diff

LOG: [mlir][ODS] Migrate generated and test attribute access to use split inherent/properties API (#218872)

Teach operation generators and the test dialect to use explicit
discardable or inherent attribute APIs. Update TableGen unit coverage
for the generated accessors.

Assisted-by: Codex

Added: 
    

Modified: 
    mlir/test/lib/Dialect/Test/TestDialect.cpp
    mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
    mlir/test/lib/Dialect/Test/TestOpDefs.cpp
    mlir/test/lib/Dialect/Test/TestOps.td
    mlir/test/lib/Dialect/Test/TestOpsSyntax.cpp
    mlir/test/lib/Dialect/Test/TestPatterns.cpp
    mlir/tools/mlir-tblgen/DialectGen.cpp
    mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
    mlir/tools/mlir-tblgen/OpFormatGen.cpp
    mlir/unittests/TableGen/OpBuildGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/test/lib/Dialect/Test/TestDialect.cpp b/mlir/test/lib/Dialect/Test/TestDialect.cpp
index 9cfd67bb0bfde..57bba2e5749a8 100644
--- a/mlir/test/lib/Dialect/Test/TestDialect.cpp
+++ b/mlir/test/lib/Dialect/Test/TestDialect.cpp
@@ -316,7 +316,8 @@ void test::testSideEffectOpGetEffect(
     Operation *op,
     SmallVectorImpl<SideEffects::EffectInstance<TestEffects::Effect>>
         &effects) {
-  auto effectsAttr = op->getAttrOfType<AffineMapAttr>("effect_parameter");
+  auto effectsAttr =
+      op->getDiscardableAttrOfType<AffineMapAttr>("effect_parameter");
   if (!effectsAttr)
     return;
 
@@ -481,7 +482,8 @@ MutableOperandRange CallWithSegmentsOp::getArgOperandsMutable() {
   // Obtain the canonical segment size attribute name for this op.
   auto segName =
       CallWithSegmentsOp::getOperandSegmentSizesAttrName(op->getName());
-  auto sizesAttr = op->getAttrOfType<DenseI32ArrayAttr>(segName);
+  auto sizesAttr = dyn_cast_or_null<DenseI32ArrayAttr>(
+      op->getInherentAttr(segName).value_or(Attribute{}));
   assert(sizesAttr && "missing operandSegmentSizes attribute on op");
 
   // Compute the start and length of the args segment from the prefix size and

diff  --git a/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp b/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
index 04d956cce2eea..2b52978fe3636 100644
--- a/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
+++ b/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
@@ -327,7 +327,7 @@ struct TestInlinerInterface : public DialectInlinerInterface {
   bool isLegalToInline(Operation *call, Operation *callable,
                        bool wouldBeCloned) const final {
     // Don't allow inlining calls that are marked `noinline`.
-    return !call->hasAttr("noinline");
+    return !call->hasDiscardableAttr("noinline");
   }
   bool isLegalToInline(Region *, Region *, bool, IRMapping &) const final {
     // Inlining into test dialect regions is legal.
@@ -420,7 +420,8 @@ struct TestInlinerInterface : public DialectInlinerInterface {
     // Set attributed on all ops in the inlined blocks.
     for (Block &block : inlinedBlocks) {
       block.walk([&](Operation *op) {
-        op->setAttr("inlined_conversion", UnitAttr::get(call->getContext()));
+        op->setDiscardableAttr("inlined_conversion",
+                               UnitAttr::get(call->getContext()));
       });
     }
   }

diff  --git a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp b/mlir/test/lib/Dialect/Test/TestOpDefs.cpp
index 2e8bce9199fd2..734fceeede346 100644
--- a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp
+++ b/mlir/test/lib/Dialect/Test/TestOpDefs.cpp
@@ -82,7 +82,7 @@ SuccessorOperands TestInternalBranchOp::getSuccessorOperands(unsigned index) {
 
 LogicalResult TestCallOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
   // Check that the callee attribute was specified.
-  auto fnAttr = (*this)->getAttrOfType<FlatSymbolRefAttr>("callee");
+  auto fnAttr = getCalleeAttr();
   if (!fnAttr)
     return emitOpError("requires a 'callee' symbol reference attribute");
   if (!symbolTable.lookupNearestSymbolFrom<FunctionOpInterface>(*this, fnAttr))
@@ -459,7 +459,8 @@ struct TestResource : public SideEffects::Resource::Base<TestResource> {
 void SideEffectOp::getEffects(
     SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
   // Check for an effects attribute on the op instance.
-  ArrayAttr effectsAttr = (*this)->getAttrOfType<ArrayAttr>("effects");
+  ArrayAttr effectsAttr =
+      (*this)->getDiscardableAttrOfType<ArrayAttr>("effects");
   if (!effectsAttr)
     return;
 
@@ -520,7 +521,8 @@ void ConditionalSideEffectOp::getEffects(
 void SideEffectWithRegionOp::getEffects(
     SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
   // Check for an effects attribute on the op instance.
-  ArrayAttr effectsAttr = (*this)->getAttrOfType<ArrayAttr>("effects");
+  ArrayAttr effectsAttr =
+      (*this)->getDiscardableAttrOfType<ArrayAttr>("effects");
   if (!effectsAttr)
     return;
 
@@ -629,10 +631,15 @@ void StringAttrPrettyNameOp::print(OpAsmPrinter &p) {
     }
   }
 
-  if (namesDisagree)
-    p.printOptionalAttrDictWithKeyword((*this)->getAttrs());
-  else
-    p.printOptionalAttrDictWithKeyword((*this)->getAttrs(), {"names"});
+  if (namesDisagree) {
+    SmallVector<NamedAttribute> attrs((*this)->getDiscardableAttrs());
+    attrs.emplace_back(getNamesAttrName(), getNamesAttr());
+    llvm::sort(attrs);
+    p.printOptionalAttrDictWithKeyword(attrs);
+  } else {
+    p.printOptionalAttrDictWithKeyword(
+        (*this)->getDiscardableAttrDictionary().getValue(), {"names"});
+  }
 }
 
 // We set the SSA name in the asm syntax to the contents of the name
@@ -953,7 +960,13 @@ ParseResult TestWithBoundsRegionOp::parse(OpAsmParser &parser,
 }
 
 void TestWithBoundsRegionOp::print(OpAsmPrinter &p) {
-  p.printOptionalAttrDict((*this)->getAttrs());
+  SmallVector<NamedAttribute> attrs((*this)->getDiscardableAttrs());
+  attrs.emplace_back(getUminAttrName(), getUminAttr());
+  attrs.emplace_back(getUmaxAttrName(), getUmaxAttr());
+  attrs.emplace_back(getSminAttrName(), getSminAttr());
+  attrs.emplace_back(getSmaxAttrName(), getSmaxAttr());
+  llvm::sort(attrs);
+  p.printOptionalAttrDict(attrs);
   p << ' ';
   p.printRegionArgument(getRegion().getArgument(0), /*argAttrs=*/{},
                         /*omitType=*/false);

diff  --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 7372c81898dea..96d451388e504 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -720,11 +720,11 @@ def ConversionCallOp : TEST_Op<"conversion_call_op",
   }];
   let extraClassDefinition = [{
     ::mlir::CallInterfaceCallable $cppClass::getCallableForCallee() {
-      return (*this)->getAttrOfType<::mlir::SymbolRefAttr>("callee");
+      return getCalleeAttr();
     }
 
     void $cppClass::setCalleeFromCallable(::mlir::CallInterfaceCallable callee) {
-      (*this)->setAttr("callee", cast<SymbolRefAttr>(callee));
+      setCalleeAttr(cast<SymbolRefAttr>(callee));
     }
   }];
 }
@@ -3122,13 +3122,15 @@ def TestLinalgConvOp :
     }
 
     llvm::SmallVector<mlir::utils::IteratorType> getIteratorTypesArray() {
-      auto attrs = getOperation()->getAttrOfType<mlir::ArrayAttr>("iterator_types");
+      auto attrs = getOperation()->getDiscardableAttrOfType<mlir::ArrayAttr>(
+          "iterator_types");
       auto range = attrs.getAsValueRange<IteratorTypeAttr, mlir::utils::IteratorType>();
       return {range.begin(), range.end()};
     }
 
     mlir::ArrayAttr getIndexingMaps() {
-      return getOperation()->getAttrOfType<mlir::ArrayAttr>("indexing_maps");
+      return getOperation()->getDiscardableAttrOfType<mlir::ArrayAttr>(
+          "indexing_maps");
     }
 
     std::string getLibraryCallName() {
@@ -3185,13 +3187,15 @@ def TestLinalgFillOp :
     }
 
     llvm::SmallVector<mlir::utils::IteratorType> getIteratorTypesArray() {
-      auto attrs = getOperation()->getAttrOfType<mlir::ArrayAttr>("iterator_types");
+      auto attrs = getOperation()->getDiscardableAttrOfType<mlir::ArrayAttr>(
+          "iterator_types");
       auto range = attrs.getAsValueRange<IteratorTypeAttr, mlir::utils::IteratorType>();
       return {range.begin(), range.end()};
     }
 
     mlir::ArrayAttr getIndexingMaps() {
-      return getOperation()->getAttrOfType<mlir::ArrayAttr>("indexing_maps");
+      return getOperation()->getDiscardableAttrOfType<mlir::ArrayAttr>(
+          "indexing_maps");
     }
 
     std::string getLibraryCallName() {
@@ -4715,15 +4719,16 @@ def CallWithSegmentsOp : TEST_Op<"call_with_segments",
 
   let extraClassDefinition = [{
     ::mlir::CallInterfaceCallable $cppClass::getCallableForCallee() {
-      if (auto sym = (*this)->getAttrOfType<::mlir::SymbolRefAttr>("callee"))
+      if (auto sym = (*this)->getDiscardableAttrOfType<::mlir::SymbolRefAttr>(
+              "callee"))
         return ::mlir::CallInterfaceCallable(sym);
       return ::mlir::CallInterfaceCallable();
     }
     void $cppClass::setCalleeFromCallable(::mlir::CallInterfaceCallable callee) {
       if (auto sym = callee.dyn_cast<::mlir::SymbolRefAttr>())
-        (*this)->setAttr("callee", sym);
+        (*this)->setDiscardableAttr("callee", sym);
       else
-        (*this)->removeAttr("callee");
+        (*this)->removeDiscardableAttr("callee");
     }
   }];
 }

diff  --git a/mlir/test/lib/Dialect/Test/TestOpsSyntax.cpp b/mlir/test/lib/Dialect/Test/TestOpsSyntax.cpp
index 5880c2a2302b0..3a971e462e2b8 100644
--- a/mlir/test/lib/Dialect/Test/TestOpsSyntax.cpp
+++ b/mlir/test/lib/Dialect/Test/TestOpsSyntax.cpp
@@ -456,7 +456,8 @@ void PolyForOp::print(OpAsmPrinter &p) {
 
 void PolyForOp::getAsmBlockArgumentNames(Region &region,
                                          OpAsmSetValueNameFn setNameFn) {
-  auto arrayAttr = getOperation()->getAttrOfType<ArrayAttr>("arg_names");
+  auto arrayAttr =
+      getOperation()->getDiscardableAttrOfType<ArrayAttr>("arg_names");
   if (!arrayAttr)
     return;
   auto args = getRegion().front().getArguments();

diff  --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index 9121155f70245..fbc5ab76d9ba1 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -131,7 +131,7 @@ struct FolderInsertBeforePreviouslyFoldedConstantPattern
 
   LogicalResult matchAndRewrite(TestCastOp op,
                                 PatternRewriter &rewriter) const override {
-    if (!op->hasAttr("test_fold_before_previously_folded_op"))
+    if (!op->hasDiscardableAttr("test_fold_before_previously_folded_op"))
       return failure();
     rewriter.setInsertionPointToStart(op->getBlock());
 
@@ -192,10 +192,11 @@ struct MakeOpEligible : public RewritePattern {
 
   LogicalResult matchAndRewrite(Operation *op,
                                 PatternRewriter &rewriter) const override {
-    if (op->hasAttr("eligible"))
+    if (op->hasDiscardableAttr("eligible"))
       return failure();
-    rewriter.modifyOpInPlace(
-        op, [&]() { op->setAttr("eligible", rewriter.getUnitAttr()); });
+    rewriter.modifyOpInPlace(op, [&]() {
+      op->setDiscardableAttr("eligible", rewriter.getUnitAttr());
+    });
     return success();
   }
 };
@@ -210,7 +211,7 @@ struct HoistEligibleOps : public OpRewritePattern<test::OneRegionOp> {
     Operation *toBeHoisted = terminator->getOperands()[0].getDefiningOp();
     if (toBeHoisted->getParentOp() != op)
       return failure();
-    if (!toBeHoisted->hasAttr("eligible"))
+    if (!toBeHoisted->hasDiscardableAttr("eligible"))
       return failure();
     rewriter.moveOpBefore(toBeHoisted, op);
     return success();
@@ -305,7 +306,7 @@ struct MoveAfterParentOp : public RewritePattern {
       return failure();
 
     int64_t moveForwardBy = 0;
-    if (auto advanceBy = op->getAttrOfType<IntegerAttr>("advance"))
+    if (auto advanceBy = op->getDiscardableAttrOfType<IntegerAttr>("advance"))
       moveForwardBy = advanceBy.getInt();
 
     Operation *moveAfter = op->getParentOp();
@@ -363,10 +364,10 @@ struct CloneOp : public RewritePattern {
   LogicalResult matchAndRewrite(Operation *op,
                                 PatternRewriter &rewriter) const override {
     // Do not clone already cloned ops to avoid going into an infinite loop.
-    if (op->hasAttr("was_cloned"))
+    if (op->hasDiscardableAttr("was_cloned"))
       return failure();
     Operation *cloned = rewriter.clone(*op);
-    cloned->setAttr("was_cloned", rewriter.getUnitAttr());
+    cloned->setDiscardableAttr("was_cloned", rewriter.getUnitAttr());
     return success();
   }
 };
@@ -380,12 +381,13 @@ struct CloneRegionBeforeOp : public RewritePattern {
   LogicalResult matchAndRewrite(Operation *op,
                                 PatternRewriter &rewriter) const override {
     // Do not clone already cloned ops to avoid going into an infinite loop.
-    if (op->hasAttr("was_cloned"))
+    if (op->hasDiscardableAttr("was_cloned"))
       return failure();
     for (Region &r : op->getRegions())
       rewriter.cloneRegionBefore(r, op->getBlock());
-    rewriter.modifyOpInPlace(
-        op, [&]() { op->setAttr("was_cloned", rewriter.getUnitAttr()); });
+    rewriter.modifyOpInPlace(op, [&]() {
+      op->setDiscardableAttr("was_cloned", rewriter.getUnitAttr());
+    });
     return success();
   }
 };
@@ -399,7 +401,7 @@ class ReplaceWithNewOp : public RewritePattern {
   LogicalResult matchAndRewrite(Operation *op,
                                 PatternRewriter &rewriter) const override {
     Operation *newOp;
-    if (op->hasAttr("create_erase_op")) {
+    if (op->hasDiscardableAttr("create_erase_op")) {
       newOp = rewriter.create(
           op->getLoc(),
           OperationName("test.erase_op", op->getContext()).getIdentifier(),
@@ -447,7 +449,7 @@ class CreateAndEraseOpAndBlock : public RewritePattern {
 
   LogicalResult matchAndRewrite(Operation *op,
                                 PatternRewriter &rewriter) const override {
-    if (op->hasAttr("was_rewritten"))
+    if (op->hasDiscardableAttr("was_rewritten"))
       return failure();
 
     Operation *newOp = rewriter.create(
@@ -459,8 +461,9 @@ class CreateAndEraseOpAndBlock : public RewritePattern {
     Block *newBlock = rewriter.createBlock(op->getParentRegion());
     rewriter.eraseBlock(newBlock);
 
-    rewriter.modifyOpInPlace(
-        op, [&]() { op->setAttr("was_rewritten", rewriter.getUnitAttr()); });
+    rewriter.modifyOpInPlace(op, [&]() {
+      op->setDiscardableAttr("was_rewritten", rewriter.getUnitAttr());
+    });
     return success();
   }
 };
@@ -623,9 +626,10 @@ struct TestStrictPatternDriver
     (void)applyOpPatternsGreedily(ArrayRef(ops), std::move(patterns), config,
                                   &changed, &allErased);
     Builder b(ctx);
-    getOperation()->setAttr("pattern_driver_changed", b.getBoolAttr(changed));
-    getOperation()->setAttr("pattern_driver_all_erased",
-                            b.getBoolAttr(allErased));
+    getOperation()->setDiscardableAttr("pattern_driver_changed",
+                                       b.getBoolAttr(changed));
+    getOperation()->setDiscardableAttr("pattern_driver_all_erased",
+                                       b.getBoolAttr(allErased));
   }
 
   Option<std::string> strictMode{
@@ -642,15 +646,16 @@ struct TestStrictPatternDriver
 
     LogicalResult matchAndRewrite(Operation *op,
                                   PatternRewriter &rewriter) const override {
-      if (op->hasAttr("skip"))
+      if (op->hasDiscardableAttr("skip"))
         return failure();
 
       Operation *newOp =
           rewriter.create(op->getLoc(), op->getName().getIdentifier(),
                           op->getOperands(), op->getResultTypes());
-      rewriter.modifyOpInPlace(
-          op, [&]() { op->setAttr("skip", rewriter.getBoolAttr(true)); });
-      newOp->setAttr("skip", rewriter.getBoolAttr(true));
+      rewriter.modifyOpInPlace(op, [&]() {
+        op->setDiscardableAttr("skip", rewriter.getBoolAttr(true));
+      });
+      newOp->setDiscardableAttr("skip", rewriter.getBoolAttr(true));
 
       return success();
     }
@@ -782,7 +787,8 @@ static void invokeCreateWithInferredReturnType(Operation *op) {
               properties, op->getRegions(), inferredReturnTypes))) {
         OperationState state(location, OpTy::getOperationName());
         // TODO: Expand to regions.
-        OpTy::build(b, state, values, op->getAttrs());
+        OpTy::build(b, state, values,
+                    op->getDiscardableAttrDictionary().getValue());
         (void)b.create(state);
       }
     }
@@ -1009,9 +1015,9 @@ struct TestValueReplace : public ConversionPattern {
     // Replace the first operand with 2x the second operand.
     Value from = op->getOperand(0);
     Value repl = op->getOperand(1);
-    if (op->hasAttr("conditional")) {
+    if (op->hasDiscardableAttr("conditional")) {
       rewriter.replaceUsesWithIf(from, {repl, repl}, [=](OpOperand &use) {
-        return use.getOwner()->hasAttr("replace_uses");
+        return use.getOwner()->hasDiscardableAttr("replace_uses");
       });
     } else {
       rewriter.replaceAllUsesWith(from, {repl, repl});
@@ -1019,8 +1025,8 @@ struct TestValueReplace : public ConversionPattern {
     rewriter.modifyOpInPlace(op, [&] {
       // If the "trigger_rollback" attribute is set, keep the op illegal, so
       // that a rollback is triggered.
-      if (!op->hasAttr("trigger_rollback"))
-        op->setAttr("is_legal", rewriter.getUnitAttr());
+      if (!op->hasDiscardableAttr("trigger_rollback"))
+        op->setDiscardableAttr("is_legal", rewriter.getUnitAttr());
     });
     return success();
   }
@@ -1066,7 +1072,7 @@ struct TestUndoPropertiesModification : public ConversionPattern {
   LogicalResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const final {
-    if (!op->hasAttr("modify_inplace"))
+    if (!op->hasDiscardableAttr("modify_inplace"))
       return failure();
     rewriter.modifyOpInPlace(
         op, [&]() { cast<TestOpWithProperties>(op).getProperties().setA(42); });
@@ -1283,9 +1289,7 @@ struct TestBoundedRecursiveRewrite
   LogicalResult matchAndRewrite(TestRecursiveRewriteOp op,
                                 PatternRewriter &rewriter) const final {
     // Decrement the depth of the op in-place.
-    rewriter.modifyOpInPlace(op, [&] {
-      op->setAttr("depth", rewriter.getI64IntegerAttr(op.getDepth() - 1));
-    });
+    rewriter.modifyOpInPlace(op, [&] { op.setDepth(op.getDepth() - 1); });
     return success();
   }
 };
@@ -1352,7 +1356,7 @@ class TestReplaceWithValidProducer : public ConversionPattern {
   LogicalResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const final {
-    auto attr = op->getAttrOfType<TypeAttr>("type");
+    auto attr = op->getDiscardableAttrOfType<TypeAttr>("type");
     if (!attr)
       return failure();
     rewriter.replaceOpWithNewOp<TestValidProducerOp>(op, attr.getValue());
@@ -1377,7 +1381,8 @@ class TestReplaceWithValidConsumer : public ConversionPattern {
     // converter.
     // with_converter absent: pattern must have been initialized without a type
     // converter.
-    if (op->hasAttr("with_converter") != static_cast<bool>(getTypeConverter()))
+    if (op->hasDiscardableAttr("with_converter") !=
+        static_cast<bool>(getTypeConverter()))
       return failure();
     rewriter.replaceOpWithNewOp<TestValidConsumerOp>(op, operands[0]);
     return success();
@@ -1454,7 +1459,8 @@ class TestMultiple1ToNReplacement : public ConversionPattern {
         types.push_back(t);
       }
       OperationState state(op->getLoc(), name,
-                           /*operands=*/{}, types, op->getAttrs());
+                           /*operands=*/{}, types,
+                           op->getDiscardableAttrDictionary().getValue());
       auto *newOp = rewriter.create(state);
       SmallVector<ValueRange> repls;
       for (size_t i = 0, e = op->getNumResults(); i < e; ++i)
@@ -1498,8 +1504,9 @@ class TestPostOrderLegalization : public ConversionPattern {
     for (Region &r : op->getRegions())
       if (failed(rewriter.legalize(&r)))
         return failure();
-    rewriter.modifyOpInPlace(
-        op, [&]() { op->setAttr("is_legal", rewriter.getUnitAttr()); });
+    rewriter.modifyOpInPlace(op, [&]() {
+      op->setDiscardableAttr("is_legal", rewriter.getUnitAttr());
+    });
     return success();
   }
 };
@@ -1646,10 +1653,10 @@ struct TestLegalizePatternDriver
         [&](func::CallOp op) { return converter.isLegal(op); });
     target.addDynamicallyLegalOp(
         OperationName("test.value_replace", &getContext()),
-        [](Operation *op) { return op->hasAttr("is_legal"); });
+        [](Operation *op) { return op->hasDiscardableAttr("is_legal"); });
     target.addDynamicallyLegalOp(
         OperationName("test.post_order_legalization", &getContext()),
-        [](Operation *op) { return op->hasAttr("is_legal"); });
+        [](Operation *op) { return op->hasDiscardableAttr("is_legal"); });
 
     // TestCreateUnregisteredOp creates `arith.constant` operation,
     // which was not added to target intentionally to test
@@ -1666,7 +1673,7 @@ struct TestLegalizePatternDriver
     // Check support for marking certain operations as recursively legal.
     target.markOpRecursivelyLegal<func::FuncOp, ModuleOp>([](Operation *op) {
       return static_cast<bool>(
-          op->getAttrOfType<UnitAttr>("test.recursively_legal"));
+          op->getDiscardableAttrOfType<UnitAttr>("test.recursively_legal"));
     });
 
     // Mark the bound recursion operation as dynamically legal.
@@ -1707,7 +1714,8 @@ struct TestLegalizePatternDriver
     if (mode == ConversionMode::Full) {
       // Check support for marking unknown operations as dynamically legal.
       target.markUnknownOpDynamicallyLegal([](Operation *op) {
-        return (bool)op->getAttrOfType<UnitAttr>("test.dynamically_legal");
+        return (bool)op->getDiscardableAttrOfType<UnitAttr>(
+            "test.dynamically_legal");
       });
 
       if (failed(applyFullConversion(getOperation(), target,
@@ -1952,7 +1960,7 @@ struct RewriteDynamicOp : public RewritePattern {
 
     OperationState state(op->getLoc(), "test.dynamic_generic",
                          op->getOperands(), op->getResultTypes(),
-                         op->getAttrs());
+                         op->getDiscardableAttrDictionary().getValue());
     auto *newOp = rewriter.create(state);
     rewriter.replaceOp(op, newOp->getResults());
     return success();
@@ -2198,7 +2206,8 @@ struct TestTypeConversionDriver
         op = op->getParentOfType<FunctionOpInterface>();
       if (!op)
         return Type();
-      auto incrementAttr = op->getAttrOfType<IntegerAttr>("increment");
+      auto incrementAttr =
+          op->getDiscardableAttrOfType<IntegerAttr>("increment");
       if (!incrementAttr)
         return Type();
       return IntegerType::get(v.getContext(),

diff  --git a/mlir/tools/mlir-tblgen/DialectGen.cpp b/mlir/tools/mlir-tblgen/DialectGen.cpp
index 8eecad39f49f3..d2f9faa7a9db5 100644
--- a/mlir/tools/mlir-tblgen/DialectGen.cpp
+++ b/mlir/tools/mlir-tblgen/DialectGen.cpp
@@ -215,17 +215,17 @@ static const char *const discardableAttrHelperDecl = R"(
         : name(::mlir::StringAttr::get(ctx, getNameStr())) {{}
 
      {2} getAttr(::mlir::Operation *op) const {{
-       return op->getAttrOfType<{2}>(name);
+       return op->getDiscardableAttrOfType<{2}>(name);
      }
      void setAttr(::mlir::Operation *op, {2} val) const {{
-       op->setAttr(name, val);
+       op->setDiscardableAttr(name, val);
      }
      bool isAttrPresent(::mlir::Operation *op) const {{
-       return op->hasAttrOfType<{2}>(name);
+       return op->hasDiscardableAttrOfType<{2}>(name);
      }
      void removeAttr(::mlir::Operation *op) const {{
-       assert(op->hasAttrOfType<{2}>(name));
-       op->removeAttr(name);
+       assert(op->hasDiscardableAttrOfType<{2}>(name));
+       op->removeDiscardableAttr(name);
      }
    };
    {0}AttrHelper get{0}AttrHelper() {

diff  --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index fab1c5b4870aa..6d7ec67e46dde 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -373,7 +373,7 @@ class OpOrAdaptorHelper {
 
   // Get the code snippet for getting the named attribute range.
   StringRef getAttrRange() const {
-    return emitForOp ? "(*this)->getAttrs()" : "odsAttrs";
+    return emitForOp ? "(*this)->getRawDictionaryAttrs()" : "odsAttrs";
   }
 
   // Get the prefix code for emitting an error.
@@ -2356,11 +2356,10 @@ void OpEmitter::genNamedOperandSetters() {
     // MutableOperandRangeRange that provides a range over all of the
     // sub-ranges.
     if (operand.isVariadicOfVariadic()) {
-      body << "  return "
-              "mutableRange.split(*(*this)->getAttrDictionary().getNamed("
-           << op.getGetterName(
-                  operand.constraint.getVariadicOfVariadicSegmentSizeAttr())
-           << "AttrName()));\n";
+      StringRef segmentAttr =
+          operand.constraint.getVariadicOfVariadicSegmentSizeAttr();
+      body << "  return mutableRange.split({" << op.getGetterName(segmentAttr)
+           << "AttrName(), " << op.getGetterName(segmentAttr) << "Attr()});\n";
     } else {
       // Otherwise, we use the full range directly.
       body << "  return mutableRange;\n";

diff  --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 05a26402005fd..4a3b9f57e90ad 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -2327,7 +2327,12 @@ static const char *regionSingleBlockImplicitTerminatorPrinterCode = R"(
   {
     bool printTerminator = true;
     if (auto *term = {0}.empty() ? nullptr : {0}.begin()->getTerminator()) {{
-      printTerminator = !term->getAttrDictionary().empty() ||
+      ::mlir::NamedAttrList termAttrs(term->getRawDictionaryAttrs());
+      term->getName().walkInherentAttrs(
+          term, [&](::llvm::StringRef name, ::mlir::Attribute &attr) {{
+            termAttrs.append(name, attr);
+          });
+      printTerminator = !termAttrs.empty() ||
                         term->getNumOperands() != 0 ||
                         term->getNumResults() != 0;
     }
@@ -2608,11 +2613,20 @@ static void genAttrDictPrinter(OperationFormat &fmt, Operator &op,
   if (fmt.hasPropDict)
     body << "  _odsPrinter.printOptionalAttrDict"
          << (withKeyword ? "WithKeyword" : "")
-         << "(llvm::to_vector((*this)->getDiscardableAttrs()), elidedAttrs);\n";
-  else
-    body << "  _odsPrinter.printOptionalAttrDict"
+         << "(llvm::to_vector((*this)->getDiscardableAttrDictionary().getValue("
+            ")), elidedAttrs);\n";
+  else {
+    body << "  ::mlir::NamedAttrList _odsAttrs("
+            "(*this)->getRawDictionaryAttrs());\n"
+            "  (*this)->getName().walkInherentAttrs(*this, "
+            "[&](::llvm::StringRef name, ::mlir::Attribute &attr) {\n"
+            "    _odsAttrs.append(name, attr);\n"
+            "  });\n"
+            "  _odsPrinter.printOptionalAttrDict"
          << (withKeyword ? "WithKeyword" : "")
-         << "((*this)->getAttrs(), elidedAttrs);\n";
+         << "(_odsAttrs.getDictionary(getContext()).getValue(), "
+            "elidedAttrs);\n";
+  }
 }
 
 /// Generate the printer for a literal value. `shouldEmitSpace` is true if a
@@ -2654,7 +2668,12 @@ static void genCustomDirectiveParameterPrinter(FormatElement *element,
     body << op.getGetterName(attr->getVar()->name) << "Attr()";
 
   } else if (isa<AttrDictDirective>(element)) {
-    body << "getOperation()->getAttrDictionary()";
+    body << "[&]() { ::mlir::NamedAttrList attrs("
+            "getOperation()->getRawDictionaryAttrs()); "
+            "getOperation()->getName().walkInherentAttrs(getOperation(), "
+            "[&](::llvm::StringRef name, ::mlir::Attribute &attr) { "
+            "attrs.append(name, attr); }); return attrs.getDictionary("
+            "getOperation()->getContext()); }()";
 
   } else if (isa<PropDictDirective>(element)) {
     body << "getProperties()";

diff  --git a/mlir/unittests/TableGen/OpBuildGen.cpp b/mlir/unittests/TableGen/OpBuildGen.cpp
index 53ee59c7b5fe9..f34aa4af87e9f 100644
--- a/mlir/unittests/TableGen/OpBuildGen.cpp
+++ b/mlir/unittests/TableGen/OpBuildGen.cpp
@@ -33,6 +33,15 @@ static MLIRContext &getContext() {
 /// Test fixture for providing basic utilities for testing.
 class OpBuildGenTest : public ::testing::Test {
 protected:
+  static NamedAttrList collectAttrs(Operation *op) {
+    NamedAttrList attrs(op->getDiscardableAttrDictionary());
+    if (op->getPropertiesStorageSize())
+      op->getName().walkInherentAttrs(op, [&](StringRef name, Attribute &attr) {
+        attrs.append(name, attr);
+      });
+    return NamedAttrList(attrs.getDictionary(op->getContext()));
+  }
+
   OpBuildGenTest()
       : ctx(getContext()), builder(&ctx), loc(builder.getUnknownLoc()),
         i32Ty(builder.getI32Type()), f32Ty(builder.getF32Type()),
@@ -60,10 +69,10 @@ class OpBuildGenTest : public ::testing::Test {
     for (unsigned idx : llvm::seq(0U, op->getNumOperands()))
       EXPECT_EQ(op->getOperand(idx), operands[idx]);
 
-    EXPECT_EQ(op->getAttrs().size(), attrs.size());
+    NamedAttrList actualAttrs = collectAttrs(op);
+    EXPECT_EQ(actualAttrs.getAttrs().size(), attrs.size());
     for (unsigned idx : llvm::seq<unsigned>(0U, attrs.size()))
-      EXPECT_EQ(op->getAttr(attrs[idx].getName().strref()),
-                attrs[idx].getValue());
+      EXPECT_EQ(actualAttrs.get(attrs[idx].getName()), attrs[idx].getValue());
 
     EXPECT_TRUE(mlir::succeeded(concreteOp.verify()));
     concreteOp.erase();
@@ -85,11 +94,12 @@ class OpBuildGenTest : public ::testing::Test {
     for (unsigned idx : llvm::seq(0U, op->getNumOperands()))
       EXPECT_EQ(op->getOperand(idx), operands[idx]);
 
-    EXPECT_EQ(op->getAttrs().size(), attrs.size());
-    if (op->getAttrs().size() != attrs.size()) {
+    NamedAttrList actualAttrs = collectAttrs(op);
+    EXPECT_EQ(actualAttrs.getAttrs().size(), attrs.size());
+    if (actualAttrs.getAttrs().size() != attrs.size()) {
       // Simple export where there is mismatch count.
       llvm::errs() << "Op attrs:\n";
-      for (auto it : op->getAttrs())
+      for (auto it : actualAttrs)
         llvm::errs() << "\t" << it.getName() << " = " << it.getValue() << "\n";
 
       llvm::errs() << "Expected attrs:\n";
@@ -97,8 +107,7 @@ class OpBuildGenTest : public ::testing::Test {
         llvm::errs() << "\t" << it.getName() << " = " << it.getValue() << "\n";
     } else {
       for (unsigned idx : llvm::seq<unsigned>(0U, attrs.size()))
-        EXPECT_EQ(op->getAttr(attrs[idx].getName().strref()),
-                  attrs[idx].getValue());
+        EXPECT_EQ(actualAttrs.get(attrs[idx].getName()), attrs[idx].getValue());
     }
 
     EXPECT_TRUE(mlir::succeeded(concreteOp.verify()));


        


More information about the Mlir-commits mailing list