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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 4 01:17:48 PDT 2020


jhenderson added a comment.

Hi @tinti - please remember to upload with full diff context. I previously pointed you at the page on how to do so. Not including full context makes things harder to review.



================
Comment at: llvm/test/tools/llvm-objdump/X86/source-interleave-absolute-paths.test:7
+; RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
+; RUN: llvm-objdump --source %t.o 2>&1 | FileCheck %s
+
----------------
In both tests, it probably makes sense to only have a `--prefix` case - there is existing testing for `--source` so we don't need to show that behaviour works. I also think you need a case where you have `--prefix` and it allows you to find the path. Perhaps you could use just `/Inputs` in the sed line, and then use `--prefix` to add `%/p` to the path.


================
Comment at: llvm/test/tools/llvm-objdump/X86/source-interleave-absolute-paths.test:9
+
+; CHECK: 0000000000000000 <foo>:
+; CHECK-NEXT: ; int foo() {
----------------
When using a mixture of CHECK and CHECK-NEXT lines, it's a little more readable to me if you indent the CHECK lines so that their value start lines up with those for CHECK-NEXT:
```
; CHECK:      0000000000000000 <foo>:
; CHECK-NEXT: ; int foo() {
```


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

https://reviews.llvm.org/D85024



More information about the llvm-commits mailing list