<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>Hi Vedant,</p>
<p><br>
</p>
<p>I've managed to merge this idea into the existing Debugify Pass
by introducing two modes:</p>
<p> 1) synthetic - everything stay as is; this mode deals with the
synthetic debug info<br>
</p>
<p> 2) original - this is the new mode; it checks debug info
metadata preservation on real/original/-g debug info<br>
</p>
<p>The initial patches are: <a href="https://reviews.llvm.org/D82545">https://reviews.llvm.org/D82545</a>
and <a href="https://reviews.llvm.org/D82546">https://reviews.llvm.org/D82546</a>.</p>
<p>There are still TODOs (such as to cover the dbg.values tracking
by using the idea you pointed out in a previous mail; or to cover
metadata other then DILocations and DISubprograms (if there are
any that make sense)).<br>
</p>
<p><br>
</p>
<p>Best regards,</p>
<p>Djordje<br>
</p>
<div class="moz-cite-prefix">On 19.6.20. 09:33, Djordje wrote:<br>
</div>
<blockquote type="cite" cite="mid:07f2d09d-b276-2a04-14a3-1fdd01594942@syrmia.com">
<p><br>
</p>
<div class="moz-cite-prefix">On 19.6.20. 08:27, Vedant Kumar
wrote:<br>
</div>
<blockquote type="cite" cite="mid:C15F3E23-6092-4349-BE32-9F1ACCB0E3E8@apple.com"> <br class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On Jun 18, 2020, at 1:58 AM, Djordje <<a href="mailto:djordje.todorovic@syrmia.com" class="" moz-do-not-send="true">djordje.todorovic@syrmia.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<p class="">Hi Vedant,</p>
<p class="">Thanks a lot for your comments!</p>
<p class="">>It looks like a lot of the new
infrastructure introduced <a href="https://github.com/djolertrk/llvm-di-checker/commit/9d26ac2557c584f6cf82ac5535fc47f8bd267a27" class="" moz-do-not-send="true">here</a> consists of
logic copied from the debugify implementation. Why is
introducing a new pair of passes better than extending
the ones we have? The core infrastructure needed to
track location loss for real (non-synthetic) source
variables is is in place already.</p>
<p class="">Since it is a proposal, I thought it'd
easier to understand the idea if I duplicate things.
Ideally, we can make an API that could be used from
both tools. Initially, I made a few patches locally
turning the real debug info into debugify ones, but I
realized it breaks the original idea/design of the
debugify & that is why I decided to make a
separate pass(es). This cannot stay as is with the
respect to the implementation, it should be either
merged into debugify file(s) or refactored using the
API mentioned above.</p>
</div>
</div>
</blockquote>
<div>Oh, this wasn’t clear to me. In the future, please
describe what is/isn’t part of the proposal. It’d be
especially helpful to include some discussion of the pros
& cons of the prototype design and its alternatives.</div>
<br class="">
</div>
</blockquote>
Sorry If I wasn't clear enough. I thought if I write [PROPOSAL],
all of it is considered as a proposal.<br>
<blockquote type="cite" cite="mid:C15F3E23-6092-4349-BE32-9F1ACCB0E3E8@apple.com">
<div>
<blockquote type="cite" class="">
<div class="">
<div class="">
<p class=""> Another reason for implementing it as a
different pass was the fact the debugify is meant to
be used from 'opt' level only, but if we want to
invoke the option from front end level, we need to
merge it into the IR library.</p>
</div>
</div>
</blockquote>
<div>The debugify passes are now linked by llc via the
Transforms library as part of the -mir-debugify
implementation. A frontend should be able to use them.</div>
</div>
</blockquote>
I'll try that, thanks.<br>
<blockquote type="cite" cite="mid:C15F3E23-6092-4349-BE32-9F1ACCB0E3E8@apple.com">
<div><br class="">
<blockquote type="cite" class="">
<div class="">
<div class="">
<p class=""> </p>
<div class="">>Stepping back a bit, I’m also
surprised by the decision to move away from synthetic
testing when there’s still so much low-hanging fruit
to pick using that technique. The example from <a href="https://reviews.llvm.org/D81939" class="" moz-do-not-send="true">https://reviews.llvm.org/D81939</a> illustrates
this perfectly: in this case it’s not necessary to
invent a new testing technique to uncover the bug,
because simply running `./bin/llvm-lit -Dopt="opt
-debugify-each" test/Transforms/DeadArgElim` finds the
same issue.</div>
<p class="">As I mentioned in the previous mail, I do
really think the debugify technique is great & I
use it. But, in order to detect that variable "x" was
optimized-out starting from pass Y, I only run the
di-checker option (that performs analysis only) &
find the variable in the final html report. I think
that is very user friendly concept.</p>
</div>
</div>
</blockquote>
<div>About the analysis — why not emit a report in
-check-debugify when the # of non-undef debug uses of a
variable drops to 0? This doesn’t require the -debugify
step. The list of local variables is preserved via
DISubprogram::retainedNodes.</div>
</div>
</blockquote>
<p>Hmm, good idea. But we'd need to workaround the fact/condition
the debugify(& check-debugify) works with/expects the
synthetic debug info only. Let me try merging these ideas into
the code, by removing the duplicated code (I'll try to use the
debugify/check-debugify as much as possible by performing
analysis only).<br>
</p>
<blockquote type="cite" cite="mid:C15F3E23-6092-4349-BE32-9F1ACCB0E3E8@apple.com">
<div>
<div><br class="">
</div>
<blockquote type="cite" class="">
<div class="">
<div class="">
<p class=""> At the end, when we detected what was the
spot of loosing the location, we can run debugify on
the pass-directory-tests (but there is a concern the
tests does not cover all the possible cases; and the
case found from the high level could be new to the
pass). In addition, the di-checker detects issues for
metadata other than locations (currently, the
preservation map keeps the disubprograms only, but it
should keep other kinds too).</p>
</div>
</div>
</blockquote>
<div>Note that it’s possible to to increase code coverage by
running a -debugify-each pipeline on -g0 IR produced by a
frontend.</div>
<div><br class="">
</div>
<div>Is it common for a pass to drop an entire DISubprogram? I
would hope this either never happens or is extremely rare.</div>
</div>
</blockquote>
<p>It is extremely rare, but there are passes that create new
functions, and it possible to forget to update/attach the
subprogram on that (the similar situation we face with locations
when someone forgets to set debugloc on an instruction).</p>
<p><br>
</p>
<p>Best,</p>
<p>Djordje<br>
</p>
<blockquote type="cite" cite="mid:C15F3E23-6092-4349-BE32-9F1ACCB0E3E8@apple.com">
<div>
<div><br class="">
</div>
<div>best,</div>
<div>vedant</div>
<div><br class="">
</div>
<blockquote type="cite" class="">
<div class="">
<div class="">
<p class=""> </p>
<p class="">>In D81939, you discuss finding the new
tool useful when responding to bug reports about
optimized-out variables or missing locations. We
sorely do need something better than
-opt-bisect-limit, but why not start with something
simple? -check-debugify already knows how to report
when & where a location is dropped, it would be
simple to teach it to emit a report when a variable is
fully optimized-out.</p>
<p class="">I agree. We can do that and that could be
used from both utilities.<br class="">
</p>
<p class=""><br class="">
</p>
<p class="">Best regards,</p>
<p class="">Djordje</p>
<p class=""><br class="">
</p>
<div class="moz-cite-prefix">On 17.6.20. 21:14, Vedant
Kumar wrote:<br class="">
</div>
<blockquote type="cite" cite="mid:74E217F2-67F5-44FF-BAA1-470DEFB75549@apple.com" class=""> Hey Djordje,
<div class=""><br class="">
</div>
<div class="">It looks like a lot of the new
infrastructure introduced <a href="https://github.com/djolertrk/llvm-di-checker/commit/9d26ac2557c584f6cf82ac5535fc47f8bd267a27" class="" moz-do-not-send="true">here</a> consists
of logic copied from the debugify implementation.
Why is introducing a new pair of passes better than
extending the ones we have? The core infrastructure
needed to track location loss for real
(non-synthetic) source variables is is in place
already.</div>
<div class=""><br class="">
</div>
<div class="">Stepping back a bit, I’m also surprised
by the decision to move away from synthetic testing
when there’s still so much low-hanging fruit to pick
using that technique. The example from <a href="https://reviews.llvm.org/D81939" class="" moz-do-not-send="true">https://reviews.llvm.org/D81939</a> illustrates
this perfectly: in this case it’s not necessary to
invent a new testing technique to uncover the bug,
because simply running `./bin/llvm-lit -Dopt="opt
-debugify-each" test/Transforms/DeadArgElim` finds
the same issue.</div>
<div class=""><br class="">
</div>
<div class="">In D81939, you discuss finding the new
tool useful when responding to bug reports about
optimized-out variables or missing locations. We
sorely do need something better than
-opt-bisect-limit, but why not start with something
simple? -check-debugify already knows how to report
when & where a location is dropped, it would be
simple to teach it to emit a report when a variable
is fully optimized-out.</div>
<div class=""><br class="">
</div>
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Jun 17, 2020, at 2:10 AM,
Djordje <<a href="mailto:djordje.todorovic@syrmia.com" class="" moz-do-not-send="true">djordje.todorovic@syrmia.com</a>>
wrote:</div>
<div class="">
<div class=""><br class="">
I am sharing the proposal [0] which gives a
brief introduction for the implementation of
the LLVM DI Checker utility. On a very high
level, it is a pair of LLVM (IR) Passes that
check the preservation of the original debug
info in the optimizations. There are options
controlling the passes, that could be
invoked from ``clang`` as well as from
``opt`` level.<br class="">
<br class="">
By testing the utility on the GDB 7.11
project (using it as a testbed), it has
found a certain number of potential issues
regarding the DILocations (using it on LLVM
project build itself, it has found one bug
regarding DISubprogram metadata). Please
take a look into the final report (on the
GDB 7.11 testbed) generated from the script
that collects the data at [1]. By looking at
these data, it looks that the utility like
this could be useful when trying to detect
the real issues related to debug info
production by the compiler.<br class="">
</div>
</div>
</blockquote>
<div class=""><br class="">
</div>
<div class="">Thanks for sharing these results.
The data here is older (from the 2018 debug info
BoF) and from a different project (sqlite3), but
we saw some similar patterns: <a href="https://llvm.org/devmtg/2018-10/slides/Prantl-Kumar-debug-info-bof-2018.pdf" class="" moz-do-not-send="true">https://llvm.org/devmtg/2018-10/slides/Prantl-Kumar-debug-info-bof-2018.pdf</a></div>
<div class=""><br class="">
</div>
<div class="">best</div>
<div class="">vedant</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
</blockquote>
</body>
</html>