Give embedded metadata its own type instead of relying on EmptyStructTy.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@72610 91177308-0d34-0410-b5e6-96231b3b80d8

bindings/ocaml/llvm/llvm.ml

@@ -26,6 +26,7 @@ module TypeKind = struct
   | Fp128
   | Ppc_fp128
   | Label
+  | Metadata
   | Integer
   | Function
   | Struct
@@ -840,3 +841,4 @@ let rec string_of_lltype ty =
   | TypeKind.Double -> "double"
   | TypeKind.Float -> "float"
   | TypeKind.Void -> "void"
+  | TypeKind.Metadata -> "metadata"

bindings/ocaml/llvm/llvm.mli

@@ -61,6 +61,7 @@ module TypeKind : sig
   | Fp128
   | Ppc_fp128
   | Label
+  | Metadata
   | Integer
   | Function
   | Struct

docs/LangRef.html

@@ -41,6 +41,7 @@
           <li><a href="#t_floating">Floating Point Types</a></li>
           <li><a href="#t_void">Void Type</a></li>
           <li><a href="#t_label">Label Type</a></li>
+          <li><a href="#t_metadata">Metadata Type</a></li>
         </ol>
       </li>
       <li><a href="#t_derived">Derived Types</a>
@@ -1254,14 +1255,16 @@ classifications:</p>
           <a href="#t_vector">vector</a>,
           <a href="#t_struct">structure</a>,
           <a href="#t_array">array</a>,
-          <a href="#t_label">label</a>.
+          <a href="#t_label">label</a>,
+          <a href="#t_metadata">metadata</a>.
       </td>
     </tr>
     <tr>
       <td><a href="#t_primitive">primitive</a></td>
       <td><a href="#t_label">label</a>,
           <a href="#t_void">void</a>,
-          <a href="#t_floating">floating point</a>.</td>
+          <a href="#t_floating">floating point</a>,
+          <a href="#t_metadata">metadata</a>.</td>
     </tr>
     <tr>
       <td><a href="#t_derived">derived</a></td>
@@ -1337,6 +1340,22 @@ system.</p>
 </pre>
 </div>
 
+<!-- _______________________________________________________________________ -->
+<div class="doc_subsubsection"> <a name="t_metadata">Metadata Type</a> </div>
+
+<div class="doc_text">
+<h5>Overview:</h5>
+<p>The metadata type represents embedded metadata. The only derived type that
+may contain metadata is <tt>metadata*</tt> or a function type that returns or
+takes metadata typed parameters, but not pointer to metadata types.</p>
+
+<h5>Syntax:</h5>
+
+<pre>
+  metadata
+</pre>
+</div>
+
 
 <!-- ======================================================================= -->
 <div class="doc_subsection"> <a name="t_derived">Derived Types</a> </div>
@@ -1866,9 +1885,10 @@ constants and smaller complex constants.</p>
   <dt><b>Metadata node</b></dt>
 
   <dd>A metadata node is a structure-like constant with the type of an empty
-  struct.  For example: "<tt>{ } !{ i32 0, { } !"test" }</tt>". Unlike other
+  struct.  For example: "<tt>metadata !{ i32 0, metadata !"test" }</tt>".
-  constants that are meant to be interpreted as part of the instruction stream,
+  Unlike other constants that are meant to be interpreted as part of the
-  metadata is a place to attach additional information such as debug info.
+  instruction stream, metadata is a place to attach additional information such
+  as debug info.
   </dd>
 </dl>
 
@@ -2046,9 +2066,9 @@ following is the syntax for constant expressions:</p>
 
 <p>Embedded metadata provides a way to attach arbitrary data to the
 instruction stream without affecting the behaviour of the program.  There are
-two metadata primitives, strings and nodes. All metadata has the type of an
+two metadata primitives, strings and nodes. All metadata has the
-empty struct and is identified in syntax by a preceding exclamation point
+<tt>metadata</tt> type and is identified in syntax by a preceding exclamation
-('<tt>!</tt>').
+point ('<tt>!</tt>').
 </p>
 
 <p>A metadata string is a string surrounded by double quotes.  It can contain
