seehearfeel added inline comments.
================ Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp:27 +// NT_PRSTATUS and NT_FPREGSET definition +#include <elf.h> + ---------------- SixWeining wrote: > [[ https://llvm.org/docs/CodingStandards.html#include-style | Should be > sorted lexicographically by the full path ]]. So put it before `sys/uio.h`. OK, will modify it, thank you. ================ Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp:221 + + uint8_t *src = const_cast<uint8_t *>(data_sp->GetBytes()); + if (src == nullptr) { ---------------- DavidSpickett wrote: > Possibly not needed const cast. If no const cast, build failed: ``` /home/loongson/llvm.git/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp:221:12: error: cannot initialize a variable of type 'uint8_t *' (aka 'unsigned char *') with an rvalue of type 'const uint8_t *' (aka 'const unsigned char *') uint8_t *src = data_sp->GetBytes(); ^ ~~~~~~~~~~~~~~~~~~~ 1 error generated. ``` ================ Comment at: lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:60 + virtual bool WriteGPR() = 0; + virtual bool WriteFPR() = 0; +}; ---------------- DavidSpickett wrote: > This doesn't look right, I'd expect `bool ... () override;` here. Maybe we should leave it as is, the other archs do the same thing, they will be override in the following file which is not implemented now: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_loongarch64.h/cpp, otherwise build failed: ``` In file included from /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.cpp:19: /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:57:18: error: only virtual member functions can be marked 'override' bool ReadGPR() override; ^~~~~~~~ /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:58:18: error: only virtual member functions can be marked 'override' bool ReadFPR() override; ^~~~~~~~ /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:59:19: error: only virtual member functions can be marked 'override' bool WriteGPR() override; ^~~~~~~~ /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:60:19: error: only virtual member functions can be marked 'override' bool WriteFPR() override; ^~~~~~~~ 4 errors generated. ``` ================ Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.cpp:138 +size_t RegisterInfoPOSIX_loongarch64::GetRegisterSetCount() const { + return k_num_register_sets - 1; +} ---------------- DavidSpickett wrote: > SixWeining wrote: > > Why `- 1`? > I had the same thought. From the few others I looked at, it seems that it's > count not the last index. So if you've got N sets it should return N. You are right, will modify it, thank you. ================ Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h:35 + { \ + loongarch_dwarf::dwarf_##reg, loongarch_dwarf::dwarf_##reg, generic_kind, \ + LLDB_INVALID_REGNUM, reg##_loongarch \ ---------------- SixWeining wrote: > unnecessary indent? Yes, will modify it, thank you. ================ Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h:56 + +#define DEFINE_FPR64(reg, generic_kind) \ + { \ ---------------- SixWeining wrote: > Not allow accessing FPR registers through ABI names? Will modify it, thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138407/new/ https://reviews.llvm.org/D138407 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
