[llvm] r225682 - IR: Split GenericMDNode into MDTuple and UniquableMDNode
David Blaikie
dblaikie at gmail.com
Tue Jan 13 10:37:03 PST 2015
On Mon, Jan 12, 2015 at 7:47 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > 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).
Yeah, I don't think it's necessarily easier. At least that's not a reason
I'm suggesting it. It's more syntax to add to the IR, etc. Though I'm
hoping the implementation, especially once you've ported over a few of
these records to your proposal, will look pretty similarly generic at least
(tabular (not suggesting tblgen, just arrays of string record names and
field pointers or something) description of each record's schema and using
that to do the parsing/loading) then the only difference is whether the
schema comes from a hardcoded table or dynamically generated from the IR.
> 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.
>
While some things need special handling, a lot of the DWARF is just plain
old data that we just need to ferry through (granted, one day soon with
modules, etc, we might like to have a way to serialize this data ahead of
time (Adrian is working on that - but we could end up leveraging it to
create the DWARF bytes even for non-modular builds) leaving only the
'interesting' bits of debug info to remain in the metadata/need handling
like this). It seems rather inflexible to have to change the IR schema when
we want to support another DWARF tag (last one I added was imported
entities/declarations, but there's obviously many more bits of DWARF data
we don't yet support & someone might want - I imagine Swift will need it's
own bunch of extensions to describe various language-specific features that
don't line up perfectly with the small subset of DWARF we've implemented
for C-like languages).
An extensible schema like tihs would allow out-of-tree languages to easily
add their own DWARF extensions/support without quite such intrusive patches
to core parts of LLVM, I would think. (sure, they'd still need extra
patches to DwarfDebug et. al. to do the final emission).
> 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 would imagine we would have a specific naming scheme (same way we do now
- looking for specific named metadata, etc) for the schema records we care
about in the upgrader - and cross-check the schema against the reference &
map them across if necessary.
> > [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.
>
Thanks for the review
>
> >
> > - 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
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150113/5870a260/attachment.html>
More information about the llvm-commits
mailing list