Fix quadratic behavior from Operation::setAttr() (NFC)

[mehdi_amini]

Summary

This API tries to ensure some backward compatibility for properties,
but doing so in multiple-layers was causing quadratic behavior.
Instead of setAttrs() repeatingly calling to setAttr() we inline
the logic and apply it locally in a single traversal.

Fixes #62800

[matthiaskramm]

mlir/lib/IR/Operation.cpp

308

"discardable" sounds odd to me, in this context.

"nonInherentAttrs"?

[mehdi_amini]

mlir/lib/IR/Operation.cpp

308

This is the official terminology, this class as methods setDiscardableAttrs() (which does not do this filtering): the plan is to deprecate setAttrs() and redirect users to setDiscardableAttrs() (and handle properties separately).

308

See https://mlir.llvm.org/docs/LangRef/#attributes (search for "discardable" there)

[matthiaskramm]

Thanks for fixing this issue!

mlir/lib/IR/Operation.cpp

308

Ok, thanks for clarifying!

mlir/lib/IR/Operation.cpp

if (getPropertiesStorageSize()) {
     return attrsList.getDictionary(getContext());
   }
   return attrs;
 }
 
 void Operation::setAttrs(DictionaryAttr newAttrs) {
   assert(newAttrs && "expected valid attribute dictionary");
   if (getPropertiesStorageSize()) {
-    attrs = DictionaryAttr::get(getContext(), {});
-    for (const NamedAttribute &attr : newAttrs)
-      setAttr(attr.getName(), attr.getValue());
-    return;
+    // We're spliting the providing DictionaryAttr by removing the inherentAttr
+    // which will be stored in the properties.
+    SmallVector<NamedAttribute> discardableAttrs;
+    discardableAttrs.reserve(newAttrs.size());
+    for (NamedAttribute attr : newAttrs) {
+      if (std::optional<Attribute> inherentAttr =
+              getInherentAttr(attr.getName()))
+        setInherentAttr(attr.getName(), attr.getValue());
+      else
+        discardableAttrs.push_back(attr);
+    }
+    if (discardableAttrs.size() != newAttrs.size())
+      newAttrs = DictionaryAttr::get(getContext(), discardableAttrs);
   }
   attrs = newAttrs;
 }
 void Operation::setAttrs(ArrayRef<NamedAttribute> newAttrs) {
   if (getPropertiesStorageSize()) {
-    setAttrs(DictionaryAttr::get(getContext(), {}));
-    for (const NamedAttribute &attr : newAttrs)
-      setAttr(attr.getName(), attr.getValue());
+    // We're spliting the providing array of attributes by removing the inherentAttr
+    // which will be stored in the properties.
+    SmallVector<NamedAttribute> discardableAttrs;
+    discardableAttrs.reserve(newAttrs.size());
+    for (NamedAttribute attr : newAttrs) {
+      if (std::optional<Attribute> inherentAttr =
+              getInherentAttr(attr.getName()))
+        setInherentAttr(attr.getName(), attr.getValue());
+      else
+        discardableAttrs.push_back(attr);
+    }
+    attrs = DictionaryAttr::get(getContext(), discardableAttrs);
     return;
   }
   attrs = DictionaryAttr::get(getContext(), newAttrs);
 }
 
 std::optional<Attribute> Operation::getInherentAttr(StringRef name) {
   return getName().getInherentAttr(this, name);
 }