[llvm-commits] [llvm] r74630 - in /llvm/trunk: lib/AsmParser/LLParser.cpp lib/AsmParser/LLParser.h lib/VMCore/AsmWriter.cpp test/Feature/mdnode2.ll

Nick Lewycky nicholas at mxc.ca
Wed Jul 1 21:15:15 PDT 2009


Hi Devang, I have a concern about this patch.

You're assigning the numbers as you go while printing instead of storing 
them as a vector in the Module. This has the exact same behaviour when 
you print out the entire module, but I'm worried that if you "cout << 
Inst1; cout << Inst2;" where Inst1 and Inst2 both refer to two different 
metadata nodes then the numbering will reuse !0.

If you want to avoid keeping a vector<MDNode> in the Module, the best 
thing to do would be to only use numbered metadata when printing the 
whole module and otherwise print out the full contents of the metadata node.

That should be pretty easy to verify by adding a unit test to 
unittests/VMCore/MetadataTest.cpp.

Nick

Devang Patel wrote:
> Author: dpatel
> Date: Wed Jul  1 14:21:12 2009
> New Revision: 74630
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=74630&view=rev
> Log:
> Support stand alone metadata syntax.
> 
> !0 = constant metadata !{i32 21, i32 22}
> @llvm.blah = constant metadata !{i32 1000, i16 200, metadata !0}
> 
> 
> Added:
>     llvm/trunk/test/Feature/mdnode2.ll
> Modified:
>     llvm/trunk/lib/AsmParser/LLParser.cpp
>     llvm/trunk/lib/AsmParser/LLParser.h
>     llvm/trunk/lib/VMCore/AsmWriter.cpp
> 
> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=74630&r1=74629&r2=74630&view=diff
> 
> ==============================================================================
> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Wed Jul  1 14:21:12 2009
> @@ -109,6 +109,7 @@
>      case lltok::StringConstant: // FIXME: REMOVE IN LLVM 3.0
>      case lltok::LocalVar:   if (ParseNamedType()) return true; break;
>      case lltok::GlobalVar:  if (ParseNamedGlobal()) return true; break;
> +    case lltok::Metadata:   if (ParseStandaloneMetadata()) return true; break;
>  
>      // The Global variable production with no name can have many different
>      // optional leading prefixes, the production is:
> @@ -355,6 +356,34 @@
>    return ParseAlias(Name, NameLoc, Visibility);
>  }
>  
> +/// ParseStandaloneMetadata:
> +///   !42 = !{...} 
> +bool LLParser::ParseStandaloneMetadata() {
> +  assert(Lex.getKind() == lltok::Metadata);
> +  Lex.Lex();
> +  unsigned MetadataID = 0;
> +  if (ParseUInt32(MetadataID))
> +    return true;
> +  if (MetadataCache.find(MetadataID) != MetadataCache.end())
> +    return TokError("Metadata id is already used");
> +  if (ParseToken(lltok::equal, "expected '=' here"))
> +    return true;
> +
> +  LocTy TyLoc;
> +  bool IsConstant;    
> +  PATypeHolder Ty(Type::VoidTy);
> +  if (ParseGlobalType(IsConstant) ||
> +      ParseType(Ty, TyLoc))
> +    return true;
> +  
> +  Constant *Init = 0;
> +  if (ParseGlobalValue(Ty, Init))
> +      return true;
> +
> +  MetadataCache[MetadataID] = Init;
> +  return false;
> +}
> +
>  /// ParseAlias:
>  ///   ::= GlobalVar '=' OptionalVisibility 'alias' OptionalLinkage Aliasee
>  /// Aliasee
> @@ -1596,6 +1625,17 @@
>        return false;
>      }
>  
> +    // Standalone metadata reference
> +    // !{ ..., !42, ... }
> +    unsigned MID = 0;
> +    if (!ParseUInt32(MID)) {
> +      std::map<unsigned, Constant *>::iterator I = MetadataCache.find(MID);
> +      if (I == MetadataCache.end())
> +	return TokError("Unknown metadata reference");
> +      ID.ConstantVal = I->second;
> +      return false;
> +    }
> +    
>      // MDString:
>      //   ::= '!' STRINGCONSTANT
>      std::string Str;
> 
> Modified: llvm/trunk/lib/AsmParser/LLParser.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.h?rev=74630&r1=74629&r2=74630&view=diff
> 
> ==============================================================================
> --- llvm/trunk/lib/AsmParser/LLParser.h (original)
> +++ llvm/trunk/lib/AsmParser/LLParser.h Wed Jul  1 14:21:12 2009
> @@ -43,7 +43,8 @@
>      std::map<std::string, std::pair<PATypeHolder, LocTy> > ForwardRefTypes;
>      std::map<unsigned, std::pair<PATypeHolder, LocTy> > ForwardRefTypeIDs;
>      std::vector<PATypeHolder> NumberedTypes;
> -
> +    /// MetadataCache - This map keeps track of parsed metadata constants.
> +    std::map<unsigned, Constant *> MetadataCache;
>      struct UpRefRecord {
>        /// Loc - This is the location of the upref.
>        LocTy Loc;
> @@ -139,6 +140,7 @@
>      bool ParseGlobal(const std::string &Name, LocTy Loc, unsigned Linkage,
>                       bool HasLinkage, unsigned Visibility);
>      bool ParseAlias(const std::string &Name, LocTy Loc, unsigned Visibility);
> +    bool ParseStandaloneMetadata();
>  
>      // Type Parsing.
>      bool ParseType(PATypeHolder &Result, bool AllowVoid = false);
> 
> Modified: llvm/trunk/lib/VMCore/AsmWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/AsmWriter.cpp?rev=74630&r1=74629&r2=74630&view=diff
> 
> ==============================================================================
> --- llvm/trunk/lib/VMCore/AsmWriter.cpp (original)
> +++ llvm/trunk/lib/VMCore/AsmWriter.cpp Wed Jul  1 14:21:12 2009
> @@ -35,6 +35,7 @@
>  #include "llvm/Support/raw_ostream.h"
>  #include <algorithm>
>  #include <cctype>
> +#include <map>
>  using namespace llvm;
>  
>  // Make virtual table appear in this compilation unit.
> @@ -945,25 +946,6 @@
>      return;
>    }
>  
> -  if (const MDNode *N = dyn_cast<MDNode>(CV)) {
> -    Out << "!{";
> -    for (MDNode::const_elem_iterator I = N->elem_begin(), E = N->elem_end();
> -         I != E;) {
> -      if (!*I) {
> -        Out << "null";
> -      } else {
> -        TypePrinter.print((*I)->getType(), Out);
> -        Out << ' ';
> -        WriteAsOperandInternal(Out, *I, TypePrinter, Machine);
> -      }
> -
> -      if (++I != E)
> -        Out << ", ";
> -    }
> -    Out << "}";
> -    return;
> -  }
> -  
>    if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(CV)) {
>      Out << CE->getOpcodeName();
>      if (CE->isCompare())
> @@ -1092,10 +1074,14 @@
>    TypePrinting TypePrinter;
>    AssemblyAnnotationWriter *AnnotationWriter;
>    std::vector<const Type*> NumberedTypes;
> +
> +  // Each MDNode is assigned unique MetadataIDNo.
> +  std::map<const MDNode *, unsigned> MDNodes;
> +  unsigned MetadataIDNo;
>  public:
>    inline AssemblyWriter(raw_ostream &o, SlotTracker &Mac, const Module *M,
>                          AssemblyAnnotationWriter *AAW)
> -    : Out(o), Machine(Mac), TheModule(M), AnnotationWriter(AAW) {
> +    : Out(o), Machine(Mac), TheModule(M), AnnotationWriter(AAW), MetadataIDNo(0) {
>      AddModuleTypesToPrinter(TypePrinter, NumberedTypes, M);
>    }
>  
> @@ -1124,6 +1110,7 @@
>    void printModule(const Module *M);
>    void printTypeSymbolTable(const TypeSymbolTable &ST);
>    void printGlobal(const GlobalVariable *GV);
> +  void printMDNode(const MDNode *Node, bool StandAlone);
>    void printAlias(const GlobalAlias *GV);
>    void printFunction(const Function *F);
>    void printArgument(const Argument *FA, Attributes Attrs);
> @@ -1264,6 +1251,28 @@
>  }
>  
>  void AssemblyWriter::printGlobal(const GlobalVariable *GV) {
> +  if (GV->hasInitializer())
> +    // If GV is initialized using Metadata then separate out metadata
> +    // operands used by the initializer. Note, MDNodes are not cyclic.
> +    if (MDNode *N = dyn_cast<MDNode>(GV->getInitializer())) {
> +      SmallVector<const MDNode *, 4> WorkList;
> +      // Collect MDNodes used by the initializer.
> +      for (MDNode::const_elem_iterator I = N->elem_begin(), E = N->elem_end();
> +	   I != E; ++I) {
> +	const Value *TV = *I;
> +	if (TV)
> +	  if (const MDNode *NN = dyn_cast<MDNode>(TV))
> +	    WorkList.push_back(NN);
> +      }
> +
> +      // Print MDNodes used by the initializer.
> +      while (!WorkList.empty()) {
> +	const MDNode *N = WorkList.back(); WorkList.pop_back();
> +	printMDNode(N, true);
> +	Out << '\n';
> +      }
> +    }
> +
>    if (GV->hasName()) {
>      PrintLLVMName(Out, GV);
>      Out << " = ";
> @@ -1283,7 +1292,10 @@
>  
>    if (GV->hasInitializer()) {
>      Out << ' ';
> -    writeOperand(GV->getInitializer(), false);
> +    if (MDNode *N = dyn_cast<MDNode>(GV->getInitializer()))
> +      printMDNode(N, false);
> +    else
> +      writeOperand(GV->getInitializer(), false);
>    }
>      
>    if (GV->hasSection())
> @@ -1295,6 +1307,42 @@
>    Out << '\n';
>  }
>  
> +void AssemblyWriter::printMDNode(const MDNode *Node,
> +				 bool StandAlone) {
> +  std::map<const MDNode *, unsigned>::iterator MI = MDNodes.find(Node);
> +  // If this node is already printed then just refer it using its Metadata
> +  // id number.
> +  if (MI != MDNodes.end()) {
> +    Out << "metadata !" << MI->second;
> +    return;
> +  }
> +  
> +  if (StandAlone) {
> +    // Print standalone MDNode.
> +    // !42 = !{ ... }
> +    Out << "!" << MetadataIDNo << " = ";
> +    Out << "constant metadata ";
> +  }
> +  Out << "!{";
> +  for (MDNode::const_elem_iterator I = Node->elem_begin(), E = Node->elem_end();
> +       I != E;) {
> +    const Value *TV = *I;
> +    if (!TV)
> +      Out << "null";
> +    else if (const MDNode *N = dyn_cast<MDNode>(TV)) 
> +      printMDNode(N, StandAlone);
> +    else if (!*I)
> +      Out << "null";
> +    else 
> +      writeOperand(*I, true);
> +    if (++I != E)
> +      Out << ", ";
> +  }
> +  Out << "}";
> +
> +  MDNodes[Node] = MetadataIDNo++;
> +}
> +
>  void AssemblyWriter::printAlias(const GlobalAlias *GA) {
>    // Don't crash when dumping partially built GA
>    if (!GA->hasName())
> 
> Added: llvm/trunk/test/Feature/mdnode2.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Feature/mdnode2.ll?rev=74630&view=auto
> 
> ==============================================================================
> --- llvm/trunk/test/Feature/mdnode2.ll (added)
> +++ llvm/trunk/test/Feature/mdnode2.ll Wed Jul  1 14:21:12 2009
> @@ -0,0 +1,7 @@
> +; RUN: llvm-as < %s | llvm-dis > %t.ll
> +; RUN: grep "!0 = constant metadata !{i32 21, i32 22}" %t.ll
> +; RUN: grep "!1 = constant metadata !{i32 23, i32 24}" %t.ll
> +; RUN: grep "@llvm.blah = constant metadata !{i32 1000, i16 200, metadata !1, metadata !0}" %t.ll
> +!0 = constant metadata !{i32 21, i32 22}
> +!1 = constant metadata !{i32 23, i32 24}
> + at llvm.blah = constant metadata !{i32 1000, i16 200, metadata !1, metadata !0}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 




More information about the llvm-commits mailing list