<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Apr 6, 2020, at 2:52 AM, Sourabh Singh Tomar <<a href="mailto:sourav0311@gmail.com" class="">sourav0311@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div dir="ltr" class="">This post is in continuation of checking the validity of IR(debug-info) emitted by different FE's.<div class=""><br class=""></div><div class="">flang FE at -O0(No Opt) level sometime emits -- DILocalVariable without name as in a sense --</div><div class=""><br class=""></div><div class="">[..]</div><div class="">!15 = !DILocalVariable(scope: !5, file: !3, type: !8, flags: DIFlagArtificial) --- Valid IR, since name(field) is optional while parsing and validation.<br class=""></div><div class=""><br class=""></div><div class="">Which eventually turned out in dwarf as --</div><div class="">0x0000006d: DW_TAG_variable ---------- Is this information useful for debugger ? Variable doesn't have any name.<br class=""> DW_AT_location (DW_OP_fbreg -12)<br class=""> DW_AT_type (0x00000093 "integer")<br class=""> DW_AT_artificial (0x01)<br class=""></div><div class="">[..]</div></div></div></div></blockquote><div><br class=""></div><div>Since the variable is also marked as artificial, I am *guessing* that it might be a helper variable, for example, one the is used to specify dynamic array bounds in an array type. Clang does something similar for variable length arrays — though Clang gives these variables some generic name.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">While looking for answer did some digging into clang/llvm , initially seems like it doesn't ---</div><div class="">Snippet from DebugInfoMetadata.cpp</div><div class="">[..]</div><div class="">DILocalVariable *DILocalVariable::getImpl(LLVMContext &Context, Metadata *Scope,</div><div class="">..</div><div class="">assert(isCanonical(Name) && "Expected canonical MDString");<br class=""></div><div class="">[..]</div><div class=""><br class=""></div><div class="">Is this functionality desired/needed ? To have verifier validate anonymous DILocalVariable.</div><div class=""><br class=""></div><div class="">Is this anonymous DILocalVariable and subsequent anonymous DW_TAG_variable dwarf has some other use, that I'm missing out at this point.</div><div class=""><br class=""></div><div class="">Can anybody provide some clarity WRT this. <br class=""></div></div></div></div></blockquote><div><br class=""></div><div>If this is indeed a helper variable referenced from a type, I think this is behavior should be allowed.</div><div><br class=""></div><div>-- adrian</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">Thank You!</div><div class="">Sourabh.</div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 1, 2020 at 10:19 PM Adrian Prantl <<a href="mailto:aprantl@apple.com" class="">aprantl@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;" class=""><br class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Apr 1, 2020, at 2:56 AM, Sourabh Singh Tomar <<a href="mailto:sourav0311@gmail.com" target="_blank" class="">sourav0311@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class="">>
Do you mean documenting the desired frontend behavior, or adding some verifier in<br class="">LLVM? A warning for the latter is that SROA may currently emit IR that contains a<br class="">mix of declares and values for different fragments of an aggregate variable, so I<br class="">assume that is something that would need to be fixed then.<div class=""><br class=""></div><div class="">I had a quick look on that PR(thanks for sharing). A verifier(if implemented) at LLVM level would invalidate this too. Would that be good ?</div><div class="">That brings up one other question, After SROA(like in present case) there can be mix of *dbg.decalre* and *dbg.value* of the same variable left out.</div><div class="">Snippet from PR</div><div class="">[.]</div><div class=""><pre style="white-space:pre-wrap;width:50em" class=""><font face="arial, sans-serif" class="">call void @llvm.dbg.declare(metadata i64* %arr.sroa.0, metadata !15, metadata DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !23</font></pre><pre style="white-space:pre-wrap;width:50em" class=""><pre style="white-space:pre-wrap;width:50em" class=""><pre style="white-space:pre-wrap;width:50em" class=""><font face="arial, sans-serif" class="">store i64 0, i64* %arr.sroa.0, align 16, !dbg !23
call void @llvm.dbg.value(metadata i64 0, metadata !15, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !23</font></pre><font face="arial, sans-serif" class="">[.]</font></pre><pre style="white-space:pre-wrap;width:50em" class=""><font face="arial, sans-serif" class="">Does this presence of *dbg.value* suggest that previous location/value described by *dbg.declare* is invalidated from this point. ? Not I suppose since Docs mentions *it's valid for entire lifetime*. Then why mix the at first place ?</font></pre></pre></div></div></div></blockquote><div class="">The documentation describes our intention and reality often lags behind. I think that we should fix the code to convert the dbg.declare into a dbg.value (or dbg.addr?) that points into the alloca as well. I'm guessing that it will not be easy to do this (we can call Local.cpp's LowerDbgDeclare functionality beforehand, but that may actually make SROA's job harder because it needs to find and update more values that may be difficult to associate in reverse...), but having the verifier should help in the process.</div><div class=""><br class=""></div><div class="">-- adrian</div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><pre style="white-space:pre-wrap;width:50em" class=""><pre style="white-space:pre-wrap;width:50em" class=""><span style="font-family:Arial,Helvetica,sans-serif;color:rgb(34,34,34)" class="">> Adding this to the Verifier sounds like a good idea to me. It may be possible that this uncovers existing bugs in the current flow, but that would be a good thing.</span><br class=""></pre><pre style="white-space:pre-wrap;width:50em" class=""><font face="Arial, Helvetica, sans-serif" class="">As David mentioned, **documenting the desired behavior for frontend** sounds good to me. this is based on premises that *clang* does not emit any *dbg.value* at -O0(No opt) level, which is good(since most assignments and variables are stack allocated, hence no need for *dbg.value*).
There are some FE (for instance flang), living out-of-tree they should also follow these semantics correctly.</font></pre><pre style="white-space:pre-wrap;width:50em" class=""><font face="Arial, Helvetica, sans-serif" class="">--Sourabh</font></pre></pre></div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 31, 2020 at 8:33 PM Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br class="">
<br class="">
> On Mar 30, 2020, at 11:57 PM, Sourabh Singh Tomar <<a href="mailto:sourav0311@gmail.com" target="_blank" class="">sourav0311@gmail.com</a>> wrote:<br class="">
> <br class="">
>> > My understanding is that this isn't correct: dbg.declare specifies the<br class="">
>> memory address of a variable for the whole lifetime of the function,<br class="">
>> whereas dbg.value (and dbg.addr) specify the value/address until the<br class="">
>> next debug intrinsic. Mixing these two kinds of intrinsics creates<br class="">
>> ambiguity over where the variable location is at different positions<br class="">
>> in the code.<br class="">
> <br class="">
> > Correct, you should not be mixing dbg.declare and other instrinsics for the same variable<br class="">
> <br class="">
> How about patching up llvm for the same, currently the IR showed above is valid and can be processed by llvm for target code generation.<br class="">
> Should we move ahead invalidate this behavior as in "declare and value intrinsic can't be specified for same local variable". ?<br class="">
> <br class="">
> So that no FE should generate this sort of thing in first place. clang doesn't do that so this change should not affect clang.<br class="">
<br class="">
Adding this to the Verifier sounds like a good idea to me. It may be possible that this uncovers existing bugs in the current flow, but that would be a good thing.<br class="">
<br class="">
-- adrian</blockquote></div>
</div></blockquote></div><br class=""></div></blockquote></div></div>
</div></blockquote></div><br class=""></body></html>