[PATCH] D70720: [llvm-objdump] Display locations of variables alongside disassembly
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Feb 23 17:10:17 PST 2020
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
More information about the llvm-commits
mailing list