[llvm] r226481 - IR: Use an enum to describe Metadata storage, NFC

Duncan P. N. Exon Smith dexonsmith at apple.com
Mon Jan 19 10:36:18 PST 2015


Author: dexonsmith
Date: Mon Jan 19 12:36:18 2015
New Revision: 226481

URL: http://llvm.org/viewvc/llvm-project?rev=226481&view=rev
Log:
IR: Use an enum to describe Metadata storage, NFC

More clearly describe the type of storage used for `Metadata`.

  - `Uniqued`: uniqued, stored in the context.
  - `Distinct`: distinct, stored in the context.
  - `Temporary`: not owned by anyone.

This is the first in a series of commits to fix a design problem with
`MDNodeFwdDecl` that I need to solve for PR22235.  While `MDNodeFwdDecl`
works well as a forward declaration, we use `MDNode::getTemporary()` for
more than forward declarations -- we also need to create early versions
of nodes (with fields not filled in) that we'll fill out later (see
`DIBuilder::finalize()` and `CGDebugInfo::finalize()` for examples).
This was a blind spot I had when I introduced `MDNodeFwdDecl` (which
David Blaikie (indirectly) highlighted in an unrelated review [1]).

[1]: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20150112/252381.html

In general, we need `MDTuple::getTemporary()` to give a temporary tuple
(like `MDNodeFwdDecl`), `MDLocation::getTemporary()` to give a temporary
location, and (the problem at hand) `GenericDebugMDNode::getTemporary()`
to give a temporary generic debug node.

So I need to fold the idea of "temporary" nodes back into
`UniquableMDNode`.  (More commits to follow as I refactor.)

Modified:
    llvm/trunk/include/llvm/IR/Metadata.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=226481&r1=226480&r2=226481&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Metadata.h (original)
+++ llvm/trunk/include/llvm/IR/Metadata.h Mon Jan 19 12:36:18 2015
@@ -47,8 +47,11 @@ class Metadata {
   const unsigned char SubclassID;
 
 protected:
+  /// \brief Active type of storage.
+  enum StorageType { Uniqued, Distinct, Temporary };
+
   /// \brief Storage flag for non-uniqued, otherwise unowned, metadata.
-  bool IsDistinctInContext : 1;
+  StorageType Storage : 2;
   // TODO: expose remaining bits to subclasses.
 
   unsigned short SubclassData16;
@@ -65,13 +68,13 @@ public:
   };
 
 protected:
-  Metadata(unsigned ID)
-      : SubclassID(ID), IsDistinctInContext(false), SubclassData16(0),
-        SubclassData32(0) {}
+  Metadata(unsigned ID, StorageType Storage)
+      : SubclassID(ID), Storage(Storage), SubclassData16(0), SubclassData32(0) {
+  }
   ~Metadata() {}
 
   /// \brief Store this in a big non-uniqued untyped bucket.
-  bool isStoredDistinctInContext() const { return IsDistinctInContext; }
+  bool isStoredDistinctInContext() const { return Storage == Distinct; }
 
   /// \brief Default handling of a changed operand, which asserts.
   ///
@@ -195,7 +198,7 @@ class ValueAsMetadata : public Metadata,
 
 protected:
   ValueAsMetadata(unsigned ID, Value *V)
