[PATCH] D116676: [1/4] [llvm-objdump][NFC] Add RISC-V objdump test case

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

Thank for such a rapid review @maskray! I've left a couple of queries in response, but otherwise I'll refresh the test case once I've had feedback on other patches in the series (to minimise repeated rebase effort).



================
Comment at: llvm/test/tools/llvm-objdump/ELF/RISCV/branches.s:3
+# RUN:     | llvm-objdump -d -M no-aliases --no-show-raw-insn - \
+# RUN:     | FileCheck --match-full-lines %s
+
----------------
MaskRay wrote:
> Remove `--match-full-lines`
> 
> In most test/tools/, the prevailing continuation-line format is to place `|` at the line end.
> ```
> # RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c %s |
> # RUN:     llvm-objdump -d -M no-aliases --no-show-raw-insn - |
> ```
The reasoning behind --match-full-lines was that it will catch changes to printing of branch targets (as in D116678), ensuring the test case is updated.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/RISCV/branches.s:8
+# CHECK: 0: beq t0, t1, 0x58 <foo+0x58>
+beq t0, t1, .Llocal
+# CHECK: 4: bne t0, t1, 0x58 <foo+0x58>
----------------
MaskRay wrote:
> I have a slight preference that instructions are indented by two: to be clearer that they belong to the function (nearest label).
Do you still feel it's worth doing, when taking into account the changes to this test in D116678?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116676