[PATCH] D120477: [LoongArch] Add basic support to Disassembler

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/Disassembler/LoongArchDisassembler.cpp:64
+
+  MCRegister Reg = LoongArch::R0 + RegNo;
+  Inst.addOperand(MCOperand::createReg(Reg));
----------------
inline used-once variable


================
Comment at: llvm/lib/Target/LoongArch/Disassembler/LoongArchDisassembler.cpp:73
+  assert(isUInt<N>(Imm) && "Invalid immediate");
+  Inst.addOperand(MCOperand::createImm(Imm + P));
+  return MCDisassembler::Success;
----------------
This seems strange. Is isUInt<N> check done before Imm+P?


================
Comment at: llvm/test/MC/LoongArch/Misc/aligned-nops.s:10
+       .p2align        3
+       .type   alpha, at function
+alpha:
----------------
You can generally omit `.type`, `.size` and associated labels.

Unuseful comments like `# -- End function` should be scrubbed as well.


================
Comment at: llvm/test/MC/LoongArch/Misc/unaligned-nops.s:1
+# RUN: not --crash llvm-mc --filetype=obj --triple=loongarch64 %s -o %t
+.byte 1
----------------
`report_fatal_error` easy to use but is discouraged. Proper diagnostic mechanisms like reportError are preferred.
Better not to add new `not --crash` tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120477