-      : Metadata(ID), V(V) {
+      : Metadata(ID, Uniqued), V(V) {
     assert(V && "Expected valid value");
   }
   ~ValueAsMetadata() {}
@@ -446,8 +449,8 @@ class MDString : public Metadata {
   MDString &operator=(const MDString &) LLVM_DELETED_FUNCTION;
 
   StringMapEntry<MDString> *Entry;
-  MDString() : Metadata(MDStringKind), Entry(nullptr) {}
-  MDString(MDString &&) : Metadata(MDStringKind) {}
+  MDString() : Metadata(MDStringKind, Uniqued), Entry(nullptr) {}
+  MDString(MDString &&) : Metadata(MDStringKind, Uniqued) {}
 
 public:
   static MDString *get(LLVMContext &Context, StringRef Str);
@@ -606,7 +609,8 @@ protected:
     llvm_unreachable("Constructor throws?");
   }
 
-  MDNode(LLVMContext &Context, unsigned ID, ArrayRef<Metadata *> MDs);
+  MDNode(LLVMContext &Context, unsigned ID, StorageType Storage,
+         ArrayRef<Metadata *> MDs);
   ~MDNode() {}
 
   void dropAllReferences();
@@ -728,8 +732,8 @@ protected:
   /// 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).
-  UniquableMDNode(LLVMContext &C, unsigned ID, ArrayRef<Metadata *> Vals,
-                  bool AllowRAUW);
+  UniquableMDNode(LLVMContext &C, unsigned ID, StorageType Storage,
+                  ArrayRef<Metadata *> Vals);
   ~UniquableMDNode() {}
 
   void storeDistinctInContext();
@@ -778,8 +782,8 @@ class MDTuple : public UniquableMDNode {
   friend class LLVMContextImpl;
   friend class UniquableMDNode;
 
-  MDTuple(LLVMContext &C, ArrayRef<Metadata *> Vals, bool AllowRAUW)
-      : UniquableMDNode(C, MDTupleKind, Vals, AllowRAUW) {}
+  MDTuple(LLVMContext &C, StorageType Storage, ArrayRef<Metadata *> Vals)
+      : UniquableMDNode(C, MDTupleKind, Storage, Vals) {}
   ~MDTuple() { dropAllReferences(); }
 
   void setHash(unsigned Hash) { MDNodeSubclassData = Hash; }
@@ -830,13 +834,13 @@ class MDLocation : public UniquableMDNod
   friend class LLVMContextImpl;
   friend class UniquableMDNode;
 
-  MDLocation(LLVMContext &C, unsigned Line, unsigned Column,
-             ArrayRef<Metadata *> MDs, bool AllowRAUW);
+  MDLocation(LLVMContext &C, StorageType Storage, unsigned Line,
+             unsigned Column, ArrayRef<Metadata *> MDs);
   ~MDLocation() { dropAllReferences(); }
 
-  static MDLocation *constructHelper(LLVMContext &Context, unsigned Line,
-                                     unsigned Column, Metadata *Scope,
-                                     Metadata *InlinedAt, bool AllowRAUW);
+  static MDLocation *constructHelper(LLVMContext &Context, StorageType Storage,
+                                     unsigned Line, unsigned Column,
+                                     Metadata *Scope, Metadata *InlinedAt);
 
   static MDLocation *getImpl(LLVMContext &Context, unsigned Line,
                              unsigned Column, Metadata *Scope,
@@ -889,7 +893,7 @@ class MDNodeFwdDecl : public MDNode, Rep
   friend class ReplaceableMetadataImpl;
 
   MDNodeFwdDecl(LLVMContext &C, ArrayRef<Metadata *> Vals)
-      : MDNode(C, MDNodeFwdDeclKind, Vals) {}
+      : MDNode(C, MDNodeFwdDeclKind, Temporary, Vals) {}
 
 public:
   ~MDNodeFwdDecl() { dropAllReferences(); }

Modified: llvm/trunk/lib/IR/Metadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=226481&r1=226480&r2=226481&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Metadata.cpp (original)
+++ llvm/trunk/lib/IR/Metadata.cpp Mon Jan 19 12:36:18 2015
@@ -395,8 +395,9 @@ void MDNode::operator delete(void *Mem)
   ::operator delete(O);
 }
 
-MDNode::MDNode(LLVMContext &Context, unsigned ID, ArrayRef<Metadata *> MDs)
-    : Metadata(ID), Context(Context), NumOperands(MDs.size()),
+MDNode::MDNode(LLVMContext &Context, unsigned ID, StorageType Storage,
+               ArrayRef<Metadata *> MDs)
+    : Metadata(ID, Storage), Context(Context), NumOperands(MDs.size()),
       MDNodeSubclassData(0) {
   for (unsigned I = 0, E = MDs.size(); I != E; ++I)
     setOperand(I, MDs[I]);
@@ -415,9 +416,9 @@ static bool isOperandUnresolved(Metadata
 }
 
 UniquableMDNode::UniquableMDNode(LLVMContext &C, unsigned ID,
-                                 ArrayRef<Metadata *> Vals, bool AllowRAUW)
-    : MDNode(C, ID, Vals) {
-  if (!AllowRAUW)
+                                 StorageType Storage, ArrayRef<Metadata *> Vals)
+    : MDNode(C, ID, Storage, Vals) {
+  if (Storage != Uniqued)
     return;
 
   // Check whether any operands are unresolved, requiring re-uniquing.
@@ -618,14 +619,14 @@ MDTuple *MDTuple::getImpl(LLVMContext &C
     return nullptr;
 
   // Coallocate space for the node and Operands together, then placement new.
-  auto *N = new (MDs.size()) MDTuple(Context, MDs, /* AllowRAUW */ true);
+  auto *N = new (MDs.size()) MDTuple(Context, Uniqued, MDs);
   N->setHash(Key.Hash);
   Store.insert(N);
   return N;
 }
 
 MDTuple *MDTuple::getDistinct(LLVMContext &Context, ArrayRef<Metadata *> MDs) {
-  auto *N = new (MDs.size()) MDTuple(Context, MDs, /* AllowRAUW */ false);
+  auto *N = new (MDs.size()) MDTuple(Context, Distinct, MDs);
   N->storeDistinctInContext();
   return N;
 }
@@ -645,9 +646,9 @@ MDTuple *MDTuple::uniquifyImpl() {
 
 void MDTuple::eraseFromStoreImpl() { getContext().pImpl->MDTuples.erase(this); }
 
-MDLocation::MDLocation(LLVMContext &C, unsigned Line, unsigned Column,
-                       ArrayRef<Metadata *> MDs, bool AllowRAUW)
-    : UniquableMDNode(C, MDLocationKind, MDs, AllowRAUW) {
+MDLocation::MDLocation(LLVMContext &C, StorageType Storage, unsigned Line,
+                       unsigned Column, ArrayRef<Metadata *> MDs)
+    : UniquableMDNode(C, MDLocationKind, Storage, MDs) {
   assert((MDs.size() == 1 || MDs.size() == 2) &&
          "Expected a scope and optional inlined-at");
 
@@ -659,14 +660,15 @@ MDLocation::MDLocation(LLVMContext &C, u
   SubclassData16 = Column;
 }
 
-MDLocation *MDLocation::constructHelper(LLVMContext &Context, unsigned Line,
+MDLocation *MDLocation::constructHelper(LLVMContext &Context,
+                                        StorageType Storage, unsigned Line,
                                         unsigned Column, Metadata *Scope,
-                                        Metadata *InlinedAt, bool AllowRAUW) {
+                                        Metadata *InlinedAt) {
   SmallVector<Metadata *, 2> Ops;
   Ops.push_back(Scope);
   if (InlinedAt)
     Ops.push_back(InlinedAt);
-  return new (Ops.size()) MDLocation(Context, Line, Column, Ops, AllowRAUW);
+  return new (Ops.size()) MDLocation(Context, Storage, Line, Column, Ops);
 }
 
 static void adjustLine(unsigned &Line) {
@@ -697,8 +699,7 @@ MDLocation *MDLocation::getImpl(LLVMCont
   if (!ShouldCreate)
     return nullptr;
 
-  auto *N = constructHelper(Context, Line, Column, Scope, InlinedAt,
-                            /* AllowRAUW */ true);
+  auto *N = constructHelper(Context, Uniqued, Line, Column, Scope, InlinedAt);
   Store.insert(N);
   return N;
 }
@@ -710,8 +711,7 @@ MDLocation *MDLocation::getDistinct(LLVM
   adjustLine(Line);
   adjustColumn(Column);
 
-  auto *N = constructHelper(Context, Line, Column, Scope, InlinedAt,
-                            /* AllowRAUW */ false);
+  auto *N = constructHelper(Context, Distinct, Line, Column, Scope, InlinedAt);
   N->storeDistinctInContext();
   return N;
 }
@@ -740,8 +740,7 @@ MDNodeFwdDecl *MDNode::getTemporary(LLVM
 void MDNode::deleteTemporary(MDNode *N) { delete cast<MDNodeFwdDecl>(N); }
 
 void UniquableMDNode::storeDistinctInContext() {
-  assert(!IsDistinctInContext && "Expected newly distinct metadata");
-  IsDistinctInContext = true;
+  Storage = Distinct;
   if (auto *T = dyn_cast<MDTuple>(this))
     T->setHash(0);
   getContext().pImpl->DistinctMDNodes.insert(this);





More information about the llvm-commits mailing list