<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 13, 2014 at 11:17 AM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On Nov 12, 2014, at 1:00 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
><br>
> If you don't care about function-local metadata and debug info<br>
> intrinsics, skip ahead to the section on assembly syntax in case you<br>
> have comments on that.<br>
><br>
>> On 2014-Nov-09, at 17:02, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
>><br>
>> 2. No more function-local metadata.<br>
>><br>
>>   AFAICT, function-local metadata is *only* used for indirect<br>
>>   references to instructions and arguments in `@llvm.dbg.value` and<br>
>>   `@llvm.dbg.declare` intrinsics.  The first argument of the following<br>
>>   is an example:<br>
>><br>
>>       call void @llvm.dbg.value(metadata !{i32 %val}, metadata !0,<br>
>>                                 metadata !1)<br>
>><br>
>>   Note that the debug info people uniformly seem to dislike the status<br>
>>   quo, since it's awkward to get from a `Value` to the corresponding<br>
>>   intrinsic.<br>
>><br>
>>   Upgrade path: Instead of using an intrinsic that references a<br>
>>   function-local value through an `MDNode`, attach metadata to the<br>
>>   corresponding argument or instruction, or to the terminating<br>
>>   instruction of the basic block.  (This requires new support for<br>
>>   attaching metadata to function arguments, which I'll have to add for<br>
>>   debug info anyway.)<br>
><br>
> llvm::Argument attachments are hard<br>
> ===================================<br>
><br>
> I've been looking at prototyping metadata attachments to<br>
> `llvm::Argument`, which is key to replacing debug info intrinsics.<br>
><br>
> It's a fairly big piece of new IR, and comes with its own subtle<br>
> semantic decisions.  What do you do with metadata attached to arguments<br>
> when you inline a function?  If the arguments are remapped to other<br>
> instructions (or arguments), they may have metadata of the same kind<br>
> attached.  Does one win?  Which one?  Or are they merged?  What if the<br>
> arguments get remapped to constants?  What about when a function is<br>
> cloned?<br>
><br>
> While the rest of this metadata-is-not-a-value proposal is effectively<br>
> NFC, this `Argument` part could introduce problems.  If I rewrite debug<br>
> info intrinsics as argument attachments and then immediately split<br>
> `Metadata` from `Value`, any semantic subtleties will be difficult to<br>
> diagnose in the noise of the rest of the changes.<br>
><br>
> While I was looking at this as a shortcut to avoid porting<br>
> function-local metadata, I think it introduces more points of failure<br>
> than problems it solves.<br>
<br>
<br>
</div></div>One thing to consider is that there are cases were we describe function arguments without referencing the argument in the intrinsic at all. Currently, if you compile<br>
<br>
  $ cat struct.c<br>
  struct s { int a; int b; };<br>
  int foo(struct s s1) { return s1.a; }<br>
<br>
  $ clang -g -O1 -arch x86_64 -S -emit-llvm struct.c<br>
<br>
we cannot preserve the debug info for the argument so it is turned into an intrinsic describing an undef value.<br>
<br>
  ; Function Attrs: nounwind readnone ssp uwtable<br>
  define i32 @foo(i64 %s1.coerce) #0 {<br>
  entry:<br>
    %s1.sroa.0.0.extract.trunc = trunc i64 %s1.coerce to i32<br>
    tail call void @llvm.dbg.declare(metadata !18, metadata !14, metadata !19), !dbg !20<br>
    ret i32 %s1.sroa.0.0.extract.trunc, !dbg !21<br>
  }<br>
