[Mlir-commits] [mlir] [mlir] Reduce template work in operation registration (NFC) (PR #233051)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

Save use 3.18% less user CPU when building LLVM/NVVM/ROCDL, and 0.96% total user CPU when building mlir-opt. Linked mlir-opt code is 0.60% smaller. 

The main thing is to share no-op helper specializations among traits that do not define fold, verification, region-verification, or interface hooks. We also preserve pack order and short-circuiting while removing the extra interface-pack forwarding instantiation.

Assisted-by: Codex

---
Full diff: https://github.com/llvm/llvm-project/pull/233051.diff


3 Files Affected:

- (modified) mlir/include/mlir/IR/OpDefinition.h (+20-7) 
- (modified) mlir/include/mlir/Support/InterfaceSupport.h (+8-19) 
- (modified) mlir/lib/Support/InterfaceSupport.cpp (+14) 


``````````diff
diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index 0ab6679187c8eb..38cd7da8495e19 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -1643,7 +1643,12 @@ foldTrait(Operation *, ArrayRef<Attribute>, SmallVectorImpl<OpFoldResult> &) {
 template <typename... Ts>
 LogicalResult foldTraits(Operation *op, ArrayRef<Attribute> operands,
                          SmallVectorImpl<OpFoldResult> &results) {
-  return success((succeeded(foldTrait<Ts>(op, operands, results)) || ...));
+  // Reuse one no-op specialization for traits without a folder.
+  return success(
+      (succeeded(
+           foldTrait<std::conditional_t<detect_has_any_fold_trait<Ts>::value,
+                                        Ts, void>>(op, operands, results)) ||
+       ...));
 }
 
 //===----------------------------------------------------------------------===//
@@ -1676,7 +1681,11 @@ LogicalResult verifyTrait(Operation *op) {
 /// Given a set of traits, return the result of verifying the given operation.
 template <typename... Ts>
 LogicalResult verifyTraits(Operation *op) {
-  return success((succeeded(verifyTrait<Ts>(op)) && ...));
+  // Avoid instantiating an identical no-op verifier for each absent hook.
+  return success(
+      (succeeded(verifyTrait<std::conditional_t<
+                     detect_has_verify_trait<Ts>::value, Ts, void>>(op)) &&
+       ...));
 }
 
 /// Verify the given trait if it provides a region verifier.
@@ -1692,7 +1701,12 @@ LogicalResult verifyRegionTrait(Operation *op) {
 /// given operation.
 template <typename... Ts>
 LogicalResult verifyRegionTraits(Operation *op) {
-  return success((succeeded(verifyRegionTrait<Ts>(op)) && ...));
+  // Avoid instantiating an identical no-op verifier for each absent hook.
+  return success(
+      (succeeded(
+           verifyRegionTrait<std::conditional_t<
+               detect_has_verify_region_trait<Ts>::value, Ts, void>>(op)) &&
+       ...));
 }
 } // namespace op_definition_impl
 
@@ -2135,10 +2149,9 @@ class Op : public OpState, public Traits<ConcreteType>... {
   }
 
   static constexpr bool hasNoDataMembers() {
-    // Checking that the derived class does not define any member by comparing
-    // its size to an ad-hoc EmptyOp.
-    class EmptyOp : public Op<EmptyOp, Traits...> {};
-    return sizeof(ConcreteType) == sizeof(EmptyOp);
+    // Compare with the existing base instead of instantiating the traits again
+    // for an otherwise unused operation type.
+    return sizeof(ConcreteType) == sizeof(Op);
   }
 
   /// Allow access to internal implementation methods.
diff --git a/mlir/include/mlir/Support/InterfaceSupport.h b/mlir/include/mlir/Support/InterfaceSupport.h
index 8ab691454f64b8..3d3690503f33ba 100644
--- a/mlir/include/mlir/Support/InterfaceSupport.h
+++ b/mlir/include/mlir/Support/InterfaceSupport.h
@@ -171,17 +171,9 @@ class InterfaceMap {
 
 public:
   InterfaceMap() = default;
-  InterfaceMap(InterfaceMap &&) = default;
-  InterfaceMap &operator=(InterfaceMap &&rhs) {
-    for (auto &it : interfaces)
-      free(it.second);
-    interfaces = std::move(rhs.interfaces);
-    return *this;
-  }
-  ~InterfaceMap() {
-    for (auto &it : interfaces)
-      free(it.second);
-  }
+  InterfaceMap(InterfaceMap &&);
+  InterfaceMap &operator=(InterfaceMap &&rhs);
+  ~InterfaceMap();
 
   /// Construct an InterfaceMap with the given set of template types. For
   /// convenience given that object trait lists may contain other non-interface
@@ -195,7 +187,11 @@ class InterfaceMap {
       return InterfaceMap();
     } else {
       InterfaceMap map;
-      map.insertPotentialInterfaces<Types...>();
+      // A comma fold registers interfaces in source order. All non-interface
+      // traits share the same no-op specialization.
+      (map.insertPotentialInterface<std::conditional_t<
+           detect_get_interface_id<Types>::value, Types, void>>(),
+       ...);
       return map;
     }
   }
@@ -217,13 +213,6 @@ class InterfaceMap {
   }
 
 private:
-  /// Insert the given interface types in source order. A comma fold is
-  /// sequenced left to right in C++17.
-  template <typename... Types>
-  void insertPotentialInterfaces() {
-    (insertPotentialInterface<Types>(), ...);
-  }
-
   /// Insert the given interface type into the map, ignoring it if it doesn't
   /// actually represent an interface.
   template <typename T>
diff --git a/mlir/lib/Support/InterfaceSupport.cpp b/mlir/lib/Support/InterfaceSupport.cpp
index 4f321457dd2c88..32312c1abbfc67 100644
--- a/mlir/lib/Support/InterfaceSupport.cpp
+++ b/mlir/lib/Support/InterfaceSupport.cpp
@@ -18,6 +18,20 @@
 
 using namespace mlir;
 
+detail::InterfaceMap::InterfaceMap(InterfaceMap &&) = default;
+
+detail::InterfaceMap &detail::InterfaceMap::operator=(InterfaceMap &&rhs) {
+  for (auto &it : interfaces)
+    free(it.second);
+  interfaces = std::move(rhs.interfaces);
+  return *this;
+}
+
+detail::InterfaceMap::~InterfaceMap() {
+  for (auto &it : interfaces)
+    free(it.second);
+}
+
 void detail::InterfaceMap::insert(TypeID interfaceId, void *conceptImpl) {
   // Insert directly into the right position to keep the interfaces sorted.
   auto *it =

``````````

</details>


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