[PATCH] IR: Make metadata typeless in assembly

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Dec 11 16:26:09 PST 2014


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
>  
>  
> From: Eric Christopher [mailto:echristo at gmail.com] 
> Sent: Thursday, December 11, 2014 4:07 PM
> To: Duncan P. N. Exon Smith; Robinson, Paul
> Cc: llvm-commits
> Subject: Re: [PATCH] IR: Make metadata typeless in assembly
>  
> Hi Duncan,
> 
> Looking through the LangRef as requested :)
>  
> +    A metadata node is a structure-like constant without a type (when
> +    referenced by ``call`` instructions, they use :ref:`metadata type
> +    <t_metadata>`). For example: "``!{ value i32 0, !"test" }``". Unlike other
> +    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.
>  
> This is fairly opaque wording. Honestly I'd prefer "a constant tuple without types". The last probably needs/wants rewording as well, it's just not fabulous. Perhaps even an actual separate section would work well for metadata now that it's separate from the value hierarchy?
>  
> The rest of the changes look fairly mechanical given the new description of the format.
>  
> Thanks!
>  
> -eric
>  
>  
> On Wed Dec 10 2014 at 10:42:34 PM Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> > On 2014 Dec 10, at 20:35, Robinson, Paul <Paul_Robinson at playstation.sony.com> wrote:
> >
> > Note: in case it's confusing, some of the testcase changes in the code
> > patch are killing newlines so the upgrade script can do it's magic.
> >
> > That part really feels like a purely mechanical change that oughta be separate.
> > --paulr
> 
> Thanks Paul, good point.  Committed that in r224002.
> 
> Attaching the rebased patch.  Let me know if you have any comments on LangRef!
> 
> 
> >
> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Duncan P. N. Exon Smith
> > Sent: Wednesday, December 10, 2014 6:17 PM
> > To: llvm-commits
> > Subject: [PATCH] IR: Make metadata typeless in assembly
> >
> > Now that `Metadata` is typeless, reflect that in the assembly.
> >
> > There's really only one reasonably thing to do here: strip the
> > `metadata` type (except in arguments to `call` instructions, where it
> > still has meaning), and add the symmetric `value` keyword for all
> > metadata operands that bridge to `Value`.
> >
> > I'm confident in the code changes (although obviously I'll relish any
> > criticism offered), but I'd love a review of `docs/LangRef.rst`.  I
> > wasn't really sure how much exposition was appropriate, so I kept it
> > pretty simple.
> >
> > Also, I wrote an upgrade script that handle all of the tests in llvm
> > after applying the main patch.  It even handles much of cfe (doing
> > mostly the right thing with `CHECK` lines).  I'll be attaching it the PR
> > after I make my commit, but feel free to spot-check its enormous result,
> > which I've attached as llvm-tests.patch.  (I'll squash that in with the
> > code changes when I commit.)
> >
> > Note: in case it's confusing, some of the testcase changes in the code
> > patch are killing newlines so the upgrade script can do it's magic.
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> 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