[PATCH] D72973: using symbol index+symbol name + storage mapping class as label for llvm-objdump -D
Digger via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 2 12:55:30 PST 2020
DiggerLin marked 8 inline comments as done.
DiggerLin added inline comments.
================
Comment at: llvm/test/tools/llvm-objdump/xcoff-disassemble-all.test:5
+# RUN: llvm-objdump -D %p/Inputs/xcoff-section-headers.o | \
+# RUN: FileCheck --check-prefix=CHECK-GNU %s
+
----------------
DiggerLin wrote:
> jhenderson wrote:
> > Indent continuation lines by a couple of spaces for readability.
> >
> > ```
> > # RUN: llvm-objdump -D %p/Inputs/xcoff-section-headers.o | \
> > # RUN: FileCheck --check-prefix=CHECK-GNU %s
> > ```
> >
> > Also, I think `CHECK` should be the default without `--symbol-description` and `CHECK-DESC` or something with the option, or better yet, add it to a new test file. Also, why are you checking `--symbol-description` with `--disassemble-all/-D` instead of plain disassembly (i.e. `--disassemble`).
> >
> > Finally, you probably need a test case + maybe a warning for using `--symbol-description` with something that isn't XCOFF.
> I do not saw different of "-D" "--disassemble-all", and "--disassemble" in the llvm-objdump --help
>
> and
> the original test case has
> RUN: llvm-objdump -D %p/Inputs/xcoff-section-headers.o
>
> I just added a new option --symbol-description in the this test case and test the new option. I think keep the original option of test case llvm-objdump -D is a better choice.
>
> I am prefer to put the test without --symbol-description in current test case.
> we can know what is the different output of llvm-objdump -D with and without
> --symbol-description at a glance.
>
>
I just read the source code, there are different of "--disassemble-all", and "--disassemble".
"--disassemble" only disassembly text section. "--disassemble-all" disassembly all sections
and in the test case , it also disassembly .data section, I think we need to use "--disassemble-all" or '-D" for .data section.
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