[PATCH] D72973: [llvm-objdump] Use symbol index+symbol name + storage mapping class as label for -D

Fri Mar 27 12:36:12 PDT 2020

jasonliu added inline comments.

================
Comment at: llvm/test/tools/llvm-objdump/xcoff-disassemble-all.test:5
+# RUN: llvm-objdump -D --symbol-description %p/Inputs/xcoff-section-headers.o | \
+# RUN:   FileCheck --check-prefixes=COMMON,DESC %s

----------------
hubert.reinterpretcast wrote:
> jasonliu wrote:
> > hubert.reinterpretcast wrote:
> > > @jasonliu, what is the plan for the merge conflict between this and D75131? In particular, would the relocation printing be tested with both the plain and the descriptive output?
> > I think ultimately we might want to have variations for
> > -D
> > -D --symbol-description
> > -D -r
> > -D -r --symbol-description
> > 
> > But the testing for `-D -r --symbol-description` is a bit tricky, as I think if we enable that, relocation printing's symbol should be the new format too, which is not implemented. So I'm Okay for that to be left out, or we just put it in and change later. 
> What is the difference if effort between needing to rebase this patch anyway and adding the descriptive output for the relocation printing within this patch?
We need to call printXCOFFSymbolDescription in the new getXCOFFRelocationValueString function. It shouldn't be a big effort.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72973