[PATCH] IR: Make metadata typeless in assembly

[Duncan P. N. Exon Smith]

> On 2014 Dec 12, at 15:15, Robinson, Paul <Paul_Robinson at playstation.sony.com> wrote:
> 
> 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.

Good point, I'll adjust it once more before commit, and we can go from
there.

>  
> 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

No problem, Monday might be a better day anyway :).  Just wanted to make
sure there wasn't anything bigger!

>  
> 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> 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> 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> 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
> >>> 
>