[llvm] r225682 - IR: Split GenericMDNode into MDTuple and UniquableMDNode

Duncan P. N. Exon Smith dexonsmith at apple.com
Mon Jan 12 19:47:01 PST 2015


> On 2015-Jan-12, at 14:35, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
>> On Mon, Jan 12, 2015 at 2:21 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> > On 2015-Jan-12, at 13:02, David Blaikie <dblaikie at gmail.com> wrote:
>> >
>> >
>> >
>> > On Mon, Jan 12, 2015 at 12:09 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> > Author: dexonsmith
>> > Date: Mon Jan 12 14:09:34 2015
>> > New Revision: 225682
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=225682&view=rev
>> > Log:
>> > IR: Split GenericMDNode into MDTuple and UniquableMDNode
>> >
>> > Split `GenericMDNode` into two classes (with more descriptive names).
>> >
>> >   - `UniquableMDNode` will be a common subclass for `MDNode`s that are
>> >     sometimes uniqued like constants, and sometimes 'distinct'.
>> >
>> >     This class gets the (short-lived) RAUW support and related API.
>> >
>> >   - `MDTuple` is the basic tuple that has always been returned by
>> >     `MDNode::get()`.  This is as opposed to more specific nodes to be
>> >     added soon, which have additional fields, custom assembly syntax,
>> >     and extra semantics.
>> >
>> > I don't mean to hinder progress, etc - but now that we've made the MDNode/Value split, is the "make debug info special" argument still (sufficiently) valid?
>> 
>> That's my strong opinion :).
>> 
>> > Can we not make MDNodes sufficiently cheap while being general?
>> >
>> > (Answer may well be 'no', but I'm just curious/wrapping my head around everything)
>> >
>> 
>> Making debug info "special" is about more than memory/time costs,
>> although with LTO that's certainly a pressing concern.  Large parts of
>> the graph are still cyclic,
> 
> Which cycles are you referring to? I thought the point of DIRef stuff was to break the cycles to make linking/deduping better. Are there places that wasn't applied/could be?

Known cycles:

  - Type graph, all over the place, outside of C++ types subject to
    ODR (classes declared in anonymous namespaces can't be given
    unique names even in C++).

  - Subprograms and their "retained" variables point at each other.
    (This could be partially solved similarly to the type graph, by
    using DIRefs to point at subprograms.  But everything with
    local linkage will fall through the cracks.)

  - Structures/classes with a vtable (although as you pointed out,
    this is fixable via a DIRef).

>> which makes deserialization from bitcode
>> expensive (since it still needs RAUW).  With custom records, we'll be
>> able to avoid these cycles in serialization (serializing only one
>> direction) and recreate the links on the other side.  Using the same
>> mechanisms, we'll be able to avoid duplicating these cycles in
>> `MapMetadata()`.  We'll also have the flexibility to introduce
>> first-class support for linking, e.g., subprogram descriptions.
>> (Furthermore, there's a future down this path where upgrading debug info
>> from bitcode is actually practical (although my understanding is that
>> bitcode compatibility isn't particularly important to you).)
>> 
>> But I think the resulting assembly changes are most important for the
>> broader community (who may not be using LTO).  While in C++ you have the
>> `DI*` wrapper classes that allow you to understand and work with each
>> node type, the assembly may as well be a binary stream.  Comments
>> mitigate this, but they don't get updated in testcases when the debug
>> info scripts run, they don't describe all the fields, and they're not
>> actually all that great.  Unlike the rest of the IR (which is
>> self-describing), you need to have `llvm/IR/DebugInfo.h` open (or
>> memorized) to understand any of it.
> 
> Sure enough, I wouldn't mind more readable/writable debug info - though I'm not sure yet that it'd actually reach the point of writability & would sort of like tho discuss/think about that a bit more if that's the/a major motivation, though I'm not sure when we'll be at the point where that conversation is useful.

Regarding assembly syntax, I'm still aiming at my MDLocation llvmdev
post [1] (which you seemed to like ;), at least in principle [2]).

[1]: http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-October/078173.html
[2]: http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-October/078174.html

>> It's enough for me that (in optimized code) *most* of the IR (by volume)
>> is debug info.  IMO, not having custom node types is akin to only having
>> one type of instruction (although maybe that's an unfair straw man).
>> 
>> (FWIW, I'd be in favour of first-class-ifying other metadata schemas
>> too.  There's a reason we don't use `std::tuple<>` in place of all C++
>> data structures (another straw man; maybe I'm just ranting now!).)
> 
> Sure - my thought here is just that we could do this in a self-descriptive, extensible way rather than on a per-metadata one-at-a-time basis. (more like DWARF, really - self-descriptive with some kind of record description and then entries that mention what kind of record they are)

Right, I remember this idea now.

> But perhaps that's not important/unnecessarily general, I'm not sure.

It does seem unnecessarily general (and less flexible) to me (which
might be okay if it was easier to implement well, but I actually
think it's harder).  I also don't think the upsides are particularly
important.

Just to clarify what I mean by "flexible", obviously both ideas can
represent any schema.  What I mean is that to make a node type
behave a particular way (to have some set of semantics), with the
general version we have to first add "language features" that have
these semantics, and then say that some particular type has those
features.  With custom handling, you don't have this extra language.

I also have a hard time imagining how schema changes could ever be
upgraded in bitcode with generic schemas.  The bitcode upgrade
model is: read in the bitcode record, and if it's some sort of old
record, figure out how to represent it in the current IR.  This
wouldn't be hard to do with custom types.  I think it's pretty hard
though with generic schemas.

> [I guess one issue is maybe deciding what the end of this looks like and which benefits it has, etc, might dictate the path now/soon, I'm just not sure how soon]

I'm working on putting something together RSN, just working on a
redux of my MDLocation post [1] first.

> 
> - David
>  
> 
> >     This class gets the hash-related logic, since other sublcasses of
> >     `UniquableMDNode` may need to hash based on other fields.
> >
> > To keep this diff from getting too big, I've added casts to `MDTuple`
> > that won't really scale as new subclasses of `UniquableMDNode` are
> > added, but I'll clean those up incrementally.
> >
> > (No functionality change intended.)
> >
> > Modified:
> >     llvm/trunk/include/llvm/IR/Metadata.def
> >     llvm/trunk/include/llvm/IR/Metadata.h
> >     llvm/trunk/lib/AsmParser/LLParser.cpp
> >     llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> >     llvm/trunk/lib/IR/DIBuilder.cpp
> >     llvm/trunk/lib/IR/LLVMContextImpl.cpp
> >     llvm/trunk/lib/IR/LLVMContextImpl.h
> >     llvm/trunk/lib/IR/Metadata.cpp
> >     llvm/trunk/lib/IR/MetadataTracking.cpp
> >     llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
> >
> > Modified: llvm/trunk/include/llvm/IR/Metadata.def
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.def?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/include/llvm/IR/Metadata.def (original)
> > +++ llvm/trunk/include/llvm/IR/Metadata.def Mon Jan 12 14:09:34 2015
> > @@ -37,7 +37,8 @@ HANDLE_METADATA_LEAF(ConstantAsMetadata)
> >  HANDLE_METADATA_LEAF(LocalAsMetadata)
> >  HANDLE_METADATA_BRANCH(MDNode)
> >  HANDLE_METADATA_LEAF(MDNodeFwdDecl)
> > -HANDLE_METADATA_LEAF(GenericMDNode)
> > +HANDLE_METADATA_BRANCH(UniquableMDNode)
> > +HANDLE_METADATA_LEAF(MDTuple)
> >
> >  #undef HANDLE_METADATA
> >  #undef HANDLE_METADATA_LEAF
> >
> > Modified: llvm/trunk/include/llvm/IR/Metadata.h
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/include/llvm/IR/Metadata.h (original)
> > +++ llvm/trunk/include/llvm/IR/Metadata.h Mon Jan 12 14:09:34 2015
> > @@ -56,7 +56,7 @@ protected:
> >
> >  public:
> >    enum MetadataKind {
> > -    GenericMDNodeKind,
> > +    MDTupleKind,
> >      MDNodeFwdDeclKind,
> >      ConstantAsMetadataKind,
> >      LocalAsMetadataKind,
> > @@ -158,7 +158,7 @@ public:
> >    /// \brief Resolve all uses of this.
> >    ///
> >    /// Resolve all uses of this, turning off RAUW permanently.  If \c
> > -  /// ResolveUsers, call \a GenericMDNode::resolve() on any users whose last
> > +  /// ResolveUsers, call \a UniquableMDNode::resolve() on any users whose last
> >    /// operand is resolved.
> >    void resolveAllUses(bool ResolveUsers = true);
> >
> > @@ -682,7 +682,7 @@ public:
> >
> >    /// \brief Methods for support type inquiry through isa, cast, and dyn_cast:
> >    static bool classof(const Metadata *MD) {
> > -    return MD->getMetadataID() == GenericMDNodeKind ||
> > +    return MD->getMetadataID() == MDTupleKind ||
> >             MD->getMetadataID() == MDNodeFwdDeclKind;
> >    }
> >
> > @@ -698,46 +698,46 @@ public:
> >    static MDNode *getMostGenericRange(MDNode *A, MDNode *B);
> >  };
> >
> > -/// \brief Generic metadata node.
> > +/// \brief Uniquable metadata node.
> >  ///
> > -/// Generic metadata nodes, with opt-out support for uniquing.
> > -///
> > -/// Although nodes are uniqued by default, \a GenericMDNode has no support for
> > -/// RAUW.  If an operand change (due to RAUW or otherwise) causes a uniquing
> > -/// collision, the uniquing bit is dropped.
> > -class GenericMDNode : public MDNode {
> > -  friend class Metadata;
> > +/// A uniquable metadata node.  This contains the basic functionality
> > +/// for implementing sub-types of \a MDNode that can be uniqued like
> > +/// constants.
> > +///
> > +/// There is limited support for RAUW at construction time.  At
> > +/// construction time, if any operands are an instance of \a
> > +/// MDNodeFwdDecl (or another unresolved \a UniquableMDNode, which
> > +/// indicates an \a MDNodeFwdDecl in its path), the node itself will be
> > +/// unresolved.  As soon as all operands become resolved, it will drop
> > +/// RAUW support permanently.
> > +///
> > +/// If an unresolved node is part of a cycle, \a resolveCycles() needs
> > +/// to be called on some member of the cycle when each \a MDNodeFwdDecl
> > +/// has been removed.
> > +class UniquableMDNode : public MDNode {
> > +  friend class ReplaceableMetadataImpl;
> >    friend class MDNode;
> >    friend class LLVMContextImpl;
> > -  friend class ReplaceableMetadataImpl;
> >
> >    /// \brief Support RAUW as long as one of its arguments is replaceable.
> >    ///
> > -  /// If an operand is an \a MDNodeFwdDecl (or a replaceable \a GenericMDNode),
> > -  /// support RAUW to support uniquing as forward declarations are resolved.
> > -  /// As soon as operands have been resolved, drop support.
> > -  ///
> >    /// FIXME: Save memory by storing this in a pointer union with the
> >    /// LLVMContext, and adding an LLVMContext reference to RMI.
> >    std::unique_ptr<ReplaceableMetadataImpl> ReplaceableUses;
> >
> > +protected:
> >    /// \brief Create a new node.
> >    ///
> >    /// If \c AllowRAUW, then if any operands are unresolved support RAUW.  RAUW
> >    /// will be dropped once all operands have been resolved (or if \a
> >    /// resolveCycles() is called).
> > -  GenericMDNode(LLVMContext &C, ArrayRef<Metadata *> Vals, bool AllowRAUW);
> > -  ~GenericMDNode();
> > -
> > -  void setHash(unsigned Hash) { MDNodeSubclassData = Hash; }
> > -  void recalculateHash();
> > +  UniquableMDNode(LLVMContext &C, unsigned ID, ArrayRef<Metadata *> Vals,
> > +                  bool AllowRAUW);
> > +  ~UniquableMDNode();
> >
> >  public:
> > -  /// \brief Get the hash, if any.
> > -  unsigned getHash() const { return MDNodeSubclassData; }
> > -
> >    static bool classof(const Metadata *MD) {
> > -    return MD->getMetadataID() == GenericMDNodeKind;
> > +    return MD->getMetadataID() == MDTupleKind;
> >    }
> >
> >    /// \brief Check whether any operands are forward declarations.
> > @@ -766,11 +766,36 @@ private:
> >    void decrementUnresolvedOperandCount();
> >  };
> >
> > +/// \brief Tuple of metadata.
> > +///
> > +/// This is the simple \a MDNode arbitrary tuple.  Nodes are uniqued by
> > +/// default based on their operands.
> > +class MDTuple : public UniquableMDNode {
> > +  friend class LLVMContextImpl;
> > +  friend class UniquableMDNode;
> > +  friend class MDNode;
> > +
> > +  MDTuple(LLVMContext &C, ArrayRef<Metadata *> Vals, bool AllowRAUW)
> > +      : UniquableMDNode(C, MDTupleKind, Vals, AllowRAUW) {}
> > +  ~MDTuple();
> > +
> > +  void setHash(unsigned Hash) { MDNodeSubclassData = Hash; }
> > +  void recalculateHash();
> > +
> > +public:
> > +  /// \brief Get the hash, if any.
> > +  unsigned getHash() const { return MDNodeSubclassData; }
> > +
> > +  static bool classof(const Metadata *MD) {
> > +    return MD->getMetadataID() == MDTupleKind;
> > +  }
> > +};
> > +
> >  /// \brief Forward declaration of metadata.
> >  ///
> > -/// Forward declaration of metadata, in the form of a metadata node.  Unlike \a
> > -/// GenericMDNode, this class has support for RAUW and is suitable for forward
> > -/// references.
> > +/// Forward declaration of metadata, in the form of a basic tuple.  Unlike \a
> > +/// MDTuple, this class has full support for RAUW, is not owned, is not
> > +/// uniqued, and is suitable for forward references.
> >  class MDNodeFwdDecl : public MDNode, ReplaceableMetadataImpl {
> >    friend class Metadata;
> >    friend class MDNode;
> >
> > Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
> > +++ llvm/trunk/lib/AsmParser/LLParser.cpp Mon Jan 12 14:09:34 2015
> > @@ -169,8 +169,8 @@ bool LLParser::ValidateEndOfModule() {
> >
> >    // Resolve metadata cycles.
> >    for (auto &N : NumberedMetadata)
> > -    if (auto *G = cast_or_null<GenericMDNode>(N))
> > -      G->resolveCycles();
> > +    if (auto *U = cast_or_null<UniquableMDNode>(N))
> > +      U->resolveCycles();
> >
> >    // Look for intrinsic functions and CallInst that need to be upgraded
> >    for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; )
> >
> > Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> > +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Mon Jan 12 14:09:34 2015
> > @@ -558,8 +558,8 @@ void BitcodeReaderMDValueList::tryToReso
> >    // Resolve any cycles.
> >    for (auto &MD : MDValuePtrs) {
> >      assert(!(MD && isa<MDNodeFwdDecl>(MD)) && "Unexpected forward reference");
> > -    if (auto *G = dyn_cast_or_null<GenericMDNode>(MD))
> > -      G->resolveCycles();
> > +    if (auto *N = dyn_cast_or_null<UniquableMDNode>(MD))
> > +      N->resolveCycles();
> >    }
> >  }
> >
> >
> > Modified: llvm/trunk/lib/IR/DIBuilder.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
> > +++ llvm/trunk/lib/IR/DIBuilder.cpp Mon Jan 12 14:09:34 2015
> > @@ -55,7 +55,8 @@ DIBuilder::DIBuilder(Module &m, bool All
> >        AllowUnresolvedNodes(AllowUnresolvedNodes) {}
> >
> >  static bool isUnresolved(MDNode *N) {
> > -  return N && (isa<MDNodeFwdDecl>(N) || !cast<GenericMDNode>(N)->isResolved());
> > +  return N &&
> > +         (isa<MDNodeFwdDecl>(N) || !cast<UniquableMDNode>(N)->isResolved());
> >  }
> >
> >  void DIBuilder::trackIfUnresolved(MDNode *N) {
> > @@ -110,7 +111,7 @@ void DIBuilder::finalize() {
> >    // cycles.
> >    for (const auto &N : UnresolvedNodes)
> >      if (N)
> > -      cast<GenericMDNode>(N)->resolveCycles();
> > +      cast<UniquableMDNode>(N)->resolveCycles();
> >    UnresolvedNodes.clear();
> >
> >    // Can't handle unresolved nodes anymore.
> >
> > Modified: llvm/trunk/lib/IR/LLVMContextImpl.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.cpp?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/IR/LLVMContextImpl.cpp (original)
> > +++ llvm/trunk/lib/IR/LLVMContextImpl.cpp Mon Jan 12 14:09:34 2015
> > @@ -135,17 +135,17 @@ LLVMContextImpl::~LLVMContextImpl() {
> >    for (auto &Pair : ValuesAsMetadata)
> >      delete Pair.second;
> >
> > -  // Destroy MDNodes.  ~MDNode can move and remove nodes between the MDNodeSet
> > -  // and the NonUniquedMDNodes sets, so copy the values out first.
> > -  SmallVector<GenericMDNode *, 8> MDNodes;
> > -  MDNodes.reserve(MDNodeSet.size() + NonUniquedMDNodes.size());
> > -  MDNodes.append(MDNodeSet.begin(), MDNodeSet.end());
> > -  MDNodes.append(NonUniquedMDNodes.begin(), NonUniquedMDNodes.end());
> > -  for (GenericMDNode *I : MDNodes)
> > +  // Destroy MDNodes.  ~MDNode can move and remove nodes between the MDTuples
> > +  // and the DistinctMDNodes sets, so copy the values out first.
> > +  SmallVector<UniquableMDNode *, 8> Uniquables;
> > +  Uniquables.reserve(MDTuples.size() + DistinctMDNodes.size());
> > +  Uniquables.append(MDTuples.begin(), MDTuples.end());
> > +  Uniquables.append(DistinctMDNodes.begin(), DistinctMDNodes.end());
> > +  for (UniquableMDNode *I : Uniquables)
> >      I->dropAllReferences();
> > -  for (GenericMDNode *I : MDNodes)
> > -    delete I;
> > -  assert(MDNodeSet.empty() && NonUniquedMDNodes.empty() &&
> > +  for (UniquableMDNode *I : Uniquables)
> > +    delete cast<MDTuple>(I);
> > +  assert(MDTuples.empty() && DistinctMDNodes.empty() &&
> >           "Destroying all MDNodes didn't empty the Context's sets.");
> >
> >    // Destroy MDStrings.
> >
> > Modified: llvm/trunk/lib/IR/LLVMContextImpl.h
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/IR/LLVMContextImpl.h (original)
> > +++ llvm/trunk/lib/IR/LLVMContextImpl.h Mon Jan 12 14:09:34 2015
> > @@ -166,11 +166,11 @@ struct FunctionTypeKeyInfo {
> >    }
> >  };
> >
> > -/// \brief DenseMapInfo for GenericMDNode.
> > +/// \brief DenseMapInfo for MDTuple.
> >  ///
> >  /// Note that we don't need the is-function-local bit, since that's implicit in
> >  /// the operands.
> > -struct GenericMDNodeInfo {
> > +struct MDTupleInfo {
> >    struct KeyTy {
> >      ArrayRef<Metadata *> RawOps;
> >      ArrayRef<MDOperand> Ops;
> > @@ -179,10 +179,10 @@ struct GenericMDNodeInfo {
> >      KeyTy(ArrayRef<Metadata *> Ops)
> >          : RawOps(Ops), Hash(hash_combine_range(Ops.begin(), Ops.end())) {}
> >
> > -    KeyTy(GenericMDNode *N)
> > +    KeyTy(MDTuple *N)
> >          : Ops(N->op_begin(), N->op_end()), Hash(N->getHash()) {}
> >
> > -    bool operator==(const GenericMDNode *RHS) const {
> > +    bool operator==(const MDTuple *RHS) const {
> >        if (RHS == getEmptyKey() || RHS == getTombstoneKey())
> >          return false;
> >        if (Hash != RHS->getHash())
> > @@ -191,26 +191,26 @@ struct GenericMDNodeInfo {
> >        return RawOps.empty() ? compareOps(Ops, RHS) : compareOps(RawOps, RHS);
> >      }
> >      template <class T>
> > -    static bool compareOps(ArrayRef<T> Ops, const GenericMDNode *RHS) {
> > +    static bool compareOps(ArrayRef<T> Ops, const MDTuple *RHS) {
> >        if (Ops.size() != RHS->getNumOperands())
> >          return false;
> >        return std::equal(Ops.begin(), Ops.end(), RHS->op_begin());
> >      }
> >    };
> > -  static inline GenericMDNode *getEmptyKey() {
> > -    return DenseMapInfo<GenericMDNode *>::getEmptyKey();
> > +  static inline MDTuple *getEmptyKey() {
> > +    return DenseMapInfo<MDTuple *>::getEmptyKey();
> >    }
> > -  static inline GenericMDNode *getTombstoneKey() {
> > -    return DenseMapInfo<GenericMDNode *>::getTombstoneKey();
> > +  static inline MDTuple *getTombstoneKey() {
> > +    return DenseMapInfo<MDTuple *>::getTombstoneKey();
> >    }
> >    static unsigned getHashValue(const KeyTy &Key) { return Key.Hash; }
> > -  static unsigned getHashValue(const GenericMDNode *U) {
> > +  static unsigned getHashValue(const MDTuple *U) {
> >      return U->getHash();
> >    }
> > -  static bool isEqual(const KeyTy &LHS, const GenericMDNode *RHS) {
> > +  static bool isEqual(const KeyTy &LHS, const MDTuple *RHS) {
> >      return LHS == RHS;
> >    }
> > -  static bool isEqual(const GenericMDNode *LHS, const GenericMDNode *RHS) {
> > +  static bool isEqual(const MDTuple *LHS, const MDTuple *RHS) {
> >      return LHS == RHS;
> >    }
> >  };
> > @@ -245,13 +245,13 @@ public:
> >    DenseMap<Value *, ValueAsMetadata *> ValuesAsMetadata;
> >    DenseMap<Metadata *, MetadataAsValue *> MetadataAsValues;
> >
> > -  DenseSet<GenericMDNode *, GenericMDNodeInfo> MDNodeSet;
> > +  DenseSet<MDTuple *, MDTupleInfo> MDTuples;
> >
> >    // MDNodes may be uniqued or not uniqued.  When they're not uniqued, they
> >    // aren't in the MDNodeSet, but they're still shared between objects, so no
> >    // one object can destroy them.  This set allows us to at least destroy them
> >    // on Context destruction.
> > -  SmallPtrSet<GenericMDNode *, 1> NonUniquedMDNodes;
> > +  SmallPtrSet<UniquableMDNode *, 1> DistinctMDNodes;
> >
> >    DenseMap<Type*, ConstantAggregateZero*> CAZConstants;
> >
> >
> > Modified: llvm/trunk/lib/IR/Metadata.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/IR/Metadata.cpp (original)
> > +++ llvm/trunk/lib/IR/Metadata.cpp Mon Jan 12 14:09:34 2015
> > @@ -222,8 +222,8 @@ void ReplaceableMetadataImpl::resolveAll
> >      if (Owner.is<MetadataAsValue *>())
> >        continue;
> >
> > -    // Resolve GenericMDNodes that point at this.
> > -    auto *OwnerMD = dyn_cast<GenericMDNode>(Owner.get<Metadata *>());
> > +    // Resolve UniquableMDNodes that point at this.
> > +    auto *OwnerMD = dyn_cast<UniquableMDNode>(Owner.get<Metadata *>());
> >      if (!OwnerMD)
> >        continue;
> >      if (OwnerMD->isResolved())
> > @@ -400,7 +400,7 @@ MDNode::MDNode(LLVMContext &Context, uns
> >  bool MDNode::isResolved() const {
> >    if (isa<MDNodeFwdDecl>(this))
> >      return false;
> > -  return cast<GenericMDNode>(this)->isResolved();
> > +  return cast<UniquableMDNode>(this)->isResolved();
> >  }
> >
> >  static bool isOperandUnresolved(Metadata *Op) {
> > @@ -409,9 +409,9 @@ static bool isOperandUnresolved(Metadata
> >    return false;
> >  }
> >
> > -GenericMDNode::GenericMDNode(LLVMContext &C, ArrayRef<Metadata *> Vals,
> > -                             bool AllowRAUW)
> > -    : MDNode(C, GenericMDNodeKind, Vals) {
> > +UniquableMDNode::UniquableMDNode(LLVMContext &C, unsigned ID,
> > +                                 ArrayRef<Metadata *> Vals, bool AllowRAUW)
> > +    : MDNode(C, ID, Vals) {
> >    if (!AllowRAUW)
> >      return;
> >
> > @@ -427,16 +427,14 @@ GenericMDNode::GenericMDNode(LLVMContext
> >    SubclassData32 = NumUnresolved;
> >  }
> >
> > -GenericMDNode::~GenericMDNode() {
> > -  LLVMContextImpl *pImpl = getContext().pImpl;
> > +UniquableMDNode::~UniquableMDNode() {
> >    if (isStoredDistinctInContext())
> > -    pImpl->NonUniquedMDNodes.erase(this);
> > -  else
> > -    pImpl->MDNodeSet.erase(this);
> > +    getContext().pImpl->DistinctMDNodes.erase(this);
> > +
> >    dropAllReferences();
> >  }
> >
> > -void GenericMDNode::resolve() {
> > +void UniquableMDNode::resolve() {
> >    assert(!isResolved() && "Expected this to be unresolved");
> >
> >    // Move the map, so that this immediately looks resolved.
> > @@ -448,7 +446,7 @@ void GenericMDNode::resolve() {
> >    Uses->resolveAllUses();
> >  }
> >
> > -void GenericMDNode::resolveAfterOperandChange(Metadata *Old, Metadata *New) {
> > +void UniquableMDNode::resolveAfterOperandChange(Metadata *Old, Metadata *New) {
> >    assert(SubclassData32 != 0 && "Expected unresolved operands");
> >
> >    // Check if an operand was resolved.
> > @@ -458,13 +456,13 @@ void GenericMDNode::resolveAfterOperandC
> >      decrementUnresolvedOperandCount();
> >  }
> >
> > -void GenericMDNode::decrementUnresolvedOperandCount() {
> > +void UniquableMDNode::decrementUnresolvedOperandCount() {
> >    if (!--SubclassData32)
> >      // Last unresolved operand has just been resolved.
> >      resolve();
> >  }
> >
> > -void GenericMDNode::resolveCycles() {
> > +void UniquableMDNode::resolveCycles() {
> >    if (isResolved())
> >      return;
> >
> > @@ -477,13 +475,18 @@ void GenericMDNode::resolveCycles() {
> >        continue;
> >      assert(!isa<MDNodeFwdDecl>(Op) &&
> >             "Expected all forward declarations to be resolved");
> > -    if (auto *N = dyn_cast<GenericMDNode>(Op))
> > +    if (auto *N = dyn_cast<UniquableMDNode>(Op))
> >        if (!N->isResolved())
> >          N->resolveCycles();
> >    }
> >  }
> >
> > -void GenericMDNode::recalculateHash() {
> > +MDTuple::~MDTuple() {
> > +  if (!isStoredDistinctInContext())
> > +    getContext().pImpl->MDTuples.erase(this);
> > +}
> > +
> > +void MDTuple::recalculateHash() {
> >    setHash(hash_combine_range(op_begin(), op_end()));
> >  #ifndef NDEBUG
> >    {
> > @@ -498,10 +501,10 @@ void GenericMDNode::recalculateHash() {
> >  void MDNode::dropAllReferences() {
> >    for (unsigned I = 0, E = NumOperands; I != E; ++I)
> >      setOperand(I, nullptr);
> > -  if (auto *G = dyn_cast<GenericMDNode>(this))
> > -    if (!G->isResolved()) {
> > -      G->ReplaceableUses->resolveAllUses(/* ResolveUsers */ false);
> > -      G->ReplaceableUses.reset();
> > +  if (auto *N = dyn_cast<UniquableMDNode>(this))
> > +    if (!N->isResolved()) {
> > +      N->ReplaceableUses->resolveAllUses(/* ResolveUsers */ false);
> > +      N->ReplaceableUses.reset();
> >      }
> >  }
> >
> > @@ -522,7 +525,7 @@ namespace llvm {
> >  static const Metadata *get_hashable_data(const MDOperand &X) { return X.get(); }
> >  }
> >
> > -void GenericMDNode::handleChangedOperand(void *Ref, Metadata *New) {
> > +void UniquableMDNode::handleChangedOperand(void *Ref, Metadata *New) {
> >    unsigned Op = static_cast<MDOperand *>(Ref) - op_begin();
> >    assert(Op < getNumOperands() && "Expected valid operand");
> >
> > @@ -534,8 +537,8 @@ void GenericMDNode::handleChangedOperand
> >      return;
> >    }
> >
> > -  auto &Store = getContext().pImpl->MDNodeSet;
> > -  Store.erase(this);
> > +  auto &Store = getContext().pImpl->MDTuples;
> > +  Store.erase(cast<MDTuple>(this));
> >
> >    Metadata *Old = getOperand(Op);
> >    setOperand(Op, New);
> > @@ -549,11 +552,11 @@ void GenericMDNode::handleChangedOperand
> >    }
> >
> >    // Re-unique the node.
> > -  recalculateHash();
> > -  GenericMDNodeInfo::KeyTy Key(this);
> > +  cast<MDTuple>(this)->recalculateHash();
> > +  MDTupleInfo::KeyTy Key(cast<MDTuple>(this));
> >    auto I = Store.find_as(Key);
> >    if (I == Store.end()) {
> > -    Store.insert(this);
> > +    Store.insert(cast<MDTuple>(this));
> >
> >      if (!isResolved())
> >        resolveAfterOperandChange(Old, New);
> > @@ -570,7 +573,7 @@ void GenericMDNode::handleChangedOperand
> >      for (unsigned O = 0, E = getNumOperands(); O != E; ++O)
> >        setOperand(O, nullptr);
> >      ReplaceableUses->replaceAllUsesWith(*I);
> > -    delete this;
> > +    delete cast<MDTuple>(this);
> >      return;
> >    }
> >
> > @@ -580,9 +583,9 @@ void GenericMDNode::handleChangedOperand
> >
> >  MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef<Metadata *> MDs,
> >                            bool Insert) {
> > -  auto &Store = Context.pImpl->MDNodeSet;
> > +  auto &Store = Context.pImpl->MDTuples;
> >
> > -  GenericMDNodeInfo::KeyTy Key(MDs);
> > +  MDTupleInfo::KeyTy Key(MDs);
> >    auto I = Store.find_as(Key);
> >    if (I != Store.end())
> >      return *I;
> > @@ -590,14 +593,14 @@ MDNode *MDNode::getMDNode(LLVMContext &C
> >      return nullptr;
> >
> >    // Coallocate space for the node and Operands together, then placement new.
> > -  auto *N = new (MDs.size()) GenericMDNode(Context, MDs, /* AllowRAUW */ true);
> > +  auto *N = new (MDs.size()) MDTuple(Context, MDs, /* AllowRAUW */ true);
> >    N->setHash(Key.Hash);
> >    Store.insert(N);
> >    return N;
> >  }
> >
> >  MDNode *MDNode::getDistinct(LLVMContext &Context, ArrayRef<Metadata *> MDs) {
> > -  auto *N = new (MDs.size()) GenericMDNode(Context, MDs, /* AllowRAUW */ false);
> > +  auto *N = new (MDs.size()) MDTuple(Context, MDs, /* AllowRAUW */ false);
> >    N->storeDistinctInContext();
> >    return N;
> >  }
> > @@ -616,9 +619,9 @@ void MDNode::deleteTemporary(MDNode *N)
> >  void MDNode::storeDistinctInContext() {
> >    assert(!IsDistinctInContext && "Expected newly distinct metadata");
> >    IsDistinctInContext = true;
> > -  auto *G = cast<GenericMDNode>(this);
> > -  G->setHash(0);
> > -  getContext().pImpl->NonUniquedMDNodes.insert(G);
> > +  auto *T = cast<MDTuple>(this);
> > +  T->setHash(0);
> > +  getContext().pImpl->DistinctMDNodes.insert(T);
> >  }
> >
> >  void MDNode::replaceOperandWith(unsigned I, Metadata *New) {
> > @@ -630,7 +633,7 @@ void MDNode::replaceOperandWith(unsigned
> >      return;
> >    }
> >
> > -  cast<GenericMDNode>(this)->handleChangedOperand(mutable_begin() + I, New);
> > +  cast<UniquableMDNode>(this)->handleChangedOperand(mutable_begin() + I, New);
> >  }
> >
> >  void MDNode::setOperand(unsigned I, Metadata *New) {
> >
> > Modified: llvm/trunk/lib/IR/MetadataTracking.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/MetadataTracking.cpp?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/IR/MetadataTracking.cpp (original)
> > +++ llvm/trunk/lib/IR/MetadataTracking.cpp Mon Jan 12 14:09:34 2015
> > @@ -18,8 +18,8 @@ using namespace llvm;
> >
> >  ReplaceableMetadataImpl *ReplaceableMetadataImpl::get(Metadata &MD) {
> >    if (auto *N = dyn_cast<MDNode>(&MD)) {
> > -    if (auto *G = dyn_cast<GenericMDNode>(N))
> > -      return G->ReplaceableUses.get();
> > +    if (auto *U = dyn_cast<UniquableMDNode>(N))
> > +      return U->ReplaceableUses.get();
> >      return cast<MDNodeFwdDecl>(N);
> >    }
> >    return dyn_cast<ValueAsMetadata>(&MD);
> >
> > Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=225682&r1=225681&r2=225682&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Mon Jan 12 14:09:34 2015
> > @@ -260,8 +260,8 @@ Metadata *llvm::MapMetadata(const Metada
> >                              ValueMaterializer *Materializer) {
> >    Metadata *NewMD = MapMetadataImpl(MD, VM, Flags, TypeMapper, Materializer);
> >    if (NewMD && NewMD != MD)
> > -    if (auto *G = dyn_cast<GenericMDNode>(NewMD))
> > -      G->resolveCycles();
> > +    if (auto *N = dyn_cast<UniquableMDNode>(NewMD))
> > +      N->resolveCycles();
> >    return NewMD;
> >  }
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 





More information about the llvm-commits mailing list