<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 4, 2015 at 2:58 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2015-Feb-04, at 14:54, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Wed, Feb 4, 2015 at 2:08 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> Author: dexonsmith<br>
> Date: Wed Feb  4 16:08:30 2015<br>
> New Revision: 228212<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=228212&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=228212&view=rev</a><br>
> Log:<br>
> IR: Reduce boilerplate in DenseMapInfo overrides, NFC<br>
><br>
> Minimize the boilerplate required for the `MDNode` subclass<br>
> `DenseMapInfo<>` overrides in `LLVMContextImpl`.<br>
><br>
> Modified:<br>
>     llvm/trunk/lib/IR/LLVMContextImpl.h<br>
><br>
> Modified: llvm/trunk/lib/IR/LLVMContextImpl.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=228212&r1=228211&r2=228212&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=228212&r1=228211&r2=228212&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/LLVMContextImpl.h (original)<br>
> +++ llvm/trunk/lib/IR/LLVMContextImpl.h Wed Feb  4 16:08:30 2015<br>
> @@ -179,7 +179,7 @@ protected:<br>
>        : RawOps(Ops), Hash(calculateHash(Ops)) {}<br>
><br>
>    template <class NodeTy><br>
> -  MDNodeOpsKey(NodeTy *N, unsigned Offset = 0)<br>
> +  MDNodeOpsKey(const NodeTy *N, unsigned Offset = 0)<br>
>        : Ops(N->op_begin() + Offset, N->op_end()), Hash(N->getHash()) {}<br>
><br>
>    template <class NodeTy><br>
> @@ -208,125 +208,97 @@ public:<br>
>    unsigned getHash() const { return Hash; }<br>
>  };<br>
><br>
> +template <class NodeTy> struct MDNodeKeyImpl;<br>
> +template <class NodeTy> struct MDNodeInfo;<br>
> +<br>
>  /// \brief DenseMapInfo for MDTuple.<br>
>  ///<br>
>  /// Note that we don't need the is-function-local bit, since that's implicit in<br>
>  /// the operands.<br>
> -struct MDTupleInfo {<br>
> -  struct KeyTy : MDNodeOpsKey {<br>
> -    KeyTy(ArrayRef<Metadata *> Ops) : MDNodeOpsKey(Ops) {}<br>
> -    KeyTy(MDTuple *N) : MDNodeOpsKey(N) {}<br>
> -<br>
> -    bool operator==(const MDTuple *RHS) const {<br>
> -      if (RHS == getEmptyKey() || RHS == getTombstoneKey())<br>
> -        return false;<br>
> -      return compareOps(RHS);<br>
> -    }<br>
> -<br>
> -    static unsigned calculateHash(MDTuple *N) {<br>
> -      return MDNodeOpsKey::calculateHash(N);<br>
> -    }<br>
> -  };<br>
> -  static inline MDTuple *getEmptyKey() {<br>
> -    return DenseMapInfo<MDTuple *>::getEmptyKey();<br>
> -  }<br>
> -  static inline MDTuple *getTombstoneKey() {<br>
> -    return DenseMapInfo<MDTuple *>::getTombstoneKey();<br>
> -  }<br>
> -  static unsigned getHashValue(const KeyTy &Key) { return Key.getHash(); }<br>
> -  static unsigned getHashValue(const MDTuple *U) { return U->getHash(); }<br>
> -  static bool isEqual(const KeyTy &LHS, const MDTuple *RHS) {<br>
> -    return LHS == RHS;<br>
> -  }<br>
> -  static bool isEqual(const MDTuple *LHS, const MDTuple *RHS) {<br>
> -    return LHS == RHS;<br>
> +template <> struct MDNodeKeyImpl<MDTuple> : MDNodeOpsKey {<br>
> +  MDNodeKeyImpl(ArrayRef<Metadata *> Ops) : MDNodeOpsKey(Ops) {}<br>
> +  MDNodeKeyImpl(const MDTuple *N) : MDNodeOpsKey(N) {}<br>
> +<br>
> +  bool operator==(const MDTuple *RHS) const { return compareOps(RHS); }<br>
><br>
> The assymetry here is a bit odd. Probably nicer to give it a name than call it operator== when it's not symmetric (compares distinct types, doesn't work if you compare them in the opposite order, etc - or should the RHS just be MDNodeKeyImpl and rely on implicit conversion through the converting ctor (still better as a (possibly friended) non-member to make RHS conversions and LHS conversions equal/possible)<br>
<br>
</div></div>Any ideas for a function name?  I feel like `equals()` has the same<br>
problem.<br>
<br>
Maybe `matches()`?<br></blockquote><div><br>Sounds plausible. Worth avoiding the:<br><br>friend bool operator==(const MDNodeKeyImpl &A, const MDNodeKeyImpl &B) { A.compareOps(B); } ?<br><br>(or is there any reason not to just use/call compareOps directly? But I'm not sure where that function comes from/haven't stared at the DenseMap Key info details long enough to understand how they work)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> +<br>
> +  unsigned getHashValue() const { return getHash(); }<br>
> +<br>
> +  static unsigned calculateHash(MDTuple *N) {<br>
> +    return MDNodeOpsKey::calculateHash(N);<br>
>    }<br>
>  };<br>
><br>
>  /// \brief DenseMapInfo for MDLocation.<br>
> -struct MDLocationInfo {<br>
> -  struct KeyTy {<br>
> -    unsigned Line;<br>
> -    unsigned Column;<br>
> -    Metadata *Scope;<br>
> -    Metadata *InlinedAt;<br>
> -<br>
> -    KeyTy(unsigned Line, unsigned Column, Metadata *Scope, Metadata *InlinedAt)<br>
> -        : Line(Line), Column(Column), Scope(Scope), InlinedAt(InlinedAt) {}<br>
> -<br>
> -    KeyTy(const MDLocation *L)<br>
> -        : Line(L->getLine()), Column(L->getColumn()), Scope(L->getScope()),<br>
> -          InlinedAt(L->getInlinedAt()) {}<br>
> -<br>
> -    bool operator==(const MDLocation *RHS) const {<br>
> -      if (RHS == getEmptyKey() || RHS == getTombstoneKey())<br>
> -        return false;<br>
> -      return Line == RHS->getLine() && Column == RHS->getColumn() &&<br>
> -             Scope == RHS->getScope() && InlinedAt == RHS->getInlinedAt();<br>
> -    }<br>
> -  };<br>
> -  static inline MDLocation *getEmptyKey() {<br>
> -    return DenseMapInfo<MDLocation *>::getEmptyKey();<br>
> -  }<br>
> -  static inline MDLocation *getTombstoneKey() {<br>
> -    return DenseMapInfo<MDLocation *>::getTombstoneKey();<br>
> -  }<br>
> -  static unsigned getHashValue(const KeyTy &Key) {<br>
> -    return hash_combine(Key.Line, Key.Column, Key.Scope, Key.InlinedAt);<br>
> -  }<br>
> -  static unsigned getHashValue(const MDLocation *U) {<br>
> -    return getHashValue(KeyTy(U));<br>
> +template <> struct MDNodeKeyImpl<MDLocation> {<br>
> +  unsigned Line;<br>
> +  unsigned Column;<br>
> +  Metadata *Scope;<br>
> +  Metadata *InlinedAt;<br>
> +<br>
> +  MDNodeKeyImpl(unsigned Line, unsigned Column, Metadata *Scope,<br>
> +                Metadata *InlinedAt)<br>
> +      : Line(Line), Column(Column), Scope(Scope), InlinedAt(InlinedAt) {}<br>
> +<br>
> +  MDNodeKeyImpl(const MDLocation *L)<br>
> +      : Line(L->getLine()), Column(L->getColumn()), Scope(L->getScope()),<br>
> +        InlinedAt(L->getInlinedAt()) {}<br>
> +<br>
> +  bool operator==(const MDLocation *RHS) const {<br>
> +    return Line == RHS->getLine() && Column == RHS->getColumn() &&<br>
> +           Scope == RHS->getScope() && InlinedAt == RHS->getInlinedAt();<br>
>    }<br>
> -  static bool isEqual(const KeyTy &LHS, const MDLocation *RHS) {<br>
> -    return LHS == RHS;<br>
> -  }<br>
> -  static bool isEqual(const MDLocation *LHS, const MDLocation *RHS) {<br>
> -    return LHS == RHS;<br>
> +  unsigned getHashValue() const {<br>
> +    return hash_combine(Line, Column, Scope, InlinedAt);<br>
>    }<br>
>  };<br>
><br>
>  /// \brief DenseMapInfo for GenericDebugNode.<br>
> -struct GenericDebugNodeInfo {<br>
> -  struct KeyTy : MDNodeOpsKey {<br>
> -    unsigned Tag;<br>
> -    StringRef Header;<br>
> -    KeyTy(unsigned Tag, StringRef Header, ArrayRef<Metadata *> DwarfOps)<br>
> -        : MDNodeOpsKey(DwarfOps), Tag(Tag), Header(Header) {}<br>
> -    KeyTy(GenericDebugNode *N)<br>
> -        : MDNodeOpsKey(N, 1), Tag(N->getTag()), Header(N->getHeader()) {}<br>
> -<br>
> -    bool operator==(const GenericDebugNode *RHS) const {<br>
> -      if (RHS == getEmptyKey() || RHS == getTombstoneKey())<br>
> -        return false;<br>
> -      return Tag == RHS->getTag() && Header == RHS->getHeader() &&<br>
> -             compareOps(RHS, 1);<br>
> -    }<br>
> -<br>
> -    static unsigned calculateHash(GenericDebugNode *N) {<br>
> -      return MDNodeOpsKey::calculateHash(N, 1);<br>
> -    }<br>
> -  };<br>
> -  static inline GenericDebugNode *getEmptyKey() {<br>
> -    return DenseMapInfo<GenericDebugNode *>::getEmptyKey();<br>
> -  }<br>
> -  static inline GenericDebugNode *getTombstoneKey() {<br>
> -    return DenseMapInfo<GenericDebugNode *>::getTombstoneKey();<br>
> +template <> struct MDNodeKeyImpl<GenericDebugNode> : MDNodeOpsKey {<br>
> +  unsigned Tag;<br>
> +  StringRef Header;<br>
> +  MDNodeKeyImpl(unsigned Tag, StringRef Header, ArrayRef<Metadata *> DwarfOps)<br>
> +      : MDNodeOpsKey(DwarfOps), Tag(Tag), Header(Header) {}<br>
> +  MDNodeKeyImpl(const GenericDebugNode *N)<br>
> +      : MDNodeOpsKey(N, 1), Tag(N->getTag()), Header(N->getHeader()) {}<br>
> +<br>
> +  bool operator==(const GenericDebugNode *RHS) const {<br>
> +    return Tag == RHS->getTag() && Header == RHS->getHeader() &&<br>
> +           compareOps(RHS, 1);<br>
>    }<br>
> -  static unsigned getHashValue(const KeyTy &Key) {<br>
> -    return hash_combine(Key.getHash(), Key.Tag, Key.Header);<br>
> +<br>
> +  unsigned getHashValue() const { return hash_combine(getHash(), Tag, Header); }<br>
> +<br>
> +  static unsigned calculateHash(GenericDebugNode *N) {<br>
> +    return MDNodeOpsKey::calculateHash(N, 1);<br>
>    }<br>
> -  static unsigned getHashValue(const GenericDebugNode *U) {<br>
> -    return hash_combine(U->getHash(), U->getTag(), U->getHeader());<br>
> +};<br>
> +<br>
> +/// \brief DenseMapInfo for MDNode subclasses.<br>
> +template <class NodeTy> struct MDNodeInfo {<br>
> +  typedef MDNodeKeyImpl<NodeTy> KeyTy;<br>
> +  static inline NodeTy *getEmptyKey() {<br>
> +    return DenseMapInfo<NodeTy *>::getEmptyKey();<br>
> +  }<br>
> +  static inline NodeTy *getTombstoneKey() {<br>
> +    return DenseMapInfo<NodeTy *>::getTombstoneKey();<br>
> +  }<br>
> +  static unsigned getHashValue(const KeyTy &Key) { return Key.getHashValue(); }<br>
> +  static unsigned getHashValue(const NodeTy *N) {<br>
> +    return KeyTy(N).getHashValue();<br>
>    }<br>
> -  static bool isEqual(const KeyTy &LHS, const GenericDebugNode *RHS) {<br>
> +  static bool isEqual(const KeyTy &LHS, const NodeTy *RHS) {<br>
> +    if (RHS == getEmptyKey() || RHS == getTombstoneKey())<br>
> +      return false;<br>
>      return LHS == RHS;<br>
>    }<br>
> -  static bool isEqual(const GenericDebugNode *LHS,<br>
> -                      const GenericDebugNode *RHS) {<br>
> +  static bool isEqual(const NodeTy *LHS, const NodeTy *RHS) {<br>
>      return LHS == RHS;<br>
>    }<br>
>  };<br>
><br>
> +#define HANDLE_MDNODE_LEAF(CLASS) typedef MDNodeInfo<CLASS> CLASS##Info;<br>
> +#include "llvm/IR/Metadata.def"<br>
> +<br>
>  class LLVMContextImpl {<br>
>  public:<br>
>    /// OwnedModules - The set of modules instantiated in this context, and which<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>