[PDLL] Collapse TypeDetail.h into Types.h

[keith]

This allows this code to be compiled with MLIR_USE_FALLBACK_TYPE_IDS=1
which is used to solve TypeID issues across different DSOs. This Types.h
is only included in a few tools so the impact of this header being
larger should be limited.

[llvmbot]

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Keith Smiley (keith)

Changes

This allows this code to be compiled with MLIR_USE_FALLBACK_TYPE_IDS=1
which is used to solve TypeID issues across different DSOs. This Types.h
is only included in a few tools so the impact of this header being
larger should be limited.

---

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

5 Files Affected:

• (modified) mlir/include/mlir/Tools/PDLL/AST/Types.h (+121-11)
• (modified) mlir/lib/Tools/PDLL/AST/Context.cpp (+1-1)
• (removed) mlir/lib/Tools/PDLL/AST/TypeDetail.h (-141)
• (modified) mlir/lib/Tools/PDLL/AST/Types.cpp (-1)
• (modified) utils/bazel/llvm-project-overlay/mlir/BUILD.bazel (+1-6)

diff --git a/mlir/include/mlir/Tools/PDLL/AST/Types.h b/mlir/include/mlir/Tools/PDLL/AST/Types.h
index 538ea7c61b447..da74c50ac2797 100644
--- a/mlir/include/mlir/Tools/PDLL/AST/Types.h
+++ b/mlir/include/mlir/Tools/PDLL/AST/Types.h
@@ -22,17 +22,6 @@ class Operation;
 namespace ast {
 class Context;
 
-namespace detail {
-struct AttributeTypeStorage;
-struct ConstraintTypeStorage;
-struct OperationTypeStorage;
-struct RangeTypeStorage;
-struct RewriteTypeStorage;
-struct TupleTypeStorage;
-struct TypeTypeStorage;
-struct ValueTypeStorage;
-} // namespace detail
-
 //===----------------------------------------------------------------------===//
 // Type
 //===----------------------------------------------------------------------===//
@@ -99,6 +88,127 @@ inline raw_ostream &operator<<(raw_ostream &os, Type type) {
   return os;
 }
 
+//===----------------------------------------------------------------------===//
+// Type::Storage
+//===----------------------------------------------------------------------===//
+
+struct Type::Storage : public StorageUniquer::BaseStorage {
+  Storage(TypeID typeID) : typeID(typeID) {}
+
+  /// The type identifier for the derived type class.
+  TypeID typeID;
+};
+
+namespace detail {
+
+/// A utility CRTP base class that defines many of the necessary utilities for
+/// defining a PDLL AST Type.
+template <typename ConcreteT, typename KeyT = void>
+struct TypeStorageBase : public Type::Storage {
+  using KeyTy = KeyT;
+  using Base = TypeStorageBase<ConcreteT, KeyT>;
+  TypeStorageBase(KeyTy key)
+      : Type::Storage(TypeID::get<ConcreteT>()), key(key) {}
+
+  /// Construct an instance with the given storage allocator.
+  static ConcreteT *construct(StorageUniquer::StorageAllocator &alloc,
+                              const KeyTy &key) {
+    return new (alloc.allocate<ConcreteT>()) ConcreteT(key);
+  }
+
+  /// Utility methods required by the storage allocator.
+  bool operator==(const KeyTy &key) const { return this->key == key; }
+
+  /// Return the key value of this storage class.
+  const KeyTy &getValue() const { return key; }
+
+protected:
+  KeyTy key;
+};
+/// A specialization of the storage base for singleton types.
+template <typename ConcreteT>
+struct TypeStorageBase<ConcreteT, void> : public Type::Storage {
+  using Base = TypeStorageBase<ConcreteT, void>;
+  TypeStorageBase() : Type::Storage(TypeID::get<ConcreteT>()) {}
+};
+
+//===----------------------------------------------------------------------===//
+// AttributeTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct AttributeTypeStorage : public TypeStorageBase<AttributeTypeStorage> {};
+
+//===----------------------------------------------------------------------===//
+// ConstraintTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct ConstraintTypeStorage : public TypeStorageBase<ConstraintTypeStorage> {};
+
+//===----------------------------------------------------------------------===//
+// OperationTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct OperationTypeStorage
+    : public TypeStorageBase<OperationTypeStorage,
+                             std::pair<StringRef, const ods::Operation *>> {
+  using Base::Base;
+
+  static OperationTypeStorage *
+  construct(StorageUniquer::StorageAllocator &alloc,
+            const std::pair<StringRef, const ods::Operation *> &key) {
+    return new (alloc.allocate<OperationTypeStorage>()) OperationTypeStorage(
+        std::make_pair(alloc.copyInto(key.first), key.second));
+  }
+};
+
+//===----------------------------------------------------------------------===//
+// RangeTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct RangeTypeStorage : public TypeStorageBase<RangeTypeStorage, Type> {
+  using Base::Base;
+};
+
+//===----------------------------------------------------------------------===//
+// RewriteTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct RewriteTypeStorage : public TypeStorageBase<RewriteTypeStorage> {};
+
+//===----------------------------------------------------------------------===//
+// TupleTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct TupleTypeStorage
+    : public TypeStorageBase<TupleTypeStorage,
+                             std::pair<ArrayRef<Type>, ArrayRef<StringRef>>> {
+  using Base::Base;
+
+  static TupleTypeStorage *
+  construct(StorageUniquer::StorageAllocator &alloc,
+            std::pair<ArrayRef<Type>, ArrayRef<StringRef>> key) {
+    SmallVector<StringRef> names = llvm::to_vector(llvm::map_range(
+        key.second, [&](StringRef name) { return alloc.copyInto(name); }));
+    return new (alloc.allocate<TupleTypeStorage>())
+        TupleTypeStorage(std::make_pair(alloc.copyInto(key.first),
+                                        alloc.copyInto(llvm::ArrayRef(names))));
+  }
+};
+
+//===----------------------------------------------------------------------===//
+// TypeTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct TypeTypeStorage : public TypeStorageBase<TypeTypeStorage> {};
+
+//===----------------------------------------------------------------------===//
+// ValueTypeStorage
+//===----------------------------------------------------------------------===//
+
+struct ValueTypeStorage : public TypeStorageBase<ValueTypeStorage> {};
+
+} // namespace detail
+
 //===----------------------------------------------------------------------===//
 // AttributeType
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Tools/PDLL/AST/Context.cpp b/mlir/lib/Tools/PDLL/AST/Context.cpp
index 6f2e4cd58082b..e82807f6322b0 100644
--- a/mlir/lib/Tools/PDLL/AST/Context.cpp
+++ b/mlir/lib/Tools/PDLL/AST/Context.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Tools/PDLL/AST/Context.h"
-#include "TypeDetail.h"
+#include "mlir/Tools/PDLL/AST/Types.h"
 
 using namespace mlir;
 using namespace mlir::pdll::ast;
