<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<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>
</body>
</html>