[LLVMdev] [RFC] Separating Metadata from the Value hierarchy

[Duncan P. N. Exon Smith]

> 
>> > llvm::Argument attachments are hard
>> > ===================================
>> >
>> > I've been looking at prototyping metadata attachments to
>> > `llvm::Argument`, which is key to replacing debug info intrinsics.
>> >
>> > It's a fairly big piece of new IR, and comes with its own subtle
>> > semantic decisions.  What do you do with metadata attached to arguments
>> > when you inline a function?  If the arguments are remapped to other
>> > instructions (or arguments), they may have metadata of the same kind
>> > attached.  Does one win?  Which one?  Or are they merged?  What if the
>> > arguments get remapped to constants?  What about when a function is
>> > cloned?
>> >
>> > While the rest of this metadata-is-not-a-value proposal is effectively
>> > NFC, this `Argument` part could introduce problems.  If I rewrite debug
>> > info intrinsics as argument attachments and then immediately split
>> > `Metadata` from `Value`, any semantic subtleties will be difficult to
>> > diagnose in the noise of the rest of the changes.
>> >
>> > While I was looking at this as a shortcut to avoid porting
>> > function-local metadata, I think it introduces more points of failure
>> > than problems it solves.
>> 
>> 
>> 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
>> 
>>   $ cat struct.c
>>   struct s { int a; int b; };
>>   int foo(struct s s1) { return s1.a; }
>> 
>>   $ clang -g -O1 -arch x86_64 -S -emit-llvm struct.c
>> 
>> we cannot preserve the debug info for the argument so it is turned into an intrinsic describing an undef value.
>> 
>>   ; Function Attrs: nounwind readnone ssp uwtable
>>   define i32 @foo(i64 %s1.coerce) #0 {
>>   entry:
>>     %s1.sroa.0.0.extract.trunc = trunc i64 %s1.coerce to i32
>>     tail call void @llvm.dbg.declare(metadata !18, metadata !14, metadata !19), !dbg !20
>>     ret i32 %s1.sroa.0.0.extract.trunc, !dbg !21
>>   }
>> 
>>   !18 = metadata !{%struct.s* undef}
> 
> 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.
>  
> 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.
> 
>> 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.
>> 
>> 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

This (new) proposal doesn't add any intrinsics, it just supports the
current use cases.

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

I think discussion about the debug info schema (and intrinsics) and their
semantics is orthogonal to separating metadata from the `Value`
hierarchy.

At least, it's orthogonal now that I've found a design for function-local
metadata that restricts it to intrinsic arguments, so it doesn't add
complexity to the rest of the IR.

IMO, it was a mistake for me to conflate the two in the first place; I
was naive about the scope of changes required to replace the intrinsics.