[PATCH] D63280: [llvm-objdump] Use <first-symbol>-<offset> as the section start symbol
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 17 04:58:51 PDT 2019
jhenderson added a comment.
I absolutely agree with @MaskRay that the GNU format is less than ideal, but the overwhelming feedback at last year's BoF on the topic was that we should match the output of GNU unless the output was wrong. If it were up to me, I'd just omit the reference entirely in these instances, since they don't really add any value!
Maybe this is another indication that we need separate GNU and LLVM styles for llvm-objdump, similar to llvm-readobj/llvm-readelf?
================
Comment at: llvm/test/MC/AMDGPU/branch-comment.s:13
s_branch loop_end_nosym
-// BIN: s_branch 0 // 000000000004: BF820000 <.text+0x8>
+// BIN: s_branch 0 // 000000000004: BF820000 <keep_symbol-0xC+0x8>
// BIN-NOT: loop_end_nosym:
----------------
This one is particularly concerning. Does this case match GNU? I'd expect it to fold the two sums together (i.e. `keep_symbol-0x4`)
================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-no-start-symbol.test:2
+## Test that if there is no symbol at the start of a section, we show a label
+## with the name <first-symbol-name>-0x<offset-of-first-symbol>
+
----------------
Nit: missing trailing full stop. It might be wise to quote the bit after "with the name".
================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1155
+ // If the section has no symbol at the start, create one with the name
+ // <first-symbol>-<offset> if later symbol exists; if not, use section name.
+ std::string StartSymbol;
----------------
if later -> if a later
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63280/new/
https://reviews.llvm.org/D63280
More information about the llvm-commits
mailing list