[llvm-dwarfdump] Fix split-dwarf bug in stats for inlined var loc cov

[djtodoro]

Summary

Initial (D96045) patch didn't handle split dwarf cases, so this fixes that bug.

In addition, before applying this patch, we had a slowdown that happened after the D96045. With this patch, the slowdown will be fixed.

[aprantl]

One comment inline.

llvm/tools/llvm-dwarfdump/Statistics.cpp

727

This comment is mostly repeats what's already in the name of the variable. It might more interesting to explain why these are being reset for each CU.

[dblaikie]

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

[djtodoro]

• Reduce test
• Improve the comment

[djtodoro]

In D100951#2705718, @dblaikie wrote:

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

I am not sure how to keep this as assembly code, since we'd need to create object files of it, and then to link it together...

[dblaikie]

In D100951#2707838, @djtodoro wrote:

In D100951#2705718, @dblaikie wrote:

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

I am not sure how to keep this as assembly code, since we'd need to create object files of it, and then to link it together...

the dwo files should be able to be assembly without any particular restrictions/complications in authoring them, right? (taking the assembly of a split DWARF compilation and pulling out the .dwo parts should be adequate, I think)
The executable - it could probably actually be an object file that happens to have two skeleton units in it (like we generate under LTO, for instance - though without any of the LTO-specific cross-CU references) so that could be written in assembly relatively easily (to hand-craft it you could probably take the assembly of the two skeletons and copy/paste them into one file for instance)

[djtodoro]

In D100951#2708809, @dblaikie wrote:

In D100951#2707838, @djtodoro wrote:

In D100951#2705718, @dblaikie wrote:

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

I am not sure how to keep this as assembly code, since we'd need to create object files of it, and then to link it together...

the dwo files should be able to be assembly without any particular restrictions/complications in authoring them, right? (taking the assembly of a split DWARF compilation and pulling out the .dwo parts should be adequate, I think)
The executable - it could probably actually be an object file that happens to have two skeleton units in it (like we generate under LTO, for instance - though without any of the LTO-specific cross-CU references) so that could be written in assembly relatively easily (to hand-craft it you could probably take the assembly of the two skeletons and copy/paste them into one file for instance)

I've accomplished to write these as asm files. Thanks!

[djtodoro]

• make the tests as assembly files

[dblaikie]

llvm/test/tools/llvm-dwarfdump/X86/inlined_variables_with_zero_cov.test

31

Output files should go in %t or a subdirectory of %t, so things like:

RUN: rm -r %t
RUN: mkdir %t
RUN: ... -o %t/test1.dwo
...

[dblaikie]

Other than the test file tweak, looks good to me!

llvm/test/tools/llvm-dwarfdump/X86/inlined_variables_with_zero_cov.test

+## This checks the number of inlined variables with 0% location
+## coverage in split dwarf cases.
+##  $ cat test1.cpp
+##  __attribute__((optnone)) static void x() {
+##  }
+##  __attribute__((always_inline)) static void y() {
+##    int var;
+##    x();
+##  }
+##  void f1() {
+##    y();
+##  }
+##
+##  $ cat test2.cpp
+##  __attribute__((optnone)) static void x() {
+##  }
+##  __attribute__((always_inline)) static void y() {
+##    int var;
+##    x();
+##  }
+##  void f1();
+##  int main() {
+##    f1();
+##    y();
+##  }
+##  $ clang++ -O1 -g -gsplit-dwarf test2.cpp test1.cpp -o split-dwarf-test
+##
+RUN: llvm-dwarfdump --statistics %S/Inputs/split-dwarf-test | FileCheck %s
+CHECK: "#variables processed by location statistics": 2
+CHECK: "#variables with 0% of parent scope covered by DW_AT_location": 2
+CHECK: "#local vars processed by location statistics": 2
+CHECK: "#local vars with 0% of parent scope covered by DW_AT_location": 2

llvm/tools/llvm-dwarfdump/Statistics.cpp

 /// useful, only the delta between compiling the same program with different
 /// compilers is.
 bool dwarfdump::collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
                                           const Twine &Filename,
                                           raw_ostream &OS) {
   StringRef FormatName = Obj.getFileFormatName();
   GlobalStats GlobalStats;
   LocationStats LocStats;
-  InlinedVarsTyMap GlobalInlinedFnInfo;
-  InlinedFnInstacesTy InlinedFnsToBeProcessed;
   StringMap<PerFunctionStats> Statistics;
   for (const auto &CU : static_cast<DWARFContext *>(&DICtx)->compile_units()) {
     if (DWARFDie CUDie = CU->getNonSkeletonUnitDIE(false)) {
+      // These variables are being reset for each CU, since there could be
+      // a situation where we have two subprogram DIEs with the same offsets
+      // in two diferent CUs, and we can end up using wrong variables info
+      // when trying to resolve abstract_orign attribute.
+      // TODO: Handle LTO cases where the abstract origin of
+      // the function is in a different CU than the one it's
+      // referenced from or inlined into.
+      InlinedVarsTyMap GlobalInlinedFnInfo;
+      InlinedFnInstacesTy InlinedFnsToBeProcessed;
+
       collectStatsRecursive(CUDie, "/", "g", 0, 0, Statistics, GlobalStats,
                             LocStats, GlobalInlinedFnInfo,
                             InlinedFnsToBeProcessed);
 
       collectZeroCovInlinedVars(CUDie.getDwarfUnit(), GlobalStats, LocStats,
                                 GlobalInlinedFnInfo, InlinedFnsToBeProcessed);
     }
   }