lh03061238 added inline comments.
================
Comment at:
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp:42
"bnez rj, offs21"},
+ {0xfc000000, 0x48000000, &EmulateInstructionLoongArch::EmulateBCXXZ,
+ "bceqz/bcnez cj, offs21"},
----------------
SixWeining wrote:
> It should be 0xfc000300. BCEQZ and BCNEZ should be seperated.
> See
> https://github.com/loongson/LoongArch-Documentation/blob/main/docs/LoongArch-Vol1-EN/table-of-instruction-encoding.adoc
My understanding of this opcode is not accurate, This will be modified.
Thanks for pointing out.
================
Comment at:
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp:303
+// bceqz cj, offs21
+// if CFR[cj] == 0:
----------------
SixWeining wrote:
> A single space is enough.
will be modified， thanks
================
Comment at:
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp:312
+ uint32_t bcxxz = (inst >> 8) & 0b11;
+ uint32_t cj = Bits32(inst, 7, 5) + fpr_first_loongarch + 32;
+ uint64_t pc = ReadPC(&success);
----------------
SixWeining wrote:
> Is this number of FPRs? Would it be changed in future when we support vertor
> registers? Adding some comment may help future readers.
It's a little complicated here, I will use “Bits32(inst, 7, 5) +
fpr_fcc0_loongarch” to make it clearer.
will be modified， thanks
================
Comment at: lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp:157
+static uint32_t BCEQZ(uint8_t cj, int32_t offs21) {
+ return EncodeBCZcondType(0b010010, cj, 0b00, uint32_t(offs21));
+}
----------------
SixWeining wrote:
> The opcode is 8 bits `0b01001000`. See:
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/LoongArch/LoongArchFloat32InstrInfo.td#L109
>
> I think you can remove the third argument `bcxxz`.
This will be modified.
Thanks for pointing out.
================
Comment at: lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp:256
+ tester->fpr.fcc = cj_val;
+ // bc<cmp>z fcc0, (256)
+ uint32_t inst = encoder(0, 256);
----------------
SixWeining wrote:
> Ditto.
will be modified， thanks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140759/new/
https://reviews.llvm.org/D140759
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
