[PATCH] D72973: using symbol index+symbol name + storage mapping class as label for llvm-objdump -D
Jason Liu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 29 10:53:48 PST 2020
jasonliu added inline comments.
================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1128
+static void printIndexNameSmcAsLable(SectionSymbolsTy &Symbols,
+ std::string SymbolName, unsigned int SI) {
----------------
hubert.reinterpretcast wrote:
> jasonliu wrote:
> > 1. Label mis-spelled in the function name. And we are not necessarily printing the "label", so the name is misleading. I would suggest to name it as printFullyQualifiedSymbolName. Or, printXCOFFSymbolName.
> > 2. This function looks very XCOFF centric, do we want to put it in XCOFFDump.cpp?
> > 3. We only want one symbol inside of SectionSymbolsTy, so it does not make much sense to pass in a pointer to all of the symbols inside of the section, and a index to just get one symbol. I would try to typedef std::tuple<uint64_t, StringRef, uint8_t, int64_t> somewhere in the header and use that type here. And that should be the only one parameter you need. We don't need to pass in the SymbolName as well.
> >
> `printXCOFFDecoratedSymbolDescription` or `printXCOFFSymbolDescription`?
I'm good with either.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72973/new/
https://reviews.llvm.org/D72973
More information about the llvm-commits
mailing list