<div dir="ltr">> 

Do you mean documenting the desired frontend behavior, or adding some verifier in<br>LLVM? A warning for the latter is that SROA may currently emit IR that contains a<br>mix of declares and values for different fragments of an aggregate variable, so I<br>assume that is something that would need to be fixed then.<div><br></div><div>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>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>Snippet from PR</div><div>[.]</div><div><pre class="gmail-bz_comment_text" style="white-space:pre-wrap;width:50em;color:rgb(0,0,0)"><font face="arial, sans-serif">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 class="gmail-bz_comment_text" style="white-space:pre-wrap;width:50em"><pre class="gmail-bz_comment_text" style="color:rgb(0,0,0);white-space:pre-wrap;width:50em"><pre class="gmail-bz_comment_text" style="white-space:pre-wrap;width:50em"><font face="arial, sans-serif">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">[.]</font></pre><pre class="gmail-bz_comment_text" style="color:rgb(0,0,0);white-space:pre-wrap;width:50em"><font face="arial, sans-serif">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 class="gmail-bz_comment_text" style="color:rgb(0,0,0);white-space:pre-wrap;width:50em"><span style="font-family:Arial,Helvetica,sans-serif;color:rgb(34,34,34)">> 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></pre><pre class="gmail-bz_comment_text" style="white-space:pre-wrap;width:50em"><font face="Arial, Helvetica, sans-serif">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 class="gmail-bz_comment_text" style="white-space:pre-wrap;width:50em"><font face="Arial, Helvetica, sans-serif">--Sourabh</font></pre></pre></div></div><br><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">aprantl@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> On Mar 30, 2020, at 11:57 PM, Sourabh Singh Tomar <<a href="mailto:sourav0311@gmail.com" target="_blank">sourav0311@gmail.com</a>> wrote:<br>
> <br>
>> > My understanding is that this isn't correct: dbg.declare specifies the<br>
>> memory address of a variable for the whole lifetime of the function,<br>
>> whereas dbg.value (and dbg.addr) specify the value/address until the<br>
>> next debug intrinsic. Mixing these two kinds of intrinsics creates<br>
>> ambiguity over where the variable location is at different positions<br>
>> in the code.<br>
> <br>
>           > Correct, you should not be mixing dbg.declare and other instrinsics for the same variable<br>
> <br>
> 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>
> Should we move ahead invalidate this behavior as in "declare and value intrinsic can't be specified for same local variable". ?<br>
> <br>
> 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>
<br>
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>
<br>
-- adrian</blockquote></div>