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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
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
+
----------------
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 - |
```


================
Comment at: llvm/test/tools/llvm-objdump/ELF/RISCV/branches.s:5
+
+# CHECK: 00000000 <foo>:
+foo:
----------------
The address formatting is checked by other tests so this test doesn't need to check it.

By removing addresses, when the number of instruction changes, the update (ongoing maintenance) will touch fewer lines.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/RISCV/branches.s:7
+foo:
+# CHECK: 0: beq t0, t1, 0x58 <foo+0x58>
+beq t0, t1, .Llocal
----------------
For similar reasons, remove leading addresses.



================
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>
----------------
I have a slight preference that instructions are indented by two: to be clearer that they belong to the function (nearest label).


================
Comment at: llvm/test/tools/llvm-objdump/ELF/RISCV/branches.s:77
+
+# CHECK: 00000060 <bar>:
+bar:
----------------



================
Comment at: llvm/test/tools/llvm-objdump/ELF/RISCV/branches.s:79
+bar:
+# CHECK: 60: c.nop
+nop
----------------
`60: c.nop` should probably keep its address to be clear that its a jump target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116676