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

Lu Weining via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 1 01:25:39 PST 2022


SixWeining added inline comments.


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


================
Comment at: llvm/test/MC/LoongArch/Directives/data-valid.s:1
+# Check that data directives supported by gas are also supported by LLVM MC.
+
----------------
MaskRay wrote:
> 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.
Thanks. I will follow this implicit standard.


================
Comment at: llvm/test/MC/LoongArch/Directives/data-valid.s:13
+# CHECK: .dword 1234605616436508552
+.byte 0x10
+.byte 0x20
----------------
MaskRay wrote:
> 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.
OK. Thanks.


================
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 \
----------------
MaskRay wrote:
> The prevailing style is to use double-dash options for FileCheck. `--check-prefix` is more common when there is just one prefix.
Yes. And also for `--triple=`. Thanks.


================
Comment at: llvm/test/MC/LoongArch/ISA/Basic/Integer/invalid.s:171-172
+
+# Instruction not in the base ISA
+# TODO: Test instructions in LSX/LASX/LBT/LVZ after defined.
+
----------------
xen0n wrote:
> ```
> # Instructions outside the base ISA
> # TODO: Test instructions in LSX/LASX/LBT/LVZ after their introduction.
> ```
OK. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120476



More information about the llvm-commits mailing list