[PATCH] IR: Make metadata typeless in assembly

[Robinson, Paul]

Yeah, sorry, just started looking at this about 10 minutes ago.
Word-smithing comments about the LangRef description, no worries on the code front.

+    A metadata node is a constant tuple without types (when referenced by
+    ``call`` instructions, they use :ref:`metadata type <t_metadata>`). For
+    example: "``!{ i32 0, !"test" }``". Unlike other typed constants that are
+    meant to be interpreted as part of the instruction stream, metadata is a
+    place to attach additional information such as debug info.

Would read a little better if the "call" bit comes after the example (otherwise the example intuitively feels like it's for "call" which it isn't).
But then "without types" is immediately followed by "i32 0" which is a typed value… needs more consideration.

Sorry to hold up your commit, didn't mean to get in the way so much.  We can fix up the wording in LangRef as needed post-commit.
--paulr

From: Duncan P. N. Exon Smith [mailto:dexonsmith at apple.com]
Sent: Friday, December 12, 2014 3:03 PM
To: Robinson, Paul
Cc: Eric Christopher; llvm-commits
Subject: Re: [PATCH] IR: Make metadata typeless in assembly


> On 2014 Dec 11, at 17:57, Duncan P. N. Exon Smith <dexonsmith at apple.com<mailto:dexonsmith at apple.com>> wrote:
>
> Okay, so I went right down the rabbit hole with this, and I've attached
> a new patch that skips the `value` keyword.  I have a new upgrade script
> that gets all the LLVM tests passing.
>
> The end result looks obviously better to me.  I wasn't sure if there'd
> be ambiguities between complex types (starting with `%{<[`) and other
> metadata operands, but it looks like all metadata operands either start
> with `!` or are `null`.  This adds a restriction on syntax for new
> types of metadata to be unambiguous w.r.t. types, but I don't think
> that's serious.
>
> Haven't attached the upgrade script or massive test patch here (although
> I have updated LangRef as per Eric's suggestions).
>
> Paul, let me know if this looks better to you too.
>

*ping*

Sorry Paul, I know this is fast, but I don't want to leave assembly
out-of-sync with the other two forms of IR (C++ and bitcode) any longer
than necessary.

It's too late for me to commit this today (as out-of-tree frontends and
backends would have broken builds all weekend), but I'd like to commit
it first thing on Monday if at all possible.

Did my updated patch look better to you?  Do you have bigger concerns
that require a longer conversation?


>
>> On 2014 Dec 11, at 16:26, Duncan P. N. Exon Smith <dexonsmith at apple.com<mailto:dexonsmith at apple.com>> wrote:
>>
>> Actually, maybe we don't need that after all.
>>
>> I had expected to need the `value` keyword to detect whether the operand
>> would take a type.  I'm not so sure now that I've written the parser (and
>> the nice errors about invalid types).  I'll see what I can do; give me a
>> few minutes.
>>
>>
>>> On 2014 Dec 11, at 16:16, Robinson, Paul <Paul_Robinson at playstation.sony.com<mailto:Paul_Robinson at playstation.sony.com>> wrote:
>>>
>>> Duncan,
>>> Remind me why it's important to have the 'value' keyword?  This bikeshed was
>>> probably painted a while back but syntactically it doesn't seem too useful.
>>> It seems like you have (a) stuff starting with '!' is a metadata node/reference,
>>> (b) everything else is a value (and must start with a type name).  There's
>>> probably something I'm forgetting, that made 'value' useful…
>>> Thanks,
>>> --paulr
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141212/1c03ce79/attachment.html>