[llvm] r222205 - IR: Split MDNode into GenericMDNode and MDNodeFwdDecl

Eric Christopher echristo at gmail.com
Mon Nov 17 20:46:10 PST 2014


On Mon Nov 17 2014 at 4:45:03 PM Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> Author: dexonsmith
> Date: Mon Nov 17 18:37:17 2014
> New Revision: 222205
>
> URL: http://llvm.org/viewvc/llvm-project?rev=222205&view=rev
> Log:
> IR: Split MDNode into GenericMDNode and MDNodeFwdDecl
>
>
Small bikeshedding:

ReplaceableNDNode maybe?

Best name I've been able to come up with so far.

-eric

Split `MDNode` into two classes:
>
>   - `GenericMDNode`, which is uniquable (and for now, always starts
>     uniqued).  Once `Metadata` is split from the `Value` hierarchy, this
>     class will lose the ability to RAUW itself.
>
>   - `MDNodeFwdDecl`, which is used for the "temporary" interface, is
>     never uniqued, and isn't managed by `LLVMContext` at all.
>
> I've left most of the guts in `MDNode` for now, but I'll incrementally
> move things to the right places (or delete the functionality, as
> appropriate).
>
> Part of PR21532.
>
> Modified:
>     llvm/trunk/include/llvm/IR/Metadata.h
>     llvm/trunk/include/llvm/IR/Value.h
>     llvm/trunk/lib/IR/LLVMContextImpl.cpp
>     llvm/trunk/lib/IR/LLVMContextImpl.h
>     llvm/trunk/lib/IR/Metadata.cpp
>
> Modified: llvm/trunk/include/llvm/IR/Metadata.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/IR/Metadata.h?rev=222205&r1=222204&r2=222205&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/IR/Metadata.h (original)
> +++ llvm/trunk/include/llvm/IR/Metadata.h Mon Nov 17 18:37:17 2014
> @@ -45,7 +45,9 @@ protected:
>
>  public:
>    static bool classof(const Value *V) {
> -    return V->getValueID() == MDNodeVal || V->getValueID() == MDStringVal;
> +    return V->getValueID() == GenericMDNodeVal ||
> +           V->getValueID() == MDNodeFwdDeclVal ||
> +           V->getValueID() == MDStringVal;
>    }
>  };
>
> @@ -137,14 +139,16 @@ struct DenseMapInfo<AAMDNodes> {
>  class MDNodeOperand;
>
>  //===-------------------------------------------------------
> ---------------===//
> -/// \brief Generic tuple of metadata.
> +/// \brief Tuple of metadata.
>  class MDNode : public Metadata {
>    MDNode(const MDNode &) LLVM_DELETED_FUNCTION;
>    void operator=(const MDNode &) LLVM_DELETED_FUNCTION;
>    friend class MDNodeOperand;
>    friend class LLVMContextImpl;
>
> -  /// \brief If the MDNode is uniqued cache the hash to speed up lookup.
> +protected:
> +  // TODO: Sink this into GenericMDNode.  Can't do this until operands are
> +  // allocated at the front (currently they're at the back).
>    unsigned Hash;
>
>    /// \brief Subclass data enums.
> @@ -174,7 +178,8 @@ class MDNode : public Metadata {
>    void replaceOperand(MDNodeOperand *Op, Value *NewVal);
>    ~MDNode();
>
> -  MDNode(LLVMContext &C, ArrayRef<Value*> Vals, bool isFunctionLocal);
> +  MDNode(LLVMContext &C, unsigned ID, ArrayRef<Value *> Vals,
> +         bool isFunctionLocal);
>
>    static MDNode *getMDNode(LLVMContext &C, ArrayRef<Value*> Vals,
>                             FunctionLocalness FL, bool Insert = true);
> @@ -223,12 +228,10 @@ public:
>    /// code because it recursively visits all the MDNode's operands.
>    const Function *getFunction() const;
>
> -  /// \brief Get the hash, if any.
> -  unsigned getHash() const { return Hash; }
> -
>    /// \brief Methods for support type inquiry through isa, cast, and
> dyn_cast:
>    static bool classof(const Value *V) {
> -    return V->getValueID() == MDNodeVal;
> +    return V->getValueID() == GenericMDNodeVal ||
> +           V->getValueID() == MDNodeFwdDeclVal;
>    }
>
>    /// \brief Check whether MDNode is a vtable access.
> @@ -241,10 +244,8 @@ public:
>    static AAMDNodes getMostGenericAA(const AAMDNodes &A, const AAMDNodes
> &B);
>    static MDNode *getMostGenericFPMath(MDNode *A, MDNode *B);
>    static MDNode *getMostGenericRange(MDNode *A, MDNode *B);
> -private:
> -  /// \brief Delete this node.  Only when there are no uses.
> -  void destroy();
>
> +protected:
>    bool isNotUniqued() const {
>      return (getSubclassDataFromValue() & NotUniquedBit) != 0;
>    }
> @@ -257,6 +258,61 @@ private:
>    }
>  };
>
> +/// \brief Generic 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.
> +///
> +/// TODO: Make uniquing opt-out (status: mandatory, sometimes dropped).
> +/// TODO: Drop support for RAUW.
> +class GenericMDNode : public MDNode {
> +  friend class MDNode;
> +
> +  GenericMDNode(LLVMContext &C, ArrayRef<Value *> Vals, bool
> isFunctionLocal)
> +      : MDNode(C, GenericMDNodeVal, Vals, isFunctionLocal) {}
> +  ~GenericMDNode();
> +
> +public:
> +  /// \brief Get the hash, if any.
> +  unsigned getHash() const { return Hash; }
> +
> +  static bool classof(const Value *V) {
> +    return V->getValueID() == GenericMDNodeVal;
> +  }
> +
> +private:
> +  /// \brief Delete this node.  Only when there are no uses.
> +  void destroy();
> +  friend class MDNode;
> +  friend class LLVMContextImpl;
> +};
> +
> +/// \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.
> +class MDNodeFwdDecl : public MDNode {
> +  friend class MDNode;
> +
> +  MDNodeFwdDecl(LLVMContext &C, ArrayRef<Value *> Vals, bool
> isFunctionLocal)
> +      : MDNode(C, MDNodeFwdDeclVal, Vals, isFunctionLocal) {}
> +  ~MDNodeFwdDecl() {}
> +
> +public:
> +  static bool classof(const Value *V) {
> +    return V->getValueID() == MDNodeFwdDeclVal;
> +  }
> +
> +private:
> +  /// \brief Delete this node.  Only when there are no uses.
> +  void destroy();
> +  friend class MDNode;
> +};
> +
>  //===-------------------------------------------------------
> ---------------===//
>  /// \brief A tuple of MDNodes.
>  ///
>
> Modified: llvm/trunk/include/llvm/IR/Value.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/IR/Value.h?rev=222205&r1=222204&r2=222205&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/IR/Value.h (original)
> +++ llvm/trunk/include/llvm/IR/Value.h Mon Nov 17 18:37:17 2014
> @@ -345,7 +345,8 @@ public:
>      ConstantStructVal,        // This is an instance of ConstantStruct
>      ConstantVectorVal,        // This is an instance of ConstantVector
>      ConstantPointerNullVal,   // This is an instance of
> ConstantPointerNull
> -    MDNodeVal,                // This is an instance of MDNode
> +    GenericMDNodeVal,         // This is an instance of GenericMDNode
> +    MDNodeFwdDeclVal,         // This is an instance of MDNodeFwdDecl
>      MDStringVal,              // This is an instance of MDString
>      InlineAsmVal,             // This is an instance of InlineAsm
>      InstructionVal,           // This is an instance of Instruction
> @@ -681,7 +682,8 @@ template <> struct isa_impl<GlobalObject
>
>  template <> struct isa_impl<MDNode, Value> {
>    static inline bool doit(const Value &Val) {
> -    return Val.getValueID() == Value::MDNodeVal;
> +    return Val.getValueID() == Value::GenericMDNodeVal ||
> +           Val.getValueID() == Value::MDNodeFwdDeclVal;
>    }
>  };
>
>
> Modified: llvm/trunk/lib/IR/LLVMContextImpl.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/
> LLVMContextImpl.cpp?rev=222205&r1=222204&r2=222205&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/IR/LLVMContextImpl.cpp (original)
> +++ llvm/trunk/lib/IR/LLVMContextImpl.cpp Mon Nov 17 18:37:17 2014
> @@ -122,13 +122,12 @@ LLVMContextImpl::~LLVMContextImpl() {
>
>    // Destroy MDNodes.  ~MDNode can move and remove nodes between the
> MDNodeSet
>    // and the NonUniquedMDNodes sets, so copy the values out first.
> -  SmallVector<MDNode*, 8> MDNodes;
> +  SmallVector<GenericMDNode *, 8> MDNodes;
>    MDNodes.reserve(MDNodeSet.size() + NonUniquedMDNodes.size());
>    MDNodes.append(MDNodeSet.begin(), MDNodeSet.end());
>    MDNodes.append(NonUniquedMDNodes.begin(), NonUniquedMDNodes.end());
> -  for (SmallVectorImpl<MDNode *>::iterator I = MDNodes.begin(),
> -         E = MDNodes.end(); I != E; ++I)
> -    (*I)->destroy();
> +  for (auto &I : MDNodes)
> +    I->destroy();
>    assert(MDNodeSet.empty() && NonUniquedMDNodes.empty() &&
>           "Destroying all MDNodes didn't empty the Context's sets.");
>
>
> Modified: llvm/trunk/lib/IR/LLVMContextImpl.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/
> LLVMContextImpl.h?rev=222205&r1=222204&r2=222205&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/IR/LLVMContextImpl.h (original)
> +++ llvm/trunk/lib/IR/LLVMContextImpl.h Mon Nov 17 18:37:17 2014
> @@ -191,7 +191,7 @@ struct FunctionTypeKeyInfo {
>    }
>  };
>
> -/// \brief DenseMapInfo for MDNode.
> +/// \brief DenseMapInfo for GenericMDNode.
>  ///
>  /// Note that we don't need the is-function-local bit, since that's
> implicit in
>  /// the operands.
> @@ -203,7 +203,7 @@ struct GenericMDNodeInfo {
>      KeyTy(ArrayRef<Value *> Ops)
>          : Ops(Ops), Hash(hash_combine_range(Ops.begin(), Ops.end())) {}
>
> -    KeyTy(MDNode *N, SmallVectorImpl<Value *> &Storage) {
> +    KeyTy(GenericMDNode *N, SmallVectorImpl<Value *> &Storage) {
>        Storage.resize(N->getNumOperands());
>        for (unsigned I = 0, E = N->getNumOperands(); I != E; ++I)
>          Storage[I] = N->getOperand(I);
> @@ -211,7 +211,7 @@ struct GenericMDNodeInfo {
>        Hash = hash_combine_range(Ops.begin(), Ops.end());
>      }
>
> -    bool operator==(const MDNode *RHS) const {
> +    bool operator==(const GenericMDNode *RHS) const {
>        if (RHS == getEmptyKey() || RHS == getTombstoneKey())
>          return false;
>        if (Hash != RHS->getHash() || Ops.size() != RHS->getNumOperands())
> @@ -222,20 +222,20 @@ struct GenericMDNodeInfo {
>        return true;
>      }
>    };
> -  static inline MDNode *getEmptyKey() {
> -    return DenseMapInfo<MDNode *>::getEmptyKey();
> +  static inline GenericMDNode *getEmptyKey() {
> +    return DenseMapInfo<GenericMDNode *>::getEmptyKey();
>    }
> -  static inline MDNode *getTombstoneKey() {
> -    return DenseMapInfo<MDNode *>::getTombstoneKey();
> +  static inline GenericMDNode *getTombstoneKey() {
> +    return DenseMapInfo<GenericMDNode *>::getTombstoneKey();
>    }
>    static unsigned getHashValue(const KeyTy &Key) { return Key.Hash; }
> -  static unsigned getHashValue(const MDNode *U) {
> +  static unsigned getHashValue(const GenericMDNode *U) {
>      return U->getHash();
>    }
> -  static bool isEqual(const KeyTy &LHS, const MDNode *RHS) {
> +  static bool isEqual(const KeyTy &LHS, const GenericMDNode *RHS) {
>      return LHS == RHS;
>    }
> -  static bool isEqual(const MDNode *LHS, const MDNode *RHS) {
> +  static bool isEqual(const GenericMDNode *LHS, const GenericMDNode *RHS)
> {
>      return LHS == RHS;
>    }
>  };
> @@ -293,14 +293,14 @@ public:
>
>    StringMap<MDString> MDStringCache;
>
> -  DenseSet<MDNode *, GenericMDNodeInfo> MDNodeSet;
> +  DenseSet<GenericMDNode *, GenericMDNodeInfo> MDNodeSet;
>
>    // 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<MDNode*, 1> NonUniquedMDNodes;
> -
> +  SmallPtrSet<GenericMDNode *, 1> NonUniquedMDNodes;
> +
>    DenseMap<Type*, ConstantAggregateZero*> CAZConstants;
>
>    typedef ConstantUniqueMap<ConstantArray> ArrayConstantsTy;
>
> Modified: llvm/trunk/lib/IR/Metadata.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/
> Metadata.cpp?rev=222205&r1=222204&r2=222205&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/IR/Metadata.cpp (original)
> +++ llvm/trunk/lib/IR/Metadata.cpp Mon Nov 17 18:37:17 2014
> @@ -108,6 +108,11 @@ void MDNodeOperand::allUsesReplacedWith(
>
>  /// \brief Get the MDNodeOperand's coallocated on the end of the MDNode.
>  static MDNodeOperand *getOperandPtr(MDNode *N, unsigned Op) {
> +  static_assert(sizeof(GenericMDNode) == sizeof(MDNode),
> +                "Expected subclasses to have no size overhead");
> +  static_assert(sizeof(MDNodeFwdDecl) == sizeof(MDNode),
> +                "Expected subclasses to have no size overhead");
> +
>    // Use <= instead of < to permit a one-past-the-end address.
>    assert(Op <= N->getNumOperands() && "Invalid operand number");
>    return reinterpret_cast<MDNodeOperand*>(N + 1) + Op;
> @@ -118,8 +123,9 @@ void MDNode::replaceOperandWith(unsigned
>    replaceOperand(Op, Val);
>  }
>
> -MDNode::MDNode(LLVMContext &C, ArrayRef<Value *> Vals, bool
> isFunctionLocal)
> -    : Metadata(C, Value::MDNodeVal), Hash(0) {
> +MDNode::MDNode(LLVMContext &C, unsigned ID, ArrayRef<Value *> Vals,
> +               bool isFunctionLocal)
> +    : Metadata(C, ID), Hash(0) {
>    NumOperands = Vals.size();
>
>    if (isFunctionLocal)
> @@ -137,16 +143,18 @@ MDNode::MDNode(LLVMContext &C, ArrayRef<
>    }
>  }
>
> -/// ~MDNode - Destroy MDNode.
> -MDNode::~MDNode() {
> -  assert((getSubclassDataFromValue() & DestroyFlag) != 0 &&
> -         "Not being destroyed through destroy()?");
> +GenericMDNode::~GenericMDNode() {
>    LLVMContextImpl *pImpl = getType()->getContext().pImpl;
>    if (isNotUniqued()) {
>      pImpl->NonUniquedMDNodes.erase(this);
>    } else {
>      pImpl->MDNodeSet.erase(this);
>    }
> +}
> +
> +MDNode::~MDNode() {
> +  assert((getSubclassDataFromValue() & DestroyFlag) != 0 &&
> +         "Not being destroyed through destroy()?");
>
>    // Destroy the operands.
>    for (MDNodeOperand *Op = getOperandPtr(this, 0), *E = Op+NumOperands;
> @@ -209,10 +217,17 @@ const Function *MDNode::getFunction() co
>  }
>
>  // destroy - Delete this node.  Only when there are no uses.
> -void MDNode::destroy() {
> +void GenericMDNode::destroy() {
> +  setValueSubclassData(getSubclassDataFromValue() | DestroyFlag);
> +  // Placement delete, then free the memory.
> +  this->~GenericMDNode();
> +  free(this);
> +}
> +
> +void MDNodeFwdDecl::destroy() {
>    setValueSubclassData(getSubclassDataFromValue() | DestroyFlag);
>    // Placement delete, then free the memory.
> -  this->~MDNode();
> +  this->~MDNodeFwdDecl();
>    free(this);
>  }
>
> @@ -253,8 +268,9 @@ MDNode *MDNode::getMDNode(LLVMContext &C
>    }
>
>    // Coallocate space for the node and Operands together, then placement
> new.
> -  void *Ptr = malloc(sizeof(MDNode) + Vals.size() *
> sizeof(MDNodeOperand));
> -  MDNode *N = new (Ptr) MDNode(Context, Vals, isFunctionLocal);
> +  void *Ptr =
> +      malloc(sizeof(GenericMDNode) + Vals.size() * sizeof(MDNodeOperand));
> +  GenericMDNode *N = new (Ptr) GenericMDNode(Context, Vals,
> isFunctionLocal);
>
>    N->Hash = Key.Hash;
>    Store.insert(N);
> @@ -276,9 +292,9 @@ MDNode *MDNode::getIfExists(LLVMContext
>  }
>
>  MDNode *MDNode::getTemporary(LLVMContext &Context, ArrayRef<Value*>
> Vals) {
> -  MDNode *N =
> -    (MDNode *)malloc(sizeof(MDNode) + Vals.size() *
> sizeof(MDNodeOperand));
> -  N = new (N) MDNode(Context, Vals, FL_No);
> +  MDNode *N = (MDNode *)malloc(sizeof(MDNodeFwdDecl) +
> +                               Vals.size() * sizeof(MDNodeOperand));
> +  N = new (N) MDNodeFwdDecl(Context, Vals, FL_No);
>    N->setValueSubclassData(N->getSubclassDataFromValue() |
>                            NotUniquedBit);
>    LeakDetector::addGarbageObject(N);
> @@ -287,16 +303,13 @@ MDNode *MDNode::getTemporary(LLVMContext
>
>  void MDNode::deleteTemporary(MDNode *N) {
>    assert(N->use_empty() && "Temporary MDNode has uses!");
> -  assert(!N->getContext().pImpl->MDNodeSet.erase(N) &&
> -         "Deleting a non-temporary uniqued node!");
> -  assert(!N->getContext().pImpl->NonUniquedMDNodes.erase(N) &&
> -         "Deleting a non-temporary non-uniqued node!");
> +  assert(isa<MDNodeFwdDecl>(N) && "Expected forward declaration");
>    assert((N->getSubclassDataFromValue() & NotUniquedBit) &&
>           "Temporary MDNode does not have NotUniquedBit set!");
>    assert((N->getSubclassDataFromValue() & DestroyFlag) == 0 &&
>           "Temporary MDNode has DestroyFlag set!");
>    LeakDetector::removeGarbageObject(N);
> -  N->destroy();
> +  cast<MDNodeFwdDecl>(N)->destroy();
>  }
>
>  /// \brief Return specified operand.
> @@ -308,8 +321,9 @@ Value *MDNode::getOperand(unsigned i) co
>  void MDNode::setIsNotUniqued() {
>    setValueSubclassData(getSubclassDataFromValue() | NotUniquedBit);
>    LLVMContextImpl *pImpl = getType()->getContext().pImpl;
> -  Hash = 0;
> -  pImpl->NonUniquedMDNodes.insert(this);
> +  auto *G = cast<GenericMDNode>(this);
> +  G->Hash = 0;
> +  pImpl->NonUniquedMDNodes.insert(G);
>  }
>
>  // Replace value from this node's operand list.
> @@ -345,9 +359,10 @@ void MDNode::replaceOperand(MDNodeOperan
>    }
>
>    auto &Store = getContext().pImpl->MDNodeSet;
> +  auto *N = cast<GenericMDNode>(this);
>
>    // Remove "this" from the context map.
> -  Store.erase(this);
> +  Store.erase(N);
>
>    // Update the operand.
>    Op->set(To);
> @@ -365,16 +380,16 @@ void MDNode::replaceOperand(MDNodeOperan
>    // check to see if another node with the same operands already exists
> in the
>    // set.  If so, then this node is redundant.
>    SmallVector<Value *, 8> Vals;
> -  GenericMDNodeInfo::KeyTy Key(this, Vals);
> +  GenericMDNodeInfo::KeyTy Key(N, Vals);
>    auto I = Store.find_as(Key);
>    if (I != Store.end()) {
> -    replaceAllUsesWith(*I);
> -    destroy();
> +    N->replaceAllUsesWith(*I);
> +    N->destroy();
>      return;
>    }
>
> -  this->Hash = Key.Hash;
> -  Store.insert(this);
> +  N->Hash = Key.Hash;
> +  Store.insert(N);
>
>    // If this MDValue was previously function-local but no longer is, clear
>    // its function-local flag.
>
>
> _______________________________________________
> 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/20141118/03df85db/attachment.html>


More information about the llvm-commits mailing list