[PATCH] D70720: [llvm-objdump] Display locations of variables alongside disassembly

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:562
+
+/// Stores a single expression representing the location of a source-level
+/// variable, along with the PC range for which that expression is valid.
----------------
`// `

Internal classes should not use Doxygen comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:658
+  unsigned getIndentLevel() const {
+    return DbgIndent + (NoShowRawInsn ? 16 : 40);
+  }
----------------
Ideally `unsigned TabStop = NoShowRawInsn ? 16 : 40` in `printInst` should be extracted as a function.

Is `--no-leading-addr` taken into account?


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:705
+
+  void AddCompileUnit(DWARFDie D) {
+    if (D.getTag() == dwarf::DW_TAG_subprogram)
----------------
`addCompileUnit`

Looks like this function can be merged with `addFunction()`.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:713
+
+  /// Update to match the state of the instruction between ThisAddr and
+  /// NextAddr. In the common case, any live range active at ThisAddr is
----------------
echristo wrote:
> Not a huge fan of boolean arguments. Is there a different factoring here that'll help?
No need to use `/// `. These classes are internal.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:839
+          } else
+            OS << "  ";
+        }
----------------
Change `OS << "  "` to print one space unconditionally can simplify code above.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:864
+          else
+            OS << "  ";
+        }
----------------
Change `OS << "  "` to `OS << ' '` unconditionally can avoid `" "` above.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1762
                              !DisassembleAll;
+      formatted_raw_ostream FOS(outs());
       while (Index < End) {
----------------
Does this harm performance when --debug-vars is not used?

I once caused a regression (fixed by D64969).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70720