[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-11 Thread River Riddle via Phabricator via cfe-commits
rriddle created this revision.
rriddle added reviewers: dblaikie, mehdi_amini, lattner, silvas.
Herald added subscribers: wenzhicui, wrengr, Chia-hungDuan, dcaballe, cota, 
teijeong, dexonsmith, rdzhabarov, tatianashp, ThomasRaoux, jdoerfert, 
AlexeySotkin, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, 
aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, mravishankar, 
bollu, sbc100.
Herald added a reviewer: antiagainst.
Herald added a reviewer: ftynse.
rriddle requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, stephenneuendorffer, 
nicolasvasilache, aheejin.
Herald added projects: clang, MLIR, LLVM.

This allows for using SFINAE partial specialization for DenseMapInfo.
In MLIR, this is particularly useful as it will allow for defining partial
specializations that support all Attribute, Op, and Type classes without
needing to specialize DenseMapInfo for each individual class.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113641

Files:
  clang/include/clang/AST/TypeOrdering.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/Sema.h
  llvm/include/llvm/ADT/APInt.h
  llvm/include/llvm/ADT/ArrayRef.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/Hashing.h
  llvm/include/llvm/ADT/ImmutableList.h
  llvm/include/llvm/ADT/PointerIntPair.h
  llvm/include/llvm/ADT/StringRef.h
  llvm/include/llvm/BinaryFormat/WasmTraits.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/unittests/ADT/DenseMapTest.cpp
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVOps.h
  mlir/include/mlir/IR/Attributes.h
  mlir/include/mlir/IR/BuiltinOps.h
  mlir/include/mlir/IR/OpDefinition.h
  mlir/include/mlir/IR/Types.h
  mlir/include/mlir/Support/LLVM.h

Index: mlir/include/mlir/Support/LLVM.h
===
--- mlir/include/mlir/Support/LLVM.h
+++ mlir/include/mlir/Support/LLVM.h
@@ -46,7 +46,8 @@
 } // namespace detail
 template 
 class DenseMap;
-template  struct DenseMapInfo;
+template 
+struct DenseMapInfo;
 template  class DenseSet;
 class MallocAllocator;
 template  class MutableArrayRef;
@@ -90,7 +91,8 @@
 //
 // Containers.
 using llvm::ArrayRef;
-using llvm::DenseMapInfo;
+template 
+using DenseMapInfo = llvm::DenseMapInfo;
 template ,
   typename BucketT = llvm::detail::DenseMapPair>
Index: mlir/include/mlir/IR/Types.h
===
--- mlir/include/mlir/IR/Types.h
+++ mlir/include/mlir/IR/Types.h
@@ -269,6 +269,18 @@
   static unsigned getHashValue(mlir::Type val) { return mlir::hash_value(val); }
   static bool isEqual(mlir::Type LHS, mlir::Type RHS) { return LHS == RHS; }
 };
+template 
+struct DenseMapInfo::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+};
 
 /// We align TypeStorage by 8, so allow LLVM to steal the low bits.
 template <> struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/OpDefinition.h
===
--- mlir/include/mlir/IR/OpDefinition.h
+++ mlir/include/mlir/IR/OpDefinition.h
@@ -1906,4 +1906,25 @@
 } // namespace impl
 } // end namespace mlir
 
