[PATCH] IR: Make metadata typeless in assembly

[Robinson, Paul]

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<mailto:dexonsmith at apple.com>> wrote:

> On 2014 Dec 10, at 20:35, Robinson, Paul <Paul_Robinson at playstation.sony.com<mailto: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> [mailto: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<mailto: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<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141212/3231f93a/attachment.html>