<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 12, 2015 at 2:21 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"><span class=""><br>
> On 2015-Jan-12, at 13:02, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Mon, Jan 12, 2015 at 12:09 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> Author: dexonsmith<br>
> Date: Mon Jan 12 14:09:34 2015<br>
> New Revision: 225682<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225682&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225682&view=rev</a><br>
> Log:<br>
> IR: Split GenericMDNode into MDTuple and UniquableMDNode<br>
><br>
> Split `GenericMDNode` into two classes (with more descriptive names).<br>
><br>
> - `UniquableMDNode` will be a common subclass for `MDNode`s that are<br>
> sometimes uniqued like constants, and sometimes 'distinct'.<br>
><br>
> This class gets the (short-lived) RAUW support and related API.<br>
><br>
> - `MDTuple` is the basic tuple that has always been returned by<br>
> `MDNode::get()`. This is as opposed to more specific nodes to be<br>
> added soon, which have additional fields, custom assembly syntax,<br>
> and extra semantics.<br>
><br>
> 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?<br>
<br>
</span>That's my strong opinion :).<br>
<span class=""><br>
> Can we not make MDNodes sufficiently cheap while being general?<br>
><br>
> (Answer may well be 'no', but I'm just curious/wrapping my head around everything)<br>
><br>
<br>
</span>Making debug info "special" is about more than memory/time costs,<br>
although with LTO that's certainly a pressing concern. Large parts of<br>
the graph are still cyclic,</blockquote><div><br>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?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> which makes deserialization from bitcode<br>
expensive (since it still needs RAUW). With custom records, we'll be<br>
able to avoid these cycles in serialization (serializing only one<br>
direction) and recreate the links on the other side. Using the same<br>
mechanisms, we'll be able to avoid duplicating these cycles in<br>
`MapMetadata()`. We'll also have the flexibility to introduce<br>
first-class support for linking, e.g., subprogram descriptions.<br>
(Furthermore, there's a future down this path where upgrading debug info<br>
from bitcode is actually practical (although my understanding is that<br>
bitcode compatibility isn't particularly important to you).)<br>
<br>
But I think the resulting assembly changes are most important for the<br>
broader community (who may not be using LTO). While in C++ you have the<br>
`DI*` wrapper classes that allow you to understand and work with each<br>
node type, the assembly may as well be a binary stream. Comments<br>
mitigate this, but they don't get updated in testcases when the debug<br>
info scripts run, they don't describe all the fields, and they're not<br>
actually all that great. Unlike the rest of the IR (which is<br>
self-describing), you need to have `llvm/IR/DebugInfo.h` open (or<br>
memorized) to understand any of it.<br></blockquote><div><br>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.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It's enough for me that (in optimized code) *most* of the IR (by volume)<br>
is debug info. IMO, not having custom node types is akin to only having<br>
one type of instruction (although maybe that's an unfair straw man).<br>
<br>
(FWIW, I'd be in favour of first-class-ifying other metadata schemas<br>
too. There's a reason we don't use `std::tuple<>` in place of all C++<br>
data structures (another straw man; maybe I'm just ranting now!).)<br></blockquote><div><br>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)<br><br>But perhaps that's not important/unnecessarily general, I'm not sure.<br><br>[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]<br><br>- David<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>
> This class gets the hash-related logic, since other sublcasses of<br>
> `UniquableMDNode` may need to hash based on other fields.<br>
><br>
> To keep this diff from getting too big, I've added casts to `MDTuple`<br>
> that won't really scale as new subclasses of `UniquableMDNode` are<br>
> added, but I'll clean those up incrementally.<br>
><br>
> (No functionality change intended.)<br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/IR/Metadata.def<br>
> llvm/trunk/include/llvm/IR/Metadata.h<br>
> llvm/trunk/lib/AsmParser/LLParser.cpp<br>
> llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp<br>
> llvm/trunk/lib/IR/DIBuilder.cpp<br>
> llvm/trunk/lib/IR/LLVMContextImpl.cpp<br>
> llvm/trunk/lib/IR/LLVMContextImpl.h<br>
> llvm/trunk/lib/IR/Metadata.cpp<br>
> llvm/trunk/lib/IR/MetadataTracking.cpp<br>
> llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/Metadata.def<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.def?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.def?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/IR/Metadata.def (original)<br>
> +++ llvm/trunk/include/llvm/IR/Metadata.def Mon Jan 12 14:09:34 2015<br>
> @@ -37,7 +37,8 @@ HANDLE_METADATA_LEAF(ConstantAsMetadata)<br>
> HANDLE_METADATA_LEAF(LocalAsMetadata)<br>
> HANDLE_METADATA_BRANCH(MDNode)<br>
> HANDLE_METADATA_LEAF(MDNodeFwdDecl)<br>
> -HANDLE_METADATA_LEAF(GenericMDNode)<br>
> +HANDLE_METADATA_BRANCH(UniquableMDNode)<br>
> +HANDLE_METADATA_LEAF(MDTuple)<br>
><br>
> #undef HANDLE_METADATA<br>
> #undef HANDLE_METADATA_LEAF<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/Metadata.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/IR/Metadata.h (original)<br>
> +++ llvm/trunk/include/llvm/IR/Metadata.h Mon Jan 12 14:09:34 2015<br>
> @@ -56,7 +56,7 @@ protected:<br>
><br>
> public:<br>
> enum MetadataKind {<br>
> - GenericMDNodeKind,<br>
> + MDTupleKind,<br>
> MDNodeFwdDeclKind,<br>
> ConstantAsMetadataKind,<br>
> LocalAsMetadataKind,<br>
> @@ -158,7 +158,7 @@ public:<br>
> /// \brief Resolve all uses of this.<br>
> ///<br>
> /// Resolve all uses of this, turning off RAUW permanently. If \c<br>
> - /// ResolveUsers, call \a GenericMDNode::resolve() on any users whose last<br>
> + /// ResolveUsers, call \a UniquableMDNode::resolve() on any users whose last<br>
> /// operand is resolved.<br>
> void resolveAllUses(bool ResolveUsers = true);<br>
><br>
> @@ -682,7 +682,7 @@ public:<br>
><br>
> /// \brief Methods for support type inquiry through isa, cast, and dyn_cast:<br>
> static bool classof(const Metadata *MD) {<br>
> - return MD->getMetadataID() == GenericMDNodeKind ||<br>
> + return MD->getMetadataID() == MDTupleKind ||<br>
> MD->getMetadataID() == MDNodeFwdDeclKind;<br>
> }<br>
><br>
> @@ -698,46 +698,46 @@ public:<br>
> static MDNode *getMostGenericRange(MDNode *A, MDNode *B);<br>
> };<br>
><br>
> -/// \brief Generic metadata node.<br>
> +/// \brief Uniquable metadata node.<br>
> ///<br>
> -/// Generic metadata nodes, with opt-out support for uniquing.<br>
> -///<br>
> -/// Although nodes are uniqued by default, \a GenericMDNode has no support for<br>
> -/// RAUW. If an operand change (due to RAUW or otherwise) causes a uniquing<br>
> -/// collision, the uniquing bit is dropped.<br>
> -class GenericMDNode : public MDNode {<br>
> - friend class Metadata;<br>
> +/// A uniquable metadata node. This contains the basic functionality<br>
> +/// for implementing sub-types of \a MDNode that can be uniqued like<br>
> +/// constants.<br>
> +///<br>
> +/// There is limited support for RAUW at construction time. At<br>
> +/// construction time, if any operands are an instance of \a<br>
> +/// MDNodeFwdDecl (or another unresolved \a UniquableMDNode, which<br>
> +/// indicates an \a MDNodeFwdDecl in its path), the node itself will be<br>
> +/// unresolved. As soon as all operands become resolved, it will drop<br>
> +/// RAUW support permanently.<br>
> +///<br>
> +/// If an unresolved node is part of a cycle, \a resolveCycles() needs<br>
> +/// to be called on some member of the cycle when each \a MDNodeFwdDecl<br>
> +/// has been removed.<br>
> +class UniquableMDNode : public MDNode {<br>
> + friend class ReplaceableMetadataImpl;<br>
> friend class MDNode;<br>
> friend class LLVMContextImpl;<br>
> - friend class ReplaceableMetadataImpl;<br>
><br>
> /// \brief Support RAUW as long as one of its arguments is replaceable.<br>
> ///<br>
> - /// If an operand is an \a MDNodeFwdDecl (or a replaceable \a GenericMDNode),<br>
> - /// support RAUW to support uniquing as forward declarations are resolved.<br>
> - /// As soon as operands have been resolved, drop support.<br>
> - ///<br>
> /// FIXME: Save memory by storing this in a pointer union with the<br>
> /// LLVMContext, and adding an LLVMContext reference to RMI.<br>
> std::unique_ptr<ReplaceableMetadataImpl> ReplaceableUses;<br>
><br>
> +protected:<br>
> /// \brief Create a new node.<br>
> ///<br>
> /// If \c AllowRAUW, then if any operands are unresolved support RAUW. RAUW<br>
> /// will be dropped once all operands have been resolved (or if \a<br>
> /// resolveCycles() is called).<br>
> - GenericMDNode(LLVMContext &C, ArrayRef<Metadata *> Vals, bool AllowRAUW);<br>
> - ~GenericMDNode();<br>
> -<br>
> - void setHash(unsigned Hash) { MDNodeSubclassData = Hash; }<br>
> - void recalculateHash();<br>
> + UniquableMDNode(LLVMContext &C, unsigned ID, ArrayRef<Metadata *> Vals,<br>
> + bool AllowRAUW);<br>
> + ~UniquableMDNode();<br>
><br>
> public:<br>
> - /// \brief Get the hash, if any.<br>
> - unsigned getHash() const { return MDNodeSubclassData; }<br>
> -<br>
> static bool classof(const Metadata *MD) {<br>
> - return MD->getMetadataID() == GenericMDNodeKind;<br>
> + return MD->getMetadataID() == MDTupleKind;<br>
> }<br>
><br>
> /// \brief Check whether any operands are forward declarations.<br>
> @@ -766,11 +766,36 @@ private:<br>
> void decrementUnresolvedOperandCount();<br>
> };<br>
><br>
> +/// \brief Tuple of metadata.<br>
> +///<br>
> +/// This is the simple \a MDNode arbitrary tuple. Nodes are uniqued by<br>
> +/// default based on their operands.<br>
> +class MDTuple : public UniquableMDNode {<br>
> + friend class LLVMContextImpl;<br>
> + friend class UniquableMDNode;<br>
> + friend class MDNode;<br>
> +<br>
> + MDTuple(LLVMContext &C, ArrayRef<Metadata *> Vals, bool AllowRAUW)<br>
> + : UniquableMDNode(C, MDTupleKind, Vals, AllowRAUW) {}<br>
> + ~MDTuple();<br>
> +<br>
> + void setHash(unsigned Hash) { MDNodeSubclassData = Hash; }<br>
> + void recalculateHash();<br>
> +<br>
> +public:<br>
> + /// \brief Get the hash, if any.<br>
> + unsigned getHash() const { return MDNodeSubclassData; }<br>
> +<br>
> + static bool classof(const Metadata *MD) {<br>
> + return MD->getMetadataID() == MDTupleKind;<br>
> + }<br>
> +};<br>
> +<br>
> /// \brief Forward declaration of metadata.<br>
> ///<br>
> -/// Forward declaration of metadata, in the form of a metadata node. Unlike \a<br>
> -/// GenericMDNode, this class has support for RAUW and is suitable for forward<br>
> -/// references.<br>
> +/// Forward declaration of metadata, in the form of a basic tuple. Unlike \a<br>
> +/// MDTuple, this class has full support for RAUW, is not owned, is not<br>
> +/// uniqued, and is suitable for forward references.<br>
> class MDNodeFwdDecl : public MDNode, ReplaceableMetadataImpl {<br>
> friend class Metadata;<br>
> friend class MDNode;<br>
><br>
> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)<br>
> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Mon Jan 12 14:09:34 2015<br>
> @@ -169,8 +169,8 @@ bool LLParser::ValidateEndOfModule() {<br>
><br>
> // Resolve metadata cycles.<br>
> for (auto &N : NumberedMetadata)<br>
> - if (auto *G = cast_or_null<GenericMDNode>(N))<br>
> - G->resolveCycles();<br>
> + if (auto *U = cast_or_null<UniquableMDNode>(N))<br>
> + U->resolveCycles();<br>
><br>
> // Look for intrinsic functions and CallInst that need to be upgraded<br>
> for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; )<br>
><br>
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)<br>
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Mon Jan 12 14:09:34 2015<br>
> @@ -558,8 +558,8 @@ void BitcodeReaderMDValueList::tryToReso<br>
> // Resolve any cycles.<br>
> for (auto &MD : MDValuePtrs) {<br>
> assert(!(MD && isa<MDNodeFwdDecl>(MD)) && "Unexpected forward reference");<br>
> - if (auto *G = dyn_cast_or_null<GenericMDNode>(MD))<br>
> - G->resolveCycles();<br>
> + if (auto *N = dyn_cast_or_null<UniquableMDNode>(MD))<br>
> + N->resolveCycles();<br>
> }<br>
> }<br>
><br>
><br>
> Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>
> +++ llvm/trunk/lib/IR/DIBuilder.cpp Mon Jan 12 14:09:34 2015<br>
> @@ -55,7 +55,8 @@ DIBuilder::DIBuilder(Module &m, bool All<br>
> AllowUnresolvedNodes(AllowUnresolvedNodes) {}<br>
><br>
> static bool isUnresolved(MDNode *N) {<br>
> - return N && (isa<MDNodeFwdDecl>(N) || !cast<GenericMDNode>(N)->isResolved());<br>
> + return N &&<br>
> + (isa<MDNodeFwdDecl>(N) || !cast<UniquableMDNode>(N)->isResolved());<br>
> }<br>
><br>
> void DIBuilder::trackIfUnresolved(MDNode *N) {<br>
> @@ -110,7 +111,7 @@ void DIBuilder::finalize() {<br>
> // cycles.<br>
> for (const auto &N : UnresolvedNodes)<br>
> if (N)<br>
> - cast<GenericMDNode>(N)->resolveCycles();<br>
> + cast<UniquableMDNode>(N)->resolveCycles();<br>
> UnresolvedNodes.clear();<br>
><br>
> // Can't handle unresolved nodes anymore.<br>
><br>
> Modified: llvm/trunk/lib/IR/LLVMContextImpl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.cpp?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.cpp?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/LLVMContextImpl.cpp (original)<br>
> +++ llvm/trunk/lib/IR/LLVMContextImpl.cpp Mon Jan 12 14:09:34 2015<br>
> @@ -135,17 +135,17 @@ LLVMContextImpl::~LLVMContextImpl() {<br>
> for (auto &Pair : ValuesAsMetadata)<br>
> delete Pair.second;<br>
><br>
> - // Destroy MDNodes. ~MDNode can move and remove nodes between the MDNodeSet<br>
> - // and the NonUniquedMDNodes sets, so copy the values out first.<br>
> - SmallVector<GenericMDNode *, 8> MDNodes;<br>
> - MDNodes.reserve(MDNodeSet.size() + NonUniquedMDNodes.size());<br>
> - MDNodes.append(MDNodeSet.begin(), MDNodeSet.end());<br>
> - MDNodes.append(NonUniquedMDNodes.begin(), NonUniquedMDNodes.end());<br>
> - for (GenericMDNode *I : MDNodes)<br>
> + // Destroy MDNodes. ~MDNode can move and remove nodes between the MDTuples<br>
> + // and the DistinctMDNodes sets, so copy the values out first.<br>
> + SmallVector<UniquableMDNode *, 8> Uniquables;<br>
> + Uniquables.reserve(MDTuples.size() + DistinctMDNodes.size());<br>
> + Uniquables.append(MDTuples.begin(), MDTuples.end());<br>
> + Uniquables.append(DistinctMDNodes.begin(), DistinctMDNodes.end());<br>
> + for (UniquableMDNode *I : Uniquables)<br>
> I->dropAllReferences();<br>
> - for (GenericMDNode *I : MDNodes)<br>
> - delete I;<br>
> - assert(MDNodeSet.empty() && NonUniquedMDNodes.empty() &&<br>
> + for (UniquableMDNode *I : Uniquables)<br>
> + delete cast<MDTuple>(I);<br>
> + assert(MDTuples.empty() && DistinctMDNodes.empty() &&<br>
> "Destroying all MDNodes didn't empty the Context's sets.");<br>
><br>
> // Destroy MDStrings.<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=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/LLVMContextImpl.h (original)<br>
> +++ llvm/trunk/lib/IR/LLVMContextImpl.h Mon Jan 12 14:09:34 2015<br>
> @@ -166,11 +166,11 @@ struct FunctionTypeKeyInfo {<br>
> }<br>
> };<br>
><br>
> -/// \brief DenseMapInfo for GenericMDNode.<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 GenericMDNodeInfo {<br>
> +struct MDTupleInfo {<br>
> struct KeyTy {<br>
> ArrayRef<Metadata *> RawOps;<br>
> ArrayRef<MDOperand> Ops;<br>
> @@ -179,10 +179,10 @@ struct GenericMDNodeInfo {<br>
> KeyTy(ArrayRef<Metadata *> Ops)<br>
> : RawOps(Ops), Hash(hash_combine_range(Ops.begin(), Ops.end())) {}<br>
><br>
> - KeyTy(GenericMDNode *N)<br>
> + KeyTy(MDTuple *N)<br>
> : Ops(N->op_begin(), N->op_end()), Hash(N->getHash()) {}<br>
><br>
> - bool operator==(const GenericMDNode *RHS) const {<br>
> + bool operator==(const MDTuple *RHS) const {<br>
> if (RHS == getEmptyKey() || RHS == getTombstoneKey())<br>
> return false;<br>
> if (Hash != RHS->getHash())<br>
> @@ -191,26 +191,26 @@ struct GenericMDNodeInfo {<br>
> return RawOps.empty() ? compareOps(Ops, RHS) : compareOps(RawOps, RHS);<br>
> }<br>
> template <class T><br>
> - static bool compareOps(ArrayRef<T> Ops, const GenericMDNode *RHS) {<br>
> + static bool compareOps(ArrayRef<T> Ops, const MDTuple *RHS) {<br>
> if (Ops.size() != RHS->getNumOperands())<br>
> return false;<br>
> return std::equal(Ops.begin(), Ops.end(), RHS->op_begin());<br>
> }<br>
> };<br>
> - static inline GenericMDNode *getEmptyKey() {<br>
> - return DenseMapInfo<GenericMDNode *>::getEmptyKey();<br>
> + static inline MDTuple *getEmptyKey() {<br>
> + return DenseMapInfo<MDTuple *>::getEmptyKey();<br>
> }<br>
> - static inline GenericMDNode *getTombstoneKey() {<br>
> - return DenseMapInfo<GenericMDNode *>::getTombstoneKey();<br>
> + static inline MDTuple *getTombstoneKey() {<br>
> + return DenseMapInfo<MDTuple *>::getTombstoneKey();<br>
> }<br>
> static unsigned getHashValue(const KeyTy &Key) { return Key.Hash; }<br>
> - static unsigned getHashValue(const GenericMDNode *U) {<br>
> + static unsigned getHashValue(const MDTuple *U) {<br>
> return U->getHash();<br>
> }<br>
> - static bool isEqual(const KeyTy &LHS, const GenericMDNode *RHS) {<br>
> + static bool isEqual(const KeyTy &LHS, const MDTuple *RHS) {<br>
> return LHS == RHS;<br>
> }<br>
> - static bool isEqual(const GenericMDNode *LHS, const GenericMDNode *RHS) {<br>
> + static bool isEqual(const MDTuple *LHS, const MDTuple *RHS) {<br>
> return LHS == RHS;<br>
> }<br>
> };<br>
> @@ -245,13 +245,13 @@ public:<br>
> DenseMap<Value *, ValueAsMetadata *> ValuesAsMetadata;<br>
> DenseMap<Metadata *, MetadataAsValue *> MetadataAsValues;<br>
><br>
> - DenseSet<GenericMDNode *, GenericMDNodeInfo> MDNodeSet;<br>
> + DenseSet<MDTuple *, MDTupleInfo> MDTuples;<br>
><br>
> // MDNodes may be uniqued or not uniqued. When they're not uniqued, they<br>
> // aren't in the MDNodeSet, but they're still shared between objects, so no<br>
> // one object can destroy them. This set allows us to at least destroy them<br>
> // on Context destruction.<br>
> - SmallPtrSet<GenericMDNode *, 1> NonUniquedMDNodes;<br>
> + SmallPtrSet<UniquableMDNode *, 1> DistinctMDNodes;<br>
><br>
> DenseMap<Type*, ConstantAggregateZero*> CAZConstants;<br>
><br>
><br>
> Modified: llvm/trunk/lib/IR/Metadata.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/Metadata.cpp (original)<br>
> +++ llvm/trunk/lib/IR/Metadata.cpp Mon Jan 12 14:09:34 2015<br>
> @@ -222,8 +222,8 @@ void ReplaceableMetadataImpl::resolveAll<br>
> if (Owner.is<MetadataAsValue *>())<br>
> continue;<br>
><br>
> - // Resolve GenericMDNodes that point at this.<br>
> - auto *OwnerMD = dyn_cast<GenericMDNode>(Owner.get<Metadata *>());<br>
> + // Resolve UniquableMDNodes that point at this.<br>
> + auto *OwnerMD = dyn_cast<UniquableMDNode>(Owner.get<Metadata *>());<br>
> if (!OwnerMD)<br>
> continue;<br>
> if (OwnerMD->isResolved())<br>
> @@ -400,7 +400,7 @@ MDNode::MDNode(LLVMContext &Context, uns<br>
> bool MDNode::isResolved() const {<br>
> if (isa<MDNodeFwdDecl>(this))<br>
> return false;<br>
> - return cast<GenericMDNode>(this)->isResolved();<br>
> + return cast<UniquableMDNode>(this)->isResolved();<br>
> }<br>
><br>
> static bool isOperandUnresolved(Metadata *Op) {<br>
> @@ -409,9 +409,9 @@ static bool isOperandUnresolved(Metadata<br>
> return false;<br>
> }<br>
><br>
> -GenericMDNode::GenericMDNode(LLVMContext &C, ArrayRef<Metadata *> Vals,<br>
> - bool AllowRAUW)<br>
> - : MDNode(C, GenericMDNodeKind, Vals) {<br>
> +UniquableMDNode::UniquableMDNode(LLVMContext &C, unsigned ID,<br>
> + ArrayRef<Metadata *> Vals, bool AllowRAUW)<br>
> + : MDNode(C, ID, Vals) {<br>
> if (!AllowRAUW)<br>
> return;<br>
><br>
> @@ -427,16 +427,14 @@ GenericMDNode::GenericMDNode(LLVMContext<br>
> SubclassData32 = NumUnresolved;<br>
> }<br>
><br>
> -GenericMDNode::~GenericMDNode() {<br>
> - LLVMContextImpl *pImpl = getContext().pImpl;<br>
> +UniquableMDNode::~UniquableMDNode() {<br>
> if (isStoredDistinctInContext())<br>
> - pImpl->NonUniquedMDNodes.erase(this);<br>
> - else<br>
> - pImpl->MDNodeSet.erase(this);<br>
> + getContext().pImpl->DistinctMDNodes.erase(this);<br>
> +<br>
> dropAllReferences();<br>
> }<br>
><br>
> -void GenericMDNode::resolve() {<br>
> +void UniquableMDNode::resolve() {<br>
> assert(!isResolved() && "Expected this to be unresolved");<br>
><br>
> // Move the map, so that this immediately looks resolved.<br>
> @@ -448,7 +446,7 @@ void GenericMDNode::resolve() {<br>
> Uses->resolveAllUses();<br>
> }<br>
><br>
> -void GenericMDNode::resolveAfterOperandChange(Metadata *Old, Metadata *New) {<br>
> +void UniquableMDNode::resolveAfterOperandChange(Metadata *Old, Metadata *New) {<br>
> assert(SubclassData32 != 0 && "Expected unresolved operands");<br>
><br>
> // Check if an operand was resolved.<br>
> @@ -458,13 +456,13 @@ void GenericMDNode::resolveAfterOperandC<br>
> decrementUnresolvedOperandCount();<br>
> }<br>
><br>
> -void GenericMDNode::decrementUnresolvedOperandCount() {<br>
> +void UniquableMDNode::decrementUnresolvedOperandCount() {<br>
> if (!--SubclassData32)<br>
> // Last unresolved operand has just been resolved.<br>
> resolve();<br>
> }<br>
><br>
> -void GenericMDNode::resolveCycles() {<br>
> +void UniquableMDNode::resolveCycles() {<br>
> if (isResolved())<br>
> return;<br>
><br>
> @@ -477,13 +475,18 @@ void GenericMDNode::resolveCycles() {<br>
> continue;<br>
> assert(!isa<MDNodeFwdDecl>(Op) &&<br>
> "Expected all forward declarations to be resolved");<br>
> - if (auto *N = dyn_cast<GenericMDNode>(Op))<br>
> + if (auto *N = dyn_cast<UniquableMDNode>(Op))<br>
> if (!N->isResolved())<br>
> N->resolveCycles();<br>
> }<br>
> }<br>
><br>
> -void GenericMDNode::recalculateHash() {<br>
> +MDTuple::~MDTuple() {<br>
> + if (!isStoredDistinctInContext())<br>
> + getContext().pImpl->MDTuples.erase(this);<br>
> +}<br>
> +<br>
> +void MDTuple::recalculateHash() {<br>
> setHash(hash_combine_range(op_begin(), op_end()));<br>
> #ifndef NDEBUG<br>
> {<br>
> @@ -498,10 +501,10 @@ void GenericMDNode::recalculateHash() {<br>
> void MDNode::dropAllReferences() {<br>
> for (unsigned I = 0, E = NumOperands; I != E; ++I)<br>
> setOperand(I, nullptr);<br>
> - if (auto *G = dyn_cast<GenericMDNode>(this))<br>
> - if (!G->isResolved()) {<br>
> - G->ReplaceableUses->resolveAllUses(/* ResolveUsers */ false);<br>
> - G->ReplaceableUses.reset();<br>
> + if (auto *N = dyn_cast<UniquableMDNode>(this))<br>
> + if (!N->isResolved()) {<br>
> + N->ReplaceableUses->resolveAllUses(/* ResolveUsers */ false);<br>
> + N->ReplaceableUses.reset();<br>
> }<br>
> }<br>
><br>
> @@ -522,7 +525,7 @@ namespace llvm {<br>
> static const Metadata *get_hashable_data(const MDOperand &X) { return X.get(); }<br>
> }<br>
><br>
> -void GenericMDNode::handleChangedOperand(void *Ref, Metadata *New) {<br>
> +void UniquableMDNode::handleChangedOperand(void *Ref, Metadata *New) {<br>
> unsigned Op = static_cast<MDOperand *>(Ref) - op_begin();<br>
> assert(Op < getNumOperands() && "Expected valid operand");<br>
><br>
> @@ -534,8 +537,8 @@ void GenericMDNode::handleChangedOperand<br>
> return;<br>
> }<br>
><br>
> - auto &Store = getContext().pImpl->MDNodeSet;<br>
> - Store.erase(this);<br>
> + auto &Store = getContext().pImpl->MDTuples;<br>
> + Store.erase(cast<MDTuple>(this));<br>
><br>
> Metadata *Old = getOperand(Op);<br>
> setOperand(Op, New);<br>
> @@ -549,11 +552,11 @@ void GenericMDNode::handleChangedOperand<br>
> }<br>
><br>
> // Re-unique the node.<br>
> - recalculateHash();<br>
> - GenericMDNodeInfo::KeyTy Key(this);<br>
> + cast<MDTuple>(this)->recalculateHash();<br>
> + MDTupleInfo::KeyTy Key(cast<MDTuple>(this));<br>
> auto I = Store.find_as(Key);<br>
> if (I == Store.end()) {<br>
> - Store.insert(this);<br>
> + Store.insert(cast<MDTuple>(this));<br>
><br>
> if (!isResolved())<br>
> resolveAfterOperandChange(Old, New);<br>
> @@ -570,7 +573,7 @@ void GenericMDNode::handleChangedOperand<br>
> for (unsigned O = 0, E = getNumOperands(); O != E; ++O)<br>
> setOperand(O, nullptr);<br>
> ReplaceableUses->replaceAllUsesWith(*I);<br>
> - delete this;<br>
> + delete cast<MDTuple>(this);<br>
> return;<br>
> }<br>
><br>
> @@ -580,9 +583,9 @@ void GenericMDNode::handleChangedOperand<br>
><br>
> MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef<Metadata *> MDs,<br>
> bool Insert) {<br>
> - auto &Store = Context.pImpl->MDNodeSet;<br>
> + auto &Store = Context.pImpl->MDTuples;<br>
><br>
> - GenericMDNodeInfo::KeyTy Key(MDs);<br>
> + MDTupleInfo::KeyTy Key(MDs);<br>
> auto I = Store.find_as(Key);<br>
> if (I != Store.end())<br>
> return *I;<br>
> @@ -590,14 +593,14 @@ MDNode *MDNode::getMDNode(LLVMContext &C<br>
> return nullptr;<br>
><br>
> // Coallocate space for the node and Operands together, then placement new.<br>
> - auto *N = new (MDs.size()) GenericMDNode(Context, MDs, /* AllowRAUW */ true);<br>
> + auto *N = new (MDs.size()) MDTuple(Context, MDs, /* AllowRAUW */ true);<br>
> N->setHash(Key.Hash);<br>
> Store.insert(N);<br>
> return N;<br>
> }<br>
><br>
> MDNode *MDNode::getDistinct(LLVMContext &Context, ArrayRef<Metadata *> MDs) {<br>
> - auto *N = new (MDs.size()) GenericMDNode(Context, MDs, /* AllowRAUW */ false);<br>
> + auto *N = new (MDs.size()) MDTuple(Context, MDs, /* AllowRAUW */ false);<br>
> N->storeDistinctInContext();<br>
> return N;<br>
> }<br>
> @@ -616,9 +619,9 @@ void MDNode::deleteTemporary(MDNode *N)<br>
> void MDNode::storeDistinctInContext() {<br>
> assert(!IsDistinctInContext && "Expected newly distinct metadata");<br>
> IsDistinctInContext = true;<br>
> - auto *G = cast<GenericMDNode>(this);<br>
> - G->setHash(0);<br>
> - getContext().pImpl->NonUniquedMDNodes.insert(G);<br>
> + auto *T = cast<MDTuple>(this);<br>
> + T->setHash(0);<br>
> + getContext().pImpl->DistinctMDNodes.insert(T);<br>
> }<br>
><br>
> void MDNode::replaceOperandWith(unsigned I, Metadata *New) {<br>
> @@ -630,7 +633,7 @@ void MDNode::replaceOperandWith(unsigned<br>
> return;<br>
> }<br>
><br>
> - cast<GenericMDNode>(this)->handleChangedOperand(mutable_begin() + I, New);<br>
> + cast<UniquableMDNode>(this)->handleChangedOperand(mutable_begin() + I, New);<br>
> }<br>
><br>
> void MDNode::setOperand(unsigned I, Metadata *New) {<br>
><br>
> Modified: llvm/trunk/lib/IR/MetadataTracking.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/MetadataTracking.cpp?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/MetadataTracking.cpp?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/MetadataTracking.cpp (original)<br>
> +++ llvm/trunk/lib/IR/MetadataTracking.cpp Mon Jan 12 14:09:34 2015<br>
> @@ -18,8 +18,8 @@ using namespace llvm;<br>
><br>
> ReplaceableMetadataImpl *ReplaceableMetadataImpl::get(Metadata &MD) {<br>
> if (auto *N = dyn_cast<MDNode>(&MD)) {<br>
> - if (auto *G = dyn_cast<GenericMDNode>(N))<br>
> - return G->ReplaceableUses.get();<br>
> + if (auto *U = dyn_cast<UniquableMDNode>(N))<br>
> + return U->ReplaceableUses.get();<br>
> return cast<MDNodeFwdDecl>(N);<br>
> }<br>
> return dyn_cast<ValueAsMetadata>(&MD);<br>
><br>
> Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=225682&r1=225681&r2=225682&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=225682&r1=225681&r2=225682&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Mon Jan 12 14:09:34 2015<br>
> @@ -260,8 +260,8 @@ Metadata *llvm::MapMetadata(const Metada<br>
> ValueMaterializer *Materializer) {<br>
> Metadata *NewMD = MapMetadataImpl(MD, VM, Flags, TypeMapper, Materializer);<br>
> if (NewMD && NewMD != MD)<br>
> - if (auto *G = dyn_cast<GenericMDNode>(NewMD))<br>
> - G->resolveCycles();<br>
> + if (auto *N = dyn_cast<UniquableMDNode>(NewMD))<br>
> + N->resolveCycles();<br>
> return NewMD;<br>
> }<br>
><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>
</div></div></blockquote></div><br></div></div>