[PATCH] D79513: [dsymutil] Print statistics about debug_info reduction in verbose mode.

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/docs/CommandGuide/dsymutil.rst:102
+ (in bytes) to the linked dSYM. The table is sorted by the output size listing
+ the obj ect files with the largest contribution first.
+
----------------
typo: object

Does this count .debug_info only, or also the other sections?

How does type uniquing factor into this? Are we counting the input or the actual contribution?


================
Comment at: llvm/include/llvm/DWARFLinker/DWARFLinker.h:561
     /// chose to keep above. If there are no valid relocs, then there's
     /// nothing to clone/emit.
+    uint64_t cloneAllCompileUnits(DWARFContext &DwarfContext,
----------------
Can you please document the return value?


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:42
+
+/// Compute the total size of the debug info.
+static uint64_t getDebugInfoSize(DWARFContext &Dwarf) {
----------------
of the debug_info *section*?


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:2543
+    auto ComputePercentange = [](int64_t Input, int64_t Output) -> float {
+      const float Difference = Output - Input;
+      const float Sum = Input + Output;
----------------
using a float to store the result of an operation on 64-bit integers sounds risky to me. Should we at least use a double?


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:2545
+      const float Sum = Input + Output;
+      if (Sum == 0)
+        return 0;
----------------
I'd feel more comfortable if we replaced this with 
`if (Input == 0 && Output == 0)` or with `if (Sum < epsilon)` or something that doesn't rely on floating point landing on exact zero


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79513/new/

https://reviews.llvm.org/D79513