diff --git a/mlir/lib/Tools/PDLL/AST/TypeDetail.h b/mlir/lib/Tools/PDLL/AST/TypeDetail.h
deleted file mode 100644
index a0bd84eacc4a2..0000000000000
--- a/mlir/lib/Tools/PDLL/AST/TypeDetail.h
+++ /dev/null
@@ -1,141 +0,0 @@
-//===- TypeDetail.h ---------------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LIB_MLIR_TOOLS_PDLL_AST_TYPEDETAIL_H_
-#define LIB_MLIR_TOOLS_PDLL_AST_TYPEDETAIL_H_
-
-#include "mlir/Tools/PDLL/AST/Types.h"
-
-namespace mlir {
-namespace pdll {
-namespace ast {
-//===----------------------------------------------------------------------===//
-// Type
-//===----------------------------------------------------------------------===//
-
-struct Type::Storage : public StorageUniquer::BaseStorage {
-  Storage(TypeID typeID) : typeID(typeID) {}
-
-  /// The type identifier for the derived type class.
-  TypeID typeID;
-};
-
-namespace detail {
-
-/// A utility CRTP base class that defines many of the necessary utilities for
-/// defining a PDLL AST Type.
-template <typename ConcreteT, typename KeyT = void>
-struct TypeStorageBase : public Type::Storage {
-  using KeyTy = KeyT;
-  using Base = TypeStorageBase<ConcreteT, KeyT>;
-  TypeStorageBase(KeyTy key)
-      : Type::Storage(TypeID::get<ConcreteT>()), key(key) {}
-
-  /// Construct an instance with the given storage allocator.
-  static ConcreteT *construct(StorageUniquer::StorageAllocator &alloc,
-                              const KeyTy &key) {
-    return new (alloc.allocate<ConcreteT>()) ConcreteT(key);
-  }
-
-  /// Utility methods required by the storage allocator.
-  bool operator==(const KeyTy &key) const { return this->key == key; }
-
-  /// Return the key value of this storage class.
-  const KeyTy &getValue() const { return key; }
-
-protected:
-  KeyTy key;
-};
-/// A specialization of the storage base for singleton types.
-template <typename ConcreteT>
-struct TypeStorageBase<ConcreteT, void> : public Type::Storage {
-  using Base = TypeStorageBase<ConcreteT, void>;
-  TypeStorageBase() : Type::Storage(TypeID::get<ConcreteT>()) {}
-};
-
-//===----------------------------------------------------------------------===//
-// AttributeType
-//===----------------------------------------------------------------------===//
-
-struct AttributeTypeStorage : public TypeStorageBase<AttributeTypeStorage> {};
-
-//===----------------------------------------------------------------------===//
-// ConstraintType
-//===----------------------------------------------------------------------===//
-
-struct ConstraintTypeStorage : public TypeStorageBase<ConstraintTypeStorage> {};
-
-//===----------------------------------------------------------------------===//
-// OperationType
-//===----------------------------------------------------------------------===//
-
-struct OperationTypeStorage
-    : public TypeStorageBase<OperationTypeStorage,
-                             std::pair<StringRef, const ods::Operation *>> {
-  using Base::Base;
-
-  static OperationTypeStorage *
-  construct(StorageUniquer::StorageAllocator &alloc,
-            const std::pair<StringRef, const ods::Operation *> &key) {
-    return new (alloc.allocate<OperationTypeStorage>()) OperationTypeStorage(
-        std::make_pair(alloc.copyInto(key.first), key.second));
-  }
-};
-
-//===----------------------------------------------------------------------===//
-// RangeType
-//===----------------------------------------------------------------------===//
-
-struct RangeTypeStorage : public TypeStorageBase<RangeTypeStorage, Type> {
-  using Base::Base;
-};
-
-//===----------------------------------------------------------------------===//
-// RewriteType
-//===----------------------------------------------------------------------===//
-
-struct RewriteTypeStorage : public TypeStorageBase<RewriteTypeStorage> {};
-
-//===----------------------------------------------------------------------===//
-// TupleType
-//===----------------------------------------------------------------------===//
-
-struct TupleTypeStorage
-    : public TypeStorageBase<TupleTypeStorage,
-                             std::pair<ArrayRef<Type>, ArrayRef<StringRef>>> {
-  using Base::Base;
-
-  static TupleTypeStorage *
-  construct(StorageUniquer::StorageAllocator &alloc,
-            std::pair<ArrayRef<Type>, ArrayRef<StringRef>> key) {
-    SmallVector<StringRef> names = llvm::to_vector(llvm::map_range(
-        key.second, [&](StringRef name) { return alloc.copyInto(name); }));
-    return new (alloc.allocate<TupleTypeStorage>())
-        TupleTypeStorage(std::make_pair(alloc.copyInto(key.first),
-                                        alloc.copyInto(llvm::ArrayRef(names))));
-  }
-};
-
-//===----------------------------------------------------------------------===//
-// TypeType
-//===----------------------------------------------------------------------===//
-
-struct TypeTypeStorage : public TypeStorageBase<TypeTypeStorage> {};
-
-//===----------------------------------------------------------------------===//
-// ValueType
-//===----------------------------------------------------------------------===//
-
-struct ValueTypeStorage : public TypeStorageBase<ValueTypeStorage> {};
-
-} // namespace detail
-} // namespace ast
-} // namespace pdll
-} // namespace mlir
-
-#endif // LIB_MLIR_TOOLS_PDLL_AST_TYPEDETAIL_H_
diff --git a/mlir/lib/Tools/PDLL/AST/Types.cpp b/mlir/lib/Tools/PDLL/AST/Types.cpp
index 1468ac2a280d5..d5497b06ba935 100644
--- a/mlir/lib/Tools/PDLL/AST/Types.cpp
+++ b/mlir/lib/Tools/PDLL/AST/Types.cpp
@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Tools/PDLL/AST/Types.h"
-#include "TypeDetail.h"
 #include "mlir/Tools/PDLL/AST/Context.h"
 #include <optional>
 
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index 6d2eedbfe2415..87fd9d7b48e5a 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -14085,12 +14085,7 @@ cc_library(
 
 cc_library(
     name = "PDLLAST",
-    srcs = glob(
-        [
-            "lib/Tools/PDLL/AST/*.cpp",
-            "lib/Tools/PDLL/AST/*.h",
-        ],
-    ),
+    srcs = glob(["lib/Tools/PDLL/AST/*.cpp"]),
     hdrs = glob(["include/mlir/Tools/PDLL/AST/*.h"]),
     includes = ["include"],
     deps = [

[keith]

happy to fixup anything that reviewers spot!