[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo
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
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
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
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
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
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
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
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
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
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::