[PATCH] D77789: [llvm-dwarfdump][Stats] Clean up

Wed Apr 29 06:24:11 PDT 2020

djtodoro marked 4 inline comments as done.
djtodoro added inline comments.

================
Comment at: llvm/docs/CommandGuide/llvm-dwarfdump.rst:116
+            as machine-readable single-line JSON output. The output
+            format is described in the section below.

----------------
jhenderson wrote:
> You might want to add a link to the section below, so that users browsing on the web can click and jump to it.
Sure, I've added it.

================
Comment at: llvm/docs/CommandGuide/llvm-dwarfdump.rst:169
+
+As mentioned above, :option:`--statistics` generates single-line JSON
+representing the Debug Info quality metrics.
----------------
aprantl wrote:
> The :option:`--statistics` option generates single-line JSON output
> representing quality metrics of the processed debug info. These metrics are useful
> to compare changes between two compilers, particularly for judging the effect
> that a change to the compiler has on the debug info quality.
> 
> The output is formatted as key-value pairs. The first pair contains a version number.
> The following naming scheme is used for the keys:
> 
> ...
Sorry I haven't had much time to take another look into the write-up, but this looks very good! Thanks a lot for the suggestion!

================
Comment at: llvm/docs/CommandGuide/llvm-dwarfdump.rst:175
+      - `variables` ==> all local variables and parameters
+      - `all local vars` ==> all local variables
+      - `all params` ==> all formal parameters
----------------
aprantl wrote:
> why `all` only in the second case?
Fixed.

================
Comment at: llvm/docs/CommandGuide/llvm-dwarfdump.rst:182
+      - `#bytes` ==> the number of bytes
+      - `#variables \\ entry values ...` ==> the number of variables excluding
+        the entry values etc.
----------------
aprantl wrote:
> `-` instead of `\\`?
Much better!

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

https://reviews.llvm.org/D77789