[PATCH] D49003: [Debugify] Export per-pass debug info loss statistics

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

At a high-level is it fine to use CSV here? I've found it really convenient because Google Sheets, Numbers, python scripts etc. can ingest the data easily.

In https://reviews.llvm.org/D49003#1154445, @gbedwell wrote:

> This is really nice.  I agree it will be really useful, albeit with a minor concern about false positives in the stats themselves due to DCE transforms like what we ran into in https://llvm.org/PR37942 .


Taken individually these reports won't be very useful. It gives us a sense of which mid-level passes drop the most debug info, but the results aren't particularly surprising. I think the real signal will come from monitoring loss stats over time on fixed inputs. That should tell us which areas of the compiler to focus on, and which areas are improving/regressing.

> On some of the codebases I've tried, the warnings can be a bit noisy (and makes attempting to do any form of automatic testcase reduction based on their presence very difficult).  For example, in my testing with this patch I can see that Dead Store Elimination is generating some missing values and locations in a few testcases.  In every one I've looked at so far, these numbers relate to eliminated dead stores :).  Similarly, we don't know how many of the large instcombine number in the above PDF is down to DCE and how much is down to genuine DI loss. We've seen cases of both, but I'm not sure of the ratio.

Right, each issue needs to be looked at individually. Even in cases where a pass is doing straight DCE, it's sometimes possible to preserve debug values by rewriting instruction effects as DIExpressions. The exported stats won't precisely signal how many bugs there are to fix.

> Maybe a compelling argument for taking some approach along the lines of the suggested one of attaching undef dbg.values during DCE?

I'm not sure this is the right tradeoff between improved stats tracking and compile-time overhead.



================
Comment at: tools/opt/Debugify.cpp:402
+  if (EC) {
+    errs() << EC.message() << ", " << Path << '\n';
+    return;
----------------
gbedwell wrote:
> prefix this with something like "could not open file: "?
Right, will do.


https://reviews.llvm.org/D49003