+namespace llvm {
+
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>> {
+  static inline T getEmptyKey() {
+auto *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static inline T getTombstoneKey() {
+auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static unsigned getHashValue(T val) {
+return hash_value(val.getAsOpaquePointer());
+  }
+  static bool isEqual(T lhs, T rhs) { return lhs == rhs; }
+};
+
+} // end namespace llvm
+
 #endif
Index: mlir/include/mlir/IR/BuiltinOps.h
===
--- mlir/include/mlir/IR/BuiltinOps.h
+++ mlir/include/mlir/IR/BuiltinOps.h
@@ -49,23 +49,6 @@
 } // end namespace mlir
 
 namespace llvm {
-// Functions hash just like pointers.
-template <>
-struct DenseMapInfo {
-  static mlir::FuncOp getEmptyKey() {
-auto *pointer = llvm::DenseMapInfo::getEmptyKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static mlir::FuncOp getTombstoneKey() {
-auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static unsigned getHashValue(mlir::FuncOp val) {
-return hash_value(val.getAsOpaquePointer());
-  }
-  static bool isEqual(mlir::FuncOp lhs, mlir::FuncOp rhs) { return lhs == rhs; }
-};
-
 /// Allow stealing the low bi

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-11 Thread River Riddle via Phabricator via cfe-commits
rriddle added a comment.

Given the new template parameter, I needed to update the forward declarations. 
Some of them already had DenseMapInfo from an include, so I just dropped them. 
Some didn't, so I opted to just add an include for DenseMapInfo (seemed small 
enough). I could avoid the includes by adding an explicit `void` for the second 
template parameter. Let me know if there is a preference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-11 Thread Uday Bondhugula via Phabricator via cfe-commits
bondhugula added inline comments.



Comment at: llvm/include/llvm/ADT/DenseMapInfo.h:42
 
-template
-struct DenseMapInfo {
+template  struct DenseMapInfo {
   //static inline T getEmptyKey();

A code comment here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-11 Thread Sean Silva via Phabricator via cfe-commits
silvas accepted this revision.
silvas added a comment.
This revision is now accepted and ready to land.

Nice :) This looks pretty straightforward, but given how core this is, wait for 
another approver or two.




Comment at: llvm/include/llvm/ADT/DenseMapInfo.h:42
 
-template
-struct DenseMapInfo {
+template  struct DenseMapInfo {
   //static inline T getEmptyKey();

bondhugula wrote:
> A code comment here?
+1. Is there a doc that needs to be updated too? We should update the source of 
truth is for "here's how to use DenseMapInfo", if such documentation even 
exists :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread Chris Lattner via Phabricator via cfe-commits
lattner accepted this revision.
lattner added a comment.
Herald added a subscriber: sdasgup3.

Nice, I'm very excited about this - this will allow a lot of cleanups.  Thank 
you for doing this!




Comment at: llvm/include/llvm/ADT/Hashing.h:59
 namespace llvm {
-template  struct DenseMapInfo;
 

Is there a way to keep the forward declarations references here instead of 
#include?  DenseMapInfo.h pulls in a ton of stuff including  and 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread River Riddle via Phabricator via cfe-commits
rriddle added inline comments.



Comment at: llvm/include/llvm/ADT/Hashing.h:59
 namespace llvm {
-template  struct DenseMapInfo;
 

lattner wrote:
> Is there a way to keep the forward declarations references here instead of 
> #include?  DenseMapInfo.h pulls in a ton of stuff including  and 
> 
I mentioned it in a comment. If we don't want the includes, we'll need to 
sprinkle `void` everywhere that we specialize the template. (I don't have a 
preference either way)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread Chris Lattner via Phabricator via cfe-commits
lattner added a comment.

Oh, are you concerned about staging this in?  If you want to stage it (add the 
includes now, remove them later or something), that is totally fine with me.  
Maybe I don't understand the impact correctly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread Chris Lattner via Phabricator via cfe-commits
lattner added a comment.

I think a few void's probably won't hurt anyone?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-16 Thread River Riddle via Phabricator via cfe-commits
rriddle updated this revision to Diff 387386.
rriddle added a comment.
Herald added subscribers: lldb-commits, hiraditya.
Herald added a project: LLDB.

resolve comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

Files:
  clang/include/clang/AST/TypeOrdering.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/Sema.h
  lldb/include/lldb/Utility/ConstString.h
  llvm/include/llvm/ADT/APInt.h
  llvm/include/llvm/ADT/APSInt.h
  llvm/include/llvm/ADT/ArrayRef.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/Hashing.h
  llvm/include/llvm/ADT/ImmutableList.h
  llvm/include/llvm/ADT/PointerIntPair.h
  llvm/include/llvm/ADT/StringRef.h
  llvm/include/llvm/BinaryFormat/WasmTraits.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Support/APInt.cpp
  llvm/unittests/ADT/DenseMapTest.cpp
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVOps.h
  mlir/include/mlir/IR/Attributes.h
  mlir/include/mlir/IR/BuiltinOps.h
  mlir/include/mlir/IR/OpDefinition.h
  mlir/include/mlir/IR/Types.h
  mlir/include/mlir/Support/LLVM.h

Index: mlir/include/mlir/Support/LLVM.h
===
--- mlir/include/mlir/Support/LLVM.h
+++ mlir/include/mlir/Support/LLVM.h
@@ -46,7 +46,8 @@
 } // namespace detail
 template 
 class DenseMap;
-template  struct DenseMapInfo;
+template 
+struct DenseMapInfo;
 template  class DenseSet;
 class MallocAllocator;
 template  class MutableArrayRef;
@@ -90,7 +91,8 @@
 //
 // Containers.
 using llvm::ArrayRef;
-using llvm::DenseMapInfo;
+template 
+using DenseMapInfo = llvm::DenseMapInfo;
 template ,
   typename BucketT = llvm::detail::DenseMapPair>
Index: mlir/include/mlir/IR/Types.h
===
--- mlir/include/mlir/IR/Types.h
+++ mlir/include/mlir/IR/Types.h
@@ -269,6 +269,18 @@
   static unsigned getHashValue(mlir::Type val) { return mlir::hash_value(val); }
   static bool isEqual(mlir::Type LHS, mlir::Type RHS) { return LHS == RHS; }
 };
+template 
+struct DenseMapInfo::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+};
 
 /// We align TypeStorage by 8, so allow LLVM to steal the low bits.
 template <> struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/OpDefinition.h
===
--- mlir/include/mlir/IR/OpDefinition.h
+++ mlir/include/mlir/IR/OpDefinition.h
@@ -1906,4 +1906,25 @@
 } // namespace impl
 } // end namespace mlir
 
+namespace llvm {
+
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>> {
+  static inline T getEmptyKey() {
+auto *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static inline T getTombstoneKey() {
+auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static unsigned getHashValue(T val) {
+return hash_value(val.getAsOpaquePointer());
+  }
+  static bool isEqual(T lhs, T rhs) { return lhs == rhs; }
+};
+
+} // end namespace llvm
+
 #endif
Index: mlir/include/mlir/IR/BuiltinOps.h
===
--- mlir/include/mlir/IR/BuiltinOps.h
+++ mlir/include/mlir/IR/BuiltinOps.h
@@ -49,23 +49,6 @@
 } // end namespace mlir
 
 namespace llvm {
-// Functions hash just like pointers.
-template <>
-struct DenseMapInfo {
-  static mlir::FuncOp getEmptyKey() {
-auto *pointer = llvm::DenseMapInfo::getEmptyKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static mlir::FuncOp getTombstoneKey() {
-auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static unsigned getHashValue(mlir::FuncOp val) {
-return hash_value(val.getAsOpaquePointer());
-  }
-  static bool isEqual(mlir::FuncOp lhs, mlir::FuncOp rhs) { return lhs == rhs; }
-};
-
 /// Allow stealing the low bits of FuncOp.
 template <>
 struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/Attributes.h
===
--- mlir/include/mlir/IR/Attributes.h
+++ mlir/include/mlir/IR/Attributes.h
@@ -201,6 +201,19 @@
 return LHS == RHS;
   }
 };
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = llvm::DenseMapIn

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-16 Thread River Riddle via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c484f11d355: [llvm] Add a SFINAE template parameter to 
DenseMapInfo (authored by rriddle).

Changed prior to commit:
  https://reviews.llvm.org/D113641?vs=387386&id=387702#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113641/new/

https://reviews.llvm.org/D113641

Files:
  clang/include/clang/AST/TypeOrdering.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/Sema.h
  lldb/include/lldb/Utility/ConstString.h
  llvm/include/llvm/ADT/APInt.h
  llvm/include/llvm/ADT/APSInt.h
  llvm/include/llvm/ADT/ArrayRef.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/Hashing.h
  llvm/include/llvm/ADT/ImmutableList.h
  llvm/include/llvm/ADT/PointerIntPair.h
  llvm/include/llvm/ADT/StringRef.h
  llvm/include/llvm/BinaryFormat/WasmTraits.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Support/APInt.cpp
  llvm/unittests/ADT/DenseMapTest.cpp
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVOps.h
  mlir/include/mlir/IR/Attributes.h
  mlir/include/mlir/IR/BuiltinOps.h
  mlir/include/mlir/IR/OpDefinition.h
  mlir/include/mlir/IR/Types.h
  mlir/include/mlir/Support/LLVM.h

Index: mlir/include/mlir/Support/LLVM.h
===
--- mlir/include/mlir/Support/LLVM.h
+++ mlir/include/mlir/Support/LLVM.h
@@ -46,7 +46,7 @@
 } // namespace detail
 template 
 class DenseMap;
-template  struct DenseMapInfo;
+template  struct DenseMapInfo;
 template  class DenseSet;
 class MallocAllocator;
 template  class MutableArrayRef;
@@ -90,7 +90,8 @@
 //
 // Containers.
 using llvm::ArrayRef;
-using llvm::DenseMapInfo;
+template 
+using DenseMapInfo = llvm::DenseMapInfo;
 template ,
   typename BucketT = llvm::detail::DenseMapPair>
Index: mlir/include/mlir/IR/Types.h
===
--- mlir/include/mlir/IR/Types.h
+++ mlir/include/mlir/IR/Types.h
@@ -269,6 +269,18 @@
   static unsigned getHashValue(mlir::Type val) { return mlir::hash_value(val); }
   static bool isEqual(mlir::Type LHS, mlir::Type RHS) { return LHS == RHS; }
 };
+template 
+struct DenseMapInfo::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+};
 
 /// We align TypeStorage by 8, so allow LLVM to steal the low bits.
 template <> struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/OpDefinition.h
===
--- mlir/include/mlir/IR/OpDefinition.h
+++ mlir/include/mlir/IR/OpDefinition.h
@@ -1906,4 +1906,25 @@
 } // namespace impl
 } // end namespace mlir
 
