[PATCH] D85024: [llvm-objdump] Implement --prefix option

[Vinicius Tinti via Phabricator via llvm-commits]

tinti added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-archive-with-modified-source-path.ll:1
+; This test checks if 'llvm-objdump' works as GNU's 'objdump' with '--prefix'
+; option.
----------------
MaskRay wrote:
> Please don't just copy `disassemble-archive-with-source.ll`
> 
> The test is named `disassemble-archive-with-modified-source-path.ll`
> I wonder whether using an `archive` is significant to the test. If not, just use a plain `.o`
> 
> If an archive can trigger a slightly different code path you also want to test, you can add a second llvm-objdump RUN line for an archive.
> 
> Consider reusing an existing .ll file and not duplicating LLVM IR, e.g. source-interleave-x86_64.test
> Please don't just copy `disassemble-archive-with-source.ll`
> 
> The test is named `disassemble-archive-with-modified-source-path.ll`
> I wonder whether using an `archive` is significant to the test. If not, just use a plain `.o`

Understood. I believe the both `.o` and `.c` are needed.
 
> If an archive can trigger a slightly different code path you also want to test, you can add a second llvm-objdump RUN line for an archive.
> 
> Consider reusing an existing .ll file and not duplicating LLVM IR, e.g. source-interleave-x86_64.test

I will. The `source-interleave-x86_64.test` has `source-interleave-x86_64.c` too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85024