[PATCH] D120476: [LoongArch] Add basic support to AsmParser

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/MC/LoongArch/Directives/cfi-invalid.s:8
+
+# CHECK: error: invalid register number
+.cfi_offset -22, -8 # err
----------------
Should test line/column
`:[[#@LINE+1]]:...: error: `


================
Comment at: llvm/test/MC/LoongArch/Directives/cfi-valid.s:49
+# CHECK: .cfi_offset 20, 160
+.cfi_offset 20, 160
+# CHECK: .cfi_offset 21, 168
----------------
Unsure it is useful enumerating all values. Having the minimum, the maximum, and a random middle value is sufficient.


================
Comment at: llvm/test/MC/LoongArch/Directives/data-valid.s:1
+# Check that data directives supported by gas are also supported by LLVM MC.
+
----------------
It's some folks's rule and used heavily in binary utilities: prefer `##` for non-RUN-non-CHECK comments so that they stand out (many editors highlight them), and in case you use some generator to generate the tests, the generator can be taught to retain `##` comments while scrubbing other comments.


================
Comment at: llvm/test/MC/LoongArch/Directives/data-valid.s:3
+
+# RUN: llvm-mc -triple loongarch32 < %s | FileCheck %s
+# RUN: llvm-mc -triple loongarch64 < %s | FileCheck %s
----------------
I find it sometimes more readable by combing non-error and error cases in one file. See `--defsym=ERR=1` in test/MC.
I usually place error cases after non-error cases.


================
Comment at: llvm/test/MC/LoongArch/Directives/data-valid.s:13
+# CHECK: .dword 1234605616436508552
+.byte 0x10
+.byte 0x20
----------------
I'd write:

```
# CHECK:      .byte 1
# CHECK-NEXT: .byte 255
.byte 1
.byte 0xff

# CHECK:      .half 1
# CHECK-NEXT: .half 65535
.half 0x1
.half 0xffff

...
```

It's important to use CHECK-NEXT more than CHECK to detect that the end-of-line is correctly handled.


================
Comment at: llvm/test/MC/LoongArch/Directives/data-valid.s:37
+# CHECK: .dword 1234605616436508552
+.byte 0x10
+.byte 0x20
----------------
Many values just duplicate and unneeded.


================
Comment at: llvm/test/MC/LoongArch/ISA/Basic/Integer/arith.s:4
+# RUN: llvm-mc %s -triple=loongarch32 -show-encoding \
+# RUN:     | FileCheck -check-prefixes=CHECK32-ASM %s
+# RUN: llvm-mc %s -triple=loongarch64 -show-encoding --defsym=LA64=1 \
----------------
The prevailing style is to use double-dash options for FileCheck. `--check-prefix` is more common when there is just one prefix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120476