+namespace llvm {
+
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>> {
+  static inline T getEmptyKey() {
+auto *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static inline T getTombstoneKey() {
+auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static unsigned getHashValue(T val) {
+return hash_value(val.getAsOpaquePointer());
+  }
+  static bool isEqual(T lhs, T rhs) { return lhs == rhs; }
+};
+
+} // end namespace llvm
+
 #endif
Index: mlir/include/mlir/IR/BuiltinOps.h
===
--- mlir/include/mlir/IR/BuiltinOps.h
+++ mlir/include/mlir/IR/BuiltinOps.h
@@ -49,23 +49,6 @@
 } // end namespace mlir
 
 namespace llvm {
-// Functions hash just like pointers.
-template <>
-struct DenseMapInfo {
-  static mlir::FuncOp getEmptyKey() {
-auto *pointer = llvm::DenseMapInfo::getEmptyKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static mlir::FuncOp getTombstoneKey() {
-auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static unsigned getHashValue(mlir::FuncOp val) {
-return hash_value(val.getAsOpaquePointer());
-  }
-  static bool isEqual(mlir::FuncOp lhs, mlir::FuncOp rhs) { return lhs == rhs; }
-};
-
 /// Allow stealing the low bits of FuncOp.
 template <>
 struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/Attributes.h
===
--- mlir/include/mlir/IR/Attributes.h
+++ mlir/include/mlir/IR/Attributes.h
@@ -201,6 +201,19 @@
 return LHS == RHS;
   }
 };
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::