@@ -2058,12 +2078,12 @@ the two digit hex code.  For example: "<tt>!"test\00"</tt>".
 
 <p>Metadata nodes are represented with notation similar to structure constants
 (a comma separated list of elements, surrounded by braces and preceeded by an
-exclamation point).  For example: "<tt>!{ { } !"test\00", i32 10}</tt>".
+exclamation point).  For example: "<tt>!{ metadata !"test\00", i32 10}</tt>".
 </p>
 
 <p>A metadata node will attempt to track changes to the values it holds. In
 the event that a value is deleted, it will be replaced with a typeless
-"<tt>null</tt>", such as "<tt>{ } !{null, i32 0}</tt>".</p> 
+"<tt>null</tt>", such as "<tt>metadata !{null, i32 10}</tt>".</p> 
 
 <p>Optimizations may rely on metadata to provide additional information about
 the program that isn't available in the instructions, or that isn't easily

include/llvm/Bitcode/LLVMBitCodes.h

@@ -90,8 +90,9 @@ namespace bitc {
     // binary compatibility.
     TYPE_CODE_X86_FP80 = 13,   // X86 LONG DOUBLE
     TYPE_CODE_FP128    = 14,   // LONG DOUBLE (112 bit mantissa)
-    TYPE_CODE_PPC_FP128= 15    // PPC LONG DOUBLE (2 doubles)
+    TYPE_CODE_PPC_FP128= 15,   // PPC LONG DOUBLE (2 doubles)
-    // Any other type code is assumed to be an unknown type.
+
+    TYPE_CODE_METADATA = 16    // METADATA
   };
 
   // The type symbol table only has one code (TST_ENTRY_CODE).

include/llvm/Constants.h

@@ -860,10 +860,10 @@ public:
   ///
   const char *end() const { return StrEnd; }
 
-  /// getType() specialization - Type is always an empty struct.
+  /// getType() specialization - Type is always MetadataTy.
   ///
   inline const Type *getType() const {
-    return Type::EmptyStructTy;
+    return Type::MetadataTy;
   }
 
   /// isNullValue - Return true if this is the value that would be returned by

include/llvm/MDNode.h

@@ -88,6 +88,10 @@ public:
     return Node.size();
   }
 
+  bool elem_empty() const {
+    return Node.empty();
+  }
+
   const_elem_iterator elem_begin() const {
     return Node.begin();
   }
@@ -96,10 +100,10 @@ public:
     return Node.end();
   }
 
-  /// getType() specialization - Type is always an empty struct.
+  /// getType() specialization - Type is always MetadataTy.
   ///
   inline const Type *getType() const {
-    return Type::EmptyStructTy;
+    return Type::MetadataTy;
   }
 
   /// isNullValue - Return true if this is the value that would be returned by

include/llvm/Type.h

@@ -75,16 +75,17 @@ public:
     FP128TyID,       ///<  4: 128 bit floating point type (112-bit mantissa)
     PPC_FP128TyID,   ///<  5: 128 bit floating point type (two 64-bits)
     LabelTyID,       ///<  6: Labels
+    MetadataTyID,    ///<  7: Metadata
 
     // Derived types... see DerivedTypes.h file...
     // Make sure FirstDerivedTyID stays up to date!!!
-    IntegerTyID,     ///<  7: Arbitrary bit width integers
+    IntegerTyID,     ///<  8: Arbitrary bit width integers
-    FunctionTyID,    ///<  8: Functions
+    FunctionTyID,    ///<  9: Functions
-    StructTyID,      ///<  9: Structures
+    StructTyID,      ///< 10: Structures
-    ArrayTyID,       ///< 10: Arrays
+    ArrayTyID,       ///< 11: Arrays
-    PointerTyID,     ///< 11: Pointers
+    PointerTyID,     ///< 12: Pointers
-    OpaqueTyID,      ///< 12: Opaque: type with unknown structure
+    OpaqueTyID,      ///< 13: Opaque: type with unknown structure
-    VectorTyID,      ///< 13: SIMD 'packed' format, or other vector type
+    VectorTyID,      ///< 14: SIMD 'packed' format, or other vector type
 
     NumTypeIDs,                         // Must remain as last defined ID
     LastPrimitiveTyID = LabelTyID,
@@ -326,7 +327,7 @@ public:
   //===--------------------------------------------------------------------===//
   // These are the builtin types that are always available...
   //
-  static const Type *VoidTy, *LabelTy, *FloatTy, *DoubleTy, *EmptyStructTy;
+  static const Type *VoidTy, *LabelTy, *FloatTy, *DoubleTy, *MetadataTy;
   static const Type *X86_FP80Ty, *FP128Ty, *PPC_FP128Ty;
   static const IntegerType *Int1Ty, *Int8Ty, *Int16Ty, *Int32Ty, *Int64Ty;
 

lib/AsmParser/LLLexer.cpp

@@ -570,6 +570,7 @@ lltok::Kind LLLexer::LexIdentifier() {
   TYPEKEYWORD("fp128",     Type::FP128Ty);
   TYPEKEYWORD("ppc_fp128", Type::PPC_FP128Ty);
   TYPEKEYWORD("label",     Type::LabelTy);
+  TYPEKEYWORD("metadata",  Type::MetadataTy);
 #undef TYPEKEYWORD
 
   // Handle special forms for autoupgrading.  Drop these in LLVM 3.0.  This is

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -498,6 +498,9 @@ bool BitcodeReader::ParseTypeTable() {
     case bitc::TYPE_CODE_OPAQUE:    // OPAQUE
       ResultTy = 0;
       break;
+    case bitc::TYPE_CODE_METADATA:  // METADATA
+      ResultTy = Type::MetadataTy;
+      break;
     case bitc::TYPE_CODE_INTEGER:   // INTEGER: [width]
       if (Record.size() < 1)
         return Error("Invalid Integer type record");

lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -206,6 +206,7 @@ static void WriteTypeTable(const ValueEnumerator &VE, BitstreamWriter &Stream) {
     case Type::PPC_FP128TyID: Code = bitc::TYPE_CODE_PPC_FP128; break;
     case Type::LabelTyID:  Code = bitc::TYPE_CODE_LABEL;  break;
     case Type::OpaqueTyID: Code = bitc::TYPE_CODE_OPAQUE; break;
+    case Type::MetadataTyID: Code = bitc::TYPE_CODE_METADATA; break;
     case Type::IntegerTyID:
       // INTEGER: [width]
       Code = bitc::TYPE_CODE_INTEGER;

lib/Transforms/Utils/ValueMapper.cpp

@@ -16,6 +16,8 @@
 #include "llvm/Constants.h"
 #include "llvm/GlobalValue.h"
 #include "llvm/Instruction.h"
+#include "llvm/MDNode.h"
+#include "llvm/ADT/SmallVector.h"
 using namespace llvm;
 
 Value *llvm::MapValue(const Value *V, ValueMapTy &VM) {
@@ -33,7 +35,7 @@ Value *llvm::MapValue(const Value *V, ValueMapTy &VM) {
   if (Constant *C = const_cast<Constant*>(dyn_cast<Constant>(V))) {
     if (isa<ConstantInt>(C) || isa<ConstantFP>(C) ||
         isa<ConstantPointerNull>(C) || isa<ConstantAggregateZero>(C) ||
-        isa<UndefValue>(C))
+        isa<UndefValue>(C) || isa<MDString>(C))
       return VMSlot = C;           // Primitive constants map directly
     else if (ConstantArray *CA = dyn_cast<ConstantArray>(C)) {
       for (User::op_iterator b = CA->op_begin(), i = b, e = CA->op_end();
@@ -100,6 +102,27 @@ Value *llvm::MapValue(const Value *V, ValueMapTy &VM) {
       }
       return VM[V] = C;
       
+    } else if (MDNode *N = dyn_cast<MDNode>(C)) {
+      for (MDNode::const_elem_iterator b = N->elem_begin(), i = b,
+             e = N->elem_end(); i != e; ++i) {
+        if (!*i) continue;
+
+        Value *MV = MapValue(*i, VM);
+        if (MV != *i) {
+          // This MDNode must contain a reference to a global, make a new MDNode
+          // and return it.
+	  SmallVector<Value*, 8> Values;
+          Values.reserve(N->getNumElements());
+          for (MDNode::const_elem_iterator j = b; j != i; ++j)
+            Values.push_back(*j);
+          Values.push_back(MV);
+          for (++i; i != e; ++i)
+            Values.push_back(MapValue(*i, VM));
+          return VM[V] = MDNode::get(Values.data(), Values.size());
+        }
+      }
+      return VM[V] = C;
+
     } else {
       assert(0 && "Unknown type of constant!");
     }

lib/VMCore/AsmWriter.cpp

@@ -200,6 +200,7 @@ void TypePrinting::CalcTypeName(const Type *Ty,
   case Type::FP128TyID:     OS << "fp128"; break;
   case Type::PPC_FP128TyID: OS << "ppc_fp128"; break;
   case Type::LabelTyID:     OS << "label"; break;
+  case Type::MetadataTyID:  OS << "metadata"; break;
   case Type::IntegerTyID:
     OS << 'i' << cast<IntegerType>(Ty)->getBitWidth();
     break;

lib/VMCore/Constants.cpp

@@ -1664,7 +1664,7 @@ void UndefValue::destroyConstant() {
 //
 
 MDString::MDString(const char *begin, const char *end)
-  : Constant(Type::EmptyStructTy, MDStringVal, 0, 0),
+  : Constant(Type::MetadataTy, MDStringVal, 0, 0),
     StrBegin(begin), StrEnd(end) {}
 
 static ManagedStatic<StringMap<MDString*> > MDStringCache;
@@ -1689,7 +1689,7 @@ void MDString::destroyConstant() {
 static ManagedStatic<FoldingSet<MDNode> > MDNodeSet;
 
 MDNode::MDNode(Value*const* Vals, unsigned NumVals)
-  : Constant(Type::EmptyStructTy, MDNodeVal, 0, 0) {
+  : Constant(Type::MetadataTy, MDNodeVal, 0, 0) {
   for (unsigned i = 0; i != NumVals; ++i)
     Node.push_back(ElementVH(Vals[i], this));
 }

lib/VMCore/Type.cpp

@@ -97,6 +97,7 @@ const Type *Type::getPrimitiveType(TypeID IDNumber) {
   case FP128TyID     : return FP128Ty;
   case PPC_FP128TyID : return PPC_FP128Ty;
   case LabelTyID     : return LabelTy;
+  case MetadataTyID  : return MetadataTy;
   default:
     return 0;
   }
@@ -276,6 +277,7 @@ const Type *Type::X86_FP80Ty   = new Type(Type::X86_FP80TyID);
 const Type *Type::FP128Ty      = new Type(Type::FP128TyID);
 const Type *Type::PPC_FP128Ty  = new Type(Type::PPC_FP128TyID);
 const Type *Type::LabelTy      = new Type(Type::LabelTyID);
+const Type *Type::MetadataTy   = new Type(Type::MetadataTyID);
 
 namespace {
   struct BuiltinIntegerType : public IntegerType {
@@ -288,9 +290,6 @@ const IntegerType *Type::Int16Ty = new BuiltinIntegerType(16);
 const IntegerType *Type::Int32Ty = new BuiltinIntegerType(32);
 const IntegerType *Type::Int64Ty = new BuiltinIntegerType(64);
 
-const Type *Type::EmptyStructTy = StructType::get(NULL, NULL);
-
-
 //===----------------------------------------------------------------------===//
 //                          Derived Type Constructors
 //===----------------------------------------------------------------------===//
@@ -298,9 +297,13 @@ const Type *Type::EmptyStructTy = StructType::get(NULL, NULL);
 /// isValidReturnType - Return true if the specified type is valid as a return
 /// type.
 bool FunctionType::isValidReturnType(const Type *RetTy) {
-  if (RetTy->isFirstClassType())
+  if (RetTy->isFirstClassType()) {
+    if (const PointerType *PTy = dyn_cast<PointerType>(RetTy))
+      return PTy->getElementType() != Type::MetadataTy;
     return true;
-  if (RetTy == Type::VoidTy || isa<OpaqueType>(RetTy))
+  }
+  if (RetTy == Type::VoidTy || RetTy == Type::MetadataTy ||
+      isa<OpaqueType>(RetTy))
     return true;
   
   // If this is a multiple return case, verify that each return is a first class
@@ -330,6 +333,9 @@ FunctionType::FunctionType(const Type *Result,
   for (unsigned i = 0; i != Params.size(); ++i) {
     assert((Params[i]->isFirstClassType() || isa<OpaqueType>(Params[i])) &&
            "Function arguments must be value types!");
+    assert((!isa<PointerType>(Params[i]) ||
+            cast<PointerType>(Params[i])->getElementType() != Type::MetadataTy)
+           && "Attempt to use metadata* as function argument type!");
     new (&ContainedTys[i+1]) PATypeHandle(Params[i], this);
     isAbstract |= Params[i]->isAbstract();
   }
@@ -348,6 +354,10 @@ StructType::StructType(const std::vector<const Type*> &Types, bool isPacked)
     assert(Types[i] && "<null> type for structure field!");
     assert(Types[i] != Type::VoidTy && "Void type for structure field!");
     assert(Types[i] != Type::LabelTy && "Label type for structure field!");
+    assert(Types[i] != Type::MetadataTy && "Metadata type for structure field");
+    assert((!isa<PointerType>(Types[i]) ||
+            cast<PointerType>(Types[i])->getElementType() != Type::MetadataTy)
+           && "Type 'metadata*' is invalid for structure field.");
     new (&ContainedTys[i]) PATypeHandle(Types[i], this);
     isAbstract |= Types[i]->isAbstract();
   }
@@ -1043,6 +1053,10 @@ ArrayType *ArrayType::get(const Type *ElementType, uint64_t NumElements) {
   assert(ElementType && "Can't get array of <null> types!");
   assert(ElementType != Type::VoidTy && "Array of void is not valid!");
   assert(ElementType != Type::LabelTy && "Array of labels is not valid!");
+  assert(ElementType != Type::MetadataTy && "Array of metadata is not valid!");
+  assert((!isa<PointerType>(ElementType) ||
+          cast<PointerType>(ElementType)->getElementType() != Type::MetadataTy)
+         && "Array of metadata* is not valid!");
 
   ArrayValType AVT(ElementType, NumElements);
   ArrayType *AT = ArrayTypes->get(AVT);
@@ -1204,6 +1218,9 @@ PointerType *PointerType::get(const Type *ValueType, unsigned AddressSpace) {
   assert(ValueType != Type::VoidTy &&
          "Pointer to void is not valid, use i8* instead!");
   assert(ValueType != Type::LabelTy && "Pointer to label is not valid!");
+  assert((!isa<PointerType>(ValueType) ||
+          cast<PointerType>(ValueType)->getElementType() != Type::MetadataTy)
+         && "Pointer to metadata* is not valid!");
   PointerValType PVT(ValueType, AddressSpace);
 
   PointerType *PT = PointerTypes->get(PVT);

lib/VMCore/Verifier.cpp

@@ -280,6 +280,7 @@ namespace {
                      bool isReturnValue, const Value *V);
     void VerifyFunctionAttrs(const FunctionType *FT, const AttrListPtr &Attrs,
                              const Value *V);
+    bool VerifyMDNode(const MDNode *N);
 
     void WriteValue(const Value *V) {
       if (!V) return;
@@ -339,37 +340,6 @@ static RegisterPass<Verifier> X("verify", "Module Verifier");
 #define Assert4(C, M, V1, V2, V3, V4) \
   do { if (!(C)) { CheckFailed(M, V1, V2, V3, V4); return; } } while (0)
 
-/// Check whether or not a Value is metadata or made up of a constant
-/// expression involving metadata.
-static bool isMetadata(Value *X) {
-  SmallPtrSet<Value *, 8> Visited;
-  SmallVector<Value *, 8> Queue;
-  Queue.push_back(X);
-
-  while (!Queue.empty()) {
-    Value *V = Queue.back();
-    Queue.pop_back();
-    if (!Visited.insert(V))
-      continue;
-
-    if (isa<MDString>(V) || isa<MDNode>(V))
-      return true;
-    if (!isa<ConstantExpr>(V))
-      continue;
-    ConstantExpr *CE = cast<ConstantExpr>(V);
-
-    if (CE->getType() != Type::EmptyStructTy)
-      continue;
-
-    // The only constant expression that works on metadata type is select.
-    if (CE->getOpcode() != Instruction::Select) return false;
-
-    Queue.push_back(CE->getOperand(1));
-    Queue.push_back(CE->getOperand(2));
-  }
-  return false;
-}
-
 void Verifier::visit(Instruction &I) {
   for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i)
     Assert1(I.getOperand(i) != 0, "Operand is null", &I);
@@ -406,6 +376,30 @@ void Verifier::visitGlobalVariable(GlobalVariable &GV) {
     Assert1(GV.getInitializer()->getType() == GV.getType()->getElementType(),
             "Global variable initializer type does not match global "
             "variable type!", &GV);
+
+    // Verify that any metadata used in a global initializer points only to
+    // other globals.
+    if (MDNode *FirstNode = dyn_cast<MDNode>(GV.getInitializer())) {
+      if (VerifyMDNode(FirstNode)) {
+        SmallVector<const MDNode *, 4> NodesToAnalyze;
+        NodesToAnalyze.push_back(FirstNode);
+        while (!NodesToAnalyze.empty()) {
+          const MDNode *N = NodesToAnalyze.back();
+          NodesToAnalyze.pop_back();
+
+          for (MDNode::const_elem_iterator I = N->elem_begin(),
+                 E = N->elem_end(); I != E; ++I)
+            if (const Value *V = *I) {
+              if (const MDNode *Next = dyn_cast<MDNode>(V))
+                NodesToAnalyze.push_back(Next);
+              else
+                Assert3(isa<Constant>(V),
+                        "reference to instruction from global metadata node",
+                        &GV, N, V);
+            }
+        }
+      }
+    }
   } else {
     Assert1(GV.hasExternalLinkage() || GV.hasDLLImportLinkage() ||
             GV.hasExternalWeakLinkage(),
@@ -583,6 +577,12 @@ void Verifier::visitFunction(Function &F) {
     break;
   }
   
+  bool isLLVMdotName = F.getName().size() >= 5 &&
+                       F.getName().substr(0, 5) == "llvm.";
+  if (!isLLVMdotName)
+    Assert1(F.getReturnType() != Type::MetadataTy,
+            "Function may not return metadata unless it's an intrinsic", &F);
+
   // Check that the argument values match the function type for this function...
   unsigned i = 0;
   for (Function::arg_iterator I = F.arg_begin(), E = F.arg_end();
@@ -592,6 +592,9 @@ void Verifier::visitFunction(Function &F) {
             I, FT->getParamType(i));
     Assert1(I->getType()->isFirstClassType(),
             "Function arguments must have first-class types!", I);
+    if (!isLLVMdotName)
+      Assert2(I->getType() != Type::MetadataTy,
+              "Function takes metadata but isn't an intrinsic", I, &F);
   }
 
   if (F.isDeclaration()) {
@@ -601,9 +604,7 @@ void Verifier::visitFunction(Function &F) {
   } else {
     // Verify that this function (which has a body) is not named "llvm.*".  It
     // is not legal to define intrinsics.
-    if (F.getName().size() >= 5)
+    Assert1(!isLLVMdotName, "llvm intrinsics cannot be defined!", &F);
-      Assert1(F.getName().substr(0, 5) != "llvm.",
-              "llvm intrinsics cannot be defined!", &F);
     
     // Check the entry node
     BasicBlock *Entry = &F.getEntryBlock();
@@ -682,7 +683,6 @@ void Verifier::visitReturnInst(ReturnInst &RI) {
             "Found return instr that returns non-void in Function of void "
             "return type!", &RI, F->getReturnType());
   else if (N == 1 && F->getReturnType() == RI.getOperand(0)->getType()) {
-    Assert1(!isMetadata(RI.getOperand(0)), "Invalid use of metadata!", &RI);
     // Exactly one return value and it matches the return type. Good.
   } else if (const StructType *STy = dyn_cast<StructType>(F->getReturnType())) {
     // The return type is a struct; check for multiple return values.
@@ -730,8 +730,6 @@ void Verifier::visitSelectInst(SelectInst &SI) {
 
   Assert1(SI.getTrueValue()->getType() == SI.getType(),
           "Select values must have same type as select instruction!", &SI);
-  Assert1(!isMetadata(SI.getOperand(1)) && !isMetadata(SI.getOperand(2)),
-          "Invalid use of metadata!", &SI);
   visitInstruction(SI);
 }
 
@@ -987,13 +985,6 @@ void Verifier::visitPHINode(PHINode &PN) {
     Assert1(PN.getType() == PN.getIncomingValue(i)->getType(),
             "PHI node operands are not the same type as the result!", &PN);
 
-  // Check that it's not a PHI of metadata.
-  if (PN.getType() == Type::EmptyStructTy) {
-    for (unsigned i = 0, e = PN.getNumIncomingValues(); i != e; ++i)
-      Assert1(!isMetadata(PN.getIncomingValue(i)),
-              "Invalid use of metadata!", &PN);
-  }
-
   // All other PHI node constraints are checked in the visitBasicBlock method.
 
   visitInstruction(PN);
@@ -1024,14 +1015,6 @@ void Verifier::VerifyCallSite(CallSite CS) {
             "Call parameter type does not match function signature!",
             CS.getArgument(i), FTy->getParamType(i), I);
 
-  if (CS.getCalledValue()->getNameLen() < 5 ||
-      strncmp(CS.getCalledValue()->getNameStart(), "llvm.", 5) != 0) {
-    // Verify that none of the arguments are metadata...
-    for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i)
-      Assert2(!isMetadata(CS.getArgument(i)), "Invalid use of metadata!",
-              CS.getArgument(i), I);
-  }
-
   const AttrListPtr &Attrs = CS.getAttributes();
 
   Assert1(VerifyAttributeCount(Attrs, CS.arg_size()),
@@ -1052,6 +1035,17 @@ void Verifier::VerifyCallSite(CallSite CS) {
               " cannot be used for vararg call arguments!", I);
     }
 
+  // Verify that there's no metadata unless it's a direct call to an intrinsic.
+  if (!CS.getCalledFunction() || CS.getCalledFunction()->getName().size() < 5 ||
+      CS.getCalledFunction()->getName().substr(0, 5) != "llvm.") {
+    Assert1(FTy->getReturnType() != Type::MetadataTy,
+            "Only intrinsics may return metadata", I);
+    for (FunctionType::param_iterator PI = FTy->param_begin(),
+           PE = FTy->param_end(); PI != PE; ++PI)
+      Assert1(PI->get() != Type::MetadataTy, "Function has metadata parameter "
+              "but isn't an intrinsic", I);
+  }
+
   visitInstruction(*I);
 }
 
@@ -1120,6 +1114,7 @@ void Verifier::visitICmpInst(ICmpInst& IC) {
   // Check that the operands are the right type
   Assert1(Op0Ty->isIntOrIntVector() || isa<PointerType>(Op0Ty),
           "Invalid operand types for ICmp instruction", &IC);
+
   visitInstruction(IC);
 }
 
@@ -1195,6 +1190,7 @@ void Verifier::visitLoadInst(LoadInst &LI) {
     cast<PointerType>(LI.getOperand(0)->getType())->getElementType();
   Assert2(ElTy == LI.getType(),
           "Load result type does not match pointer operand type!", &LI, ElTy);
+  Assert1(ElTy != Type::MetadataTy, "Can't load metadata!", &LI);
   visitInstruction(LI);
 }
 
@@ -1203,7 +1199,7 @@ void Verifier::visitStoreInst(StoreInst &SI) {
     cast<PointerType>(SI.getOperand(1)->getType())->getElementType();
   Assert2(ElTy == SI.getOperand(0)->getType(),
           "Stored value type does not match pointer operand type!", &SI, ElTy);
-  Assert1(!isMetadata(SI.getOperand(0)), "Invalid use of metadata!", &SI);
+  Assert1(ElTy != Type::MetadataTy, "Can't store metadata!", &SI);
   visitInstruction(SI);
 }
 
@@ -1264,6 +1260,17 @@ void Verifier::visitInstruction(Instruction &I) {
               && isa<StructType>(I.getType())),
           "Instruction returns a non-scalar type!", &I);
 
+  // Check that the instruction doesn't produce metadata or metadata*. Calls
+  // all already checked against the callee type.
+  Assert1(I.getType() != Type::MetadataTy ||
+          isa<CallInst>(I) || isa<InvokeInst>(I),
+          "Invalid use of metadata!", &I);
+
+  if (const PointerType *PTy = dyn_cast<PointerType>(I.getType()))
+    Assert1(PTy->getElementType() != Type::MetadataTy,
+            "Instructions may not produce pointer to metadata.", &I);
+
+
   // Check that all uses of the instruction, if they are instructions
   // themselves, actually have parent basic blocks.  If the use is not an
   // instruction, it is an error!
@@ -1285,6 +1292,11 @@ void Verifier::visitInstruction(Instruction &I) {
       Assert1(0, "Instruction operands must be first-class values!", &I);
     }
 
+    if (const PointerType *PTy =
+            dyn_cast<PointerType>(I.getOperand(i)->getType()))
+      Assert1(PTy->getElementType() != Type::MetadataTy,
+              "Invalid use of metadata pointer.", &I);
+    
     if (Function *F = dyn_cast<Function>(I.getOperand(i))) {
       // Check to make sure that the "address of" an intrinsic function is never
       // taken.
@@ -1678,6 +1690,44 @@ void Verifier::VerifyIntrinsicPrototype(Intrinsic::ID ID, Function *F,
           "Intrinsic has wrong parameter attributes!", F);
 }
 
+/// Verify that an MDNode is not cyclic.
+bool Verifier::VerifyMDNode(const MDNode *N) {
+  if (N->elem_empty()) return true;
+
+  // The current DFS path through the nodes. Node and element number.
+  typedef std::pair<const MDNode *, MDNode::const_elem_iterator> Edge;
+  SmallVector<Edge, 8> Path;
+
+  Path.push_back(std::make_pair(N, N->elem_begin()));
+  while (!Path.empty()) {
+    Edge &e = Path.back();
+    const MDNode *&e_N = e.first;
+    MDNode::const_elem_iterator &e_I = e.second;
+
+    if (e_N->elem_end() == e_I) {
+      Path.pop_back();
+      continue;
+    }
+
+    for (MDNode::const_elem_iterator e_E = e_N->elem_end(); e_I != e_E; ++e_I) {
+      if (const MDNode *C = dyn_cast_or_null<MDNode>(e_I->operator Value*())) {
+        // Is child MDNode C already in the Path?
+        for (SmallVectorImpl<Edge>::iterator I = Path.begin(), E = Path.end();
+             I != E; ++I) {
+          if (I->first != C) {
+            CheckFailed("MDNode is cyclic.", C);
+            return false;
+          }
+        }
+
+        Path.push_back(std::make_pair(C, C->elem_begin()));
+        break;
+      }
+    }
+  }
+  return true;
+}
+
 
 //===----------------------------------------------------------------------===//
 //  Implement the public interfaces to this file...

test/Feature/embeddedmetadata.ll

@@ -1,11 +1,11 @@
 ; RUN: llvm-as < %s | llvm-dis | not grep undef
 
-declare i8 @llvm.something({ } %a)
+declare i8 @llvm.something(metadata %a)
 
-@llvm.foo = internal constant { } !{i17 123, null, { } !"foobar"}
+@llvm.foo = internal constant metadata !{i17 123, null, metadata !"foobar"}
 
 define void @foo() {
-  %x = call i8 @llvm.something({ } !{{ } !"f\00oa", i42 123})
+  %x = call i8 @llvm.something(metadata !{metadata !"f\00oa", i42 123})
   ret void
 }
 

unittests/VMCore/MetadataTest.cpp

@@ -50,7 +50,7 @@ TEST(MDStringTest, PrintingSimple) {
 
   std::ostringstream oss;
   s->print(oss);
-  EXPECT_STREQ("{ } !\"testing 1 2 3\"", oss.str().c_str());
+  EXPECT_STREQ("metadata !\"testing 1 2 3\"", oss.str().c_str());
 }
 
 // Test printing of MDString with non-printable characters.
@@ -59,7 +59,7 @@ TEST(MDStringTest, PrintingComplex) {
   MDString *s = MDString::get(str+0, str+5);
   std::ostringstream oss;
   s->print(oss);
-  EXPECT_STREQ("{ } !\"\\00\\0A\\22\\5C\\FF\"", oss.str().c_str());
+  EXPECT_STREQ("metadata !\"\\00\\0A\\22\\5C\\FF\"", oss.str().c_str());
 }
 
 // Test the two constructors, and containing other Constants.
@@ -94,8 +94,10 @@ TEST(MDNodeTest, Simple) {
   std::ostringstream oss1, oss2;
   n1->print(oss1);
   n2->print(oss2);
-  EXPECT_STREQ("{ } !{{ } !\"abc\", i8 0, { } !\"123\"}", oss1.str().c_str());
+  EXPECT_STREQ("metadata !{metadata !\"abc\", i8 0, metadata !\"123\"}",
-  EXPECT_STREQ("{ } !{{ } !{{ } !\"abc\", i8 0, { } !\"123\"}}",
+               oss1.str().c_str());
+  EXPECT_STREQ("metadata !{metadata !{metadata !\"abc\", i8 0, "
+                                     "metadata !\"123\"}}",
                oss2.str().c_str());
 }
 
@@ -132,6 +134,6 @@ TEST(MDNodeTest, Delete) {
 
   std::ostringstream oss;
   wvh->print(oss);
-  EXPECT_STREQ("{ } !{null}", oss.str().c_str());
+  EXPECT_STREQ("metadata !{null}", oss.str().c_str());
 }
 }