[llvm-commits] metadata type

[Nick Lewycky]

Thanks for the review! Updated patch attached.

Frits van Bommel wrote:
> Nick Lewycky wrote:
>> This patch switches metadata away from using "{ }" type to its own new 
>> "metadata" type. As a side-effect this patch makes PHI nodes on {} legal 
>> again, similarly with %x = load {}* resulting in {}, etc.
>>
>> Please review!
> 
> --- docs/LangRef.html	(revision 72315)
> +++ docs/LangRef.html	(working copy)
> @@ -2046,8 +2063,8 @@
> 
>   <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
> -empty struct and is identified in syntax by a preceding exclamation point
> +two metadata primitives, strings and nodes. All metadata has the
> +<tt>metadata</tt> and is identified in syntax by a preceding exclamation point
>   ('<tt>!</tt>').
> 
> You probably meant "All metadata has the metadata *type*"

Yup! Fixed.

> 
> --- include/llvm/Constants.h	(revision 72315)
> +++ include/llvm/Constants.h	(working copy)
> @@ -863,7 +863,7 @@
>     /// getType() specialization - Type is always an empty struct.
>     ///
>     inline const Type *getType() const {
> -    return Type::EmptyStructTy;
> +    return Type::MetadataTy;
>     }
> 
> The comment should also be updated. (The same applies for MDNode::getType() in 
> MDNode.h)

Fixed both, thanks.

> 
> --- include/llvm/Bitcode/LLVMBitCodes.h	(revision 72315)
> +++ include/llvm/Bitcode/LLVMBitCodes.h	(working copy)
> @@ -78,6 +78,7 @@
>       TYPE_CODE_DOUBLE   =  4,   // DOUBLE
>       TYPE_CODE_LABEL    =  5,   // LABEL
>       TYPE_CODE_OPAQUE   =  6,   // OPAQUE
> +    TYPE_CODE_METADATA = 16,   // METADATA
> 
> This should probably be at the bottom, like the bigger floating-point types. (To 
> keep the numbers in-order)

Sure.

> 
> --- lib/VMCore/Verifier.cpp	(revision 72315)
> +++ lib/VMCore/Verifier.cpp	(working copy)
> @@ -1052,6 +1035,16 @@
>                 " 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(), "Only intrinsics may return metadata", I);
> 
> Don't you actually want to *check* whether it's metadata? :)

Oops! Fixed.

Nick

-------------- next part --------------
A non-text attachment was scrubbed...
Name: metadata-type2.patch
Type: text/x-diff
Size: 24080 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090523/1f0d9686/attachment.patch>