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

[Lu Weining via Phabricator via llvm-commits]

SixWeining added inline comments.


================
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;
----------------
MaskRay wrote:
> This seems strange. Is isUInt<N> check done before Imm+P?
Yes, isUInt<N> check should be done before Imm+P. The `Imm` is the immediate value in instruction's binary encoding. It's range is `[0, 2^N-1]` while the range of the MCOperand is `[1, 2^1]`.


================
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
----------------
MaskRay wrote:
> `report_fatal_error` easy to use but is discouraged. Proper diagnostic mechanisms like reportError are preferred.
> Better not to add new `not --crash` tests.
OK. Thanks for the suggestion. But for this test, the `report_fatal_error` is called in common code `lib/MC/MCAssembler.cpp` but not in LoongArch backend.

`lib/MC/MCAssembler.cpp` calls target's `writeNopData` and report fatal error when it returns false.
LoongArch overrides the `writeNopData` interface and returns false when fail to write nop datas. 

```
//LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
 43 bool LoongArchAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
 44                                        const MCSubtargetInfo *STI) const {
 45   // Check for byte count not multiple of instruction word size
 46   if (Count % 4 != 0)
 47     return false;
 48 
 49   // The nop on LoongArch is andi r0, r0, 0.
 50   for (; Count >= 4; Count -= 4)
 51     support::endian::write<uint32_t>(OS, 0x03400000, support::little);
 52 
 53   return true;
 54 }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120477