[llvm-commits] [llvm] r92761 - in /llvm/trunk: docs/LangRef.html include/llvm/Metadata.h lib/AsmParser/LLParser.cpp lib/Bitcode/Reader/BitcodeReader.cpp lib/VMCore/Metadata.cpp unittests/VMCore/MetadataTest.cpp

[Chris Lattner]

On Jan 5, 2010, at 12:41 PM, Devang Patel wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=92761&view=rev
> Log:
> NamedMDNode is a collection MDNodes.

Thanks Devang.

> +++ llvm/trunk/docs/LangRef.html Tue Jan  5 14:41:31 2010

You should also mention these in #modulestructure and add a top level  
section to "High Level Structure" after #aliasstructure describing  
these.  Please describe what the constraints are on these: the operand  
has to be an MDNode or null.

@@ -2329,6 +2329,9 @@
>    event that a value is deleted, it will be replaced with a typeless
>    "<tt>null</tt>", such as "<tt>metadata !{null, i32 10}</tt>".</p>
>
> +<p>A named metadata is a collection of metadata nodes. For example:  
> "<tt>!foo =
> +   metadata !{!4, !3}</tt>".

Please link to the section you add above.

In the TOC and in the document, Embedded Metadata should not be listed  
under Constants anymore, it should move to Other Values.  The Embedded  
Metadata section should be renamed "Metadata Nodes and Metadata  
Strings".

> +++ llvm/trunk/include/llvm/Metadata.h Tue Jan  5 14:41:31 2010
> @@ -167,7 +167,7 @@
> };
>
> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> -/// NamedMDNode - a tuple of other metadata.
> +/// NamedMDNode - a tuple of MDNodes.
> /// NamedMDNode is always named. All NamedMDNode operand has a type  
> of metadata.
> class NamedMDNode : public MetadataBase, public  
> ilist_node<NamedMDNode> {

This should inherit from Value, not MetadataBase.
>
> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Tue Jan  5 14:41:31 2010

Ok.

> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Tue Jan  5  
> 14:41:31 2010
> @@ -787,11 +787,11 @@
>
>       // Read named metadata elements.
>       unsigned Size = Record.size();
> -      SmallVector<MetadataBase*, 8> Elts;
> +      SmallVector<MDNode *, 8> Elts;
>       for (unsigned i = 0; i != Size; ++i) {
>         Value *MD = MDValueList.getValueFwdRef(Record[i]);
> -        if (MetadataBase *B = dyn_cast<MetadataBase>(MD))
> -        Elts.push_back(B);
> +        if (MDNode *B = dyn_cast_or_null<MDNode>(MD))
> +          Elts.push_back(B);

This isn't great.  Three cases here: null is always fine.  MDNode is  
also fine, non-MDNode is not fine.

You should either use cast_or_null or you should check for non-null  
and non-MDNodes and reject them with an error.

> +++ llvm/trunk/lib/VMCore/Metadata.cpp Tue Jan  5 14:41:31 2010
> @@ -252,14 +252,14 @@
> }
>
> /// getOperand - Return specified operand.
> -MetadataBase *NamedMDNode::getOperand(unsigned i) const {
> +MDNode *NamedMDNode::getOperand(unsigned i) const {
>   assert(i < getNumOperands() && "Invalid Operand number!");
> -  return getNMDOps(Operands)[i];
> +  return dyn_cast_or_null<MDNode>(getNMDOps(Operands)[i]);

This should be cast_or_null.

Thanks for cleaning this up Devang,

-Chris