<html><body><p><tt><font size="2">David Blaikie <dblaikie@gmail.com> wrote on 27.02.2018 18:47:22:<br></font></tt><br><tt><font size="2">This didn't show up in Phabricator ... was this deliberate?</font></tt><br><tt><font size="2"><br>> On Tue, Feb 27, 2018 at 12:48 AM Ulrich Weigand via Phabricator <<br>> reviews@reviews.llvm.org> wrote:</font></tt><br><tt><font size="2">>> Well, there's variants of that scenario: <a href="https://reviews.llvm.org/L1">https://reviews.llvm.org/L1</a><br>>> and L2 might already have the same scope, or they might have different scopes.</font></tt><br><tt><font size="2">> <br>> The same scope but inlined in different places?</font></tt><br><br><tt><font size="2">I guess this can happen if one function is inlined twice at two</font></tt><br><tt><font size="2">different places into the same caller ...</font></tt><br><br><tt><font size="2">>> If <a href="https://reviews.llvm.org/L1">https://reviews.llvm.org/L1</a> and L2 already have the same scope, <br>>> this **must** remain the scope of the merged location, or else we <br>>> break debug intrinsics again.</font></tt><br><tt><font size="2">> <br>> That seems strange - can you explain that constraint & what <br>> motivates it in some detail?<br></font></tt><br><tt><font size="2">Well, this is only reason why I'm working on this in the first place;</font></tt><br><tt><font size="2">to fix this bug </font></tt><a href="https://bugs.llvm.org/show_bug.cgi?id=36410"><tt><font size="2">https://bugs.llvm.org/show_bug.cgi?id=36410</font></tt></a><br><br><tt><font size="2">If the instruction for which locations are being merged is a debug</font></tt><br><tt><font size="2">intrinsics, we have no choice of scopes -- the scope of a debug</font></tt><br><tt><font size="2">intrinsic must match the scope of the variable the intrinsic </font></tt><br><tt><font size="2">refers to, otherwise the module verifier will abort.</font></tt><br><br><tt><font size="2">If you read the bugzilla, we had been discussing back and forth</font></tt><br><tt><font size="2">whether to ensure we get the correct scope in this case by either:</font></tt><br><tt><font size="2">- special-casing debug intrinsics in the caller; or</font></tt><br><tt><font size="2">- ensuring that the getMergedLocation algorithm has the property</font></tt><br><tt><font size="2"> that if the two inputs have the same scope, then the result</font></tt><br><tt><font size="2"> will as well -- then it will automatically respect the special</font></tt><br><tt><font size="2"> requirement for debug intrinsics</font></tt><br><br><tt><font size="2">This patch is an attempt to implement the second option. If we</font></tt><br><tt><font size="2">now decide that we *don't* want that property to hold, then we</font></tt><br><tt><font size="2">need to go back and fix the original bug in another way.</font></tt><br><tt><font size="2"> </font></tt><br><tt><font size="2">Bye,<br>Ulrich</font></tt><br><BR>
</body></html>