SixWeining added a comment. In D136578#3878744 <https://reviews.llvm.org/D136578#3878744>, @DavidSpickett wrote:
> Always good to see another architecture in lldb. > > A few points up front. > > Changes like this are fine and we can review them, but can only be landed > once we see that they'll be built on. So please stack changes so that we can > see what is upcoming. > https://llvm.org/docs/Phabricator.html#creating-a-patch-series > > Do you have a plan to test this configuration? I don't see any public > buildbots for Loongson (please correct me if I am wrong) but that is not a > requirement as long as you have some testing plan (RISCV doesn't have a > public bot, as one example). > > Do you have community members who can review these changes for architectural > correctness? Speaking just for myself I am happy to review bits of lldb > machinery but when it comes to architecture details I am not, and will not, > be an expert in Loongson. > > I'm ok accepting patches without a second reviewer, but I advise for your own > benefit to try to find someone who already knows the details of Loongson. Hi @DavidSpickett, thanks for your advice. I (as the LoongArch backend code owner) can review these changes for architectural correctness. And I think other LoongArch community members (non Loongson employees) also can take it. Especially @xen0n and @xry111 who are quite familar with LoongArch. What's more, AFAIK, @seehearfeel (yangtie...@loongson.cn) is the LoongArch port maintainer of gdb <https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/MAINTAINERS;h=52db8cb8f70f0f67d8bced5ac02f5fde88690bba;hb=HEAD#l274>. Regarding the public buildbot, I'm trying to set it up and maybe we can see it one or two weeks. ================ Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h:24 + +class NativeRegisterContextLinux_loongarch64 : public NativeRegisterContextLinux { +public: ---------------- Pls limit the columns count to 80 which can be done by `clang-format`. See: https://llvm.org/docs/Contributing.html#how-to-submit-a-patch ================ Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h:9 + +#if defined(__loongarch__) && __loongarch_grlen == 64 + ---------------- Seems `#if __loongarch_grlen == 64` is enough? But I'm fine with both. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136578/new/ https://reviews.llvm.org/D136578 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