<br>
  !18 = metadata !{%struct.s* undef}<br></blockquote><div><br>I'm a little confused by this example - ideally in this case we wouldn't lose the variable, it's right there and we should reference it. But, yes, that'll take your SROA patch, etc.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Note that it is critical for this DIVariable to make it into the debug info even if it is undefined, or the function arguments won’t match the function signature.<br></blockquote><div><br>In theory, at -O0 we should never "not have" an argument to attach something to, right? (that'd require DAE to run) and above -O0 we store the variable list so we can't lose variables anyway.<br><br>Should we just aim for that? How far off are we before that's a practical goal (so we can avoid having to add in an intrinsic for this special case that we plan/hope/know will go away eventually). Alternatively, should we just turn on the variable list even at -O0, I know some other bugs that would 'fix' (fix in the sense of at least providing the right signature - but still be cases of missing variable location info).<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-- adrian<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
> Limited function-local metadata<br>
> -------------------------------<br>
><br>
> Instead, I propose porting a limited form of function-local metadata,<br>
> whose use is severely restricted but covers our current use cases (keep<br>
> reading for details).  We can defer replacing debug info intrinsics<br>
> until the infrastructure has settled down and is stable.<br>
><br>
> Assembly syntax<br>
> ===============<br>
><br>
> This is a good time to talk about assembly syntax, since it will<br>
> demonstrate what I'm thinking for function-local metadata.<br>
><br>
> Assembly syntax is important.  It's our view into the IR.  If metadata<br>
> is typeless (and not a `Value`), that should be reflected in the<br>
> assembly syntax.<br>
><br>
> Old syntax<br>
> ----------<br>
><br>
> There are four places metadata can be used/reference in the IR.<br>
><br>
> 1. Operands of `MDNode`.<br>
><br>
>        !0 = metadata !{metadata !"string", metadata !1, i32* @global)<br>
><br>
>    Notice that the `@global` argument is not metadata: it's an<br>
>    `llvm::Constant`.  In the new IR, these will be wrapped in a<br>
>    `ValueAsMetadata` instance.<br>
><br>
> 2. Operands of `NamedMDNode` (yes, they're different).<br>
><br>
>        !named = metadata !{metadata !0, metadata !1}<br>
><br>
>    These operands are always `MDNode`.<br>
><br>
> 3. Attachments to instructions.<br>
><br>
>        %inst = load i32* @global, !dbg !0<br>
><br>
>    Notice that we already skip the `metadata` type here.<br>
><br>
> 4. Arguments to intrinsics.<br>
><br>
>        call void @llvm.dbg(metadata !{i32 %inst}, metadata !0)<br>
><br>
>    The first argument is subtle -- that's a function-local `MDNode`<br>
>    with `%inst` as its only operand.<br>
><br>
>    In the new IR, the second operand will be a `MetadataAsValue`<br>
>    instance that contains a reference to the `MDNode` numbered `!0`.<br>
><br>
> New syntax<br>
> ----------<br>
><br>
> Types only make sense when an operand can be an `llvm::Value`.  Let's<br>
> remove them where they don't make sense.<br>
><br>
> I propose the following syntax for the above examples, using a new<br>
> keyword, `value`:<br>
><br>
> 1. Operands of `MDNode`.  Drop `metadata`, since metadata doesn't have<br>
>    types.  Use `value` to indicate a wrapped `llvm::Value`.<br>
><br>
>        !0 = !{!"string", !1, value i32* @global)<br>
><br>
> 2. Operands of `NamedMDNode`.  Drop `metadata`, since metadata doesn't<br>
>    have types.<br>
><br>
>        !named = !{!0, !1}<br>
><br>
> 3. Attachments to instructions.  No change!<br>
><br>
>        %inst = load i32* @global, !dbg !0<br>
><br>
> 4. Arguments to intrinsics.  Keep `metadata`, since here it's wrapped<br>
>    into an `llvm::Value` (which has a type).  Use `value` to indicate a<br>
>    metadata-wrapped value.<br>
><br>
>        call void @llvm.dbg(metadata value i32 %inst, metadata !0)<br>
><br>
>    Notice that the first argument doesn't use an `MDNode` anymore.<br>
><br>
> Restrictions on function-local metadata<br>
> =======================================<br>
><br>
> In the new IR, function-local metadata (say, `LocalValueAsMetadata`)<br>
> *cannot* be used as an operand to metadata -- the only legal place for<br>
> it is in a `MetadataAsValue` instance.  This prevents the additional<br>
> complexity from poisoning the rest of the metadata hierarchy.<br>
><br>
> Effectively, this restricts function-local metadata to direct operands<br>
> of intrinsics.<br>
><br>
<br>
</div></div></blockquote></div><br></div></div>