labath marked an inline comment as done.
labath added a comment.
This looks great. I just have a quick question about the GPR vs GPRegSet
thingy...
================
Comment at: lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp:167
assert(target_arch.GetTriple().getOS() == llvm::Triple::FreeBSD);
switch (target_arch.GetMachine()) {
case llvm::Triple::aarch64:
----------------
omjavaid wrote:
> labath wrote:
> > It would be good to merge these two switches. Then the reg(set)_interface
> > variables would be local to each case label and it would not be so weird
> > that we sometimes use one and sometimes the other.
> I have tried a couple of options but the no of different combinations
> involved I feel current implementation should stay untill we incrementally
> move remaining architectures to user RegisterInfosAndSet interface.
I like this solution.
================
Comment at:
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp:46
+ : lldb_private::RegisterContext(thread, 0) {
+ m_register_info_up = std::move(register_info);
----------------
move this to the initializer list
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:85
+ gpr_w22, gpr_w23, gpr_w24, gpr_w25, gpr_w26, gpr_w27, gpr_w28,
+ LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
----------------
I'd probably just delete this comment (or merge it with the leading comment
above the array definition), and then let clang-format lay this out normally...
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:19
public:
+ enum { ARM64GPR = 0, ARM64FPR };
+
----------------
Why these names? I think [GF]PRegSet would be better for two reasons:
- the same names with the same purpose already exist in
`NativeRegisterContextNetBSD_x86_64.h`
- it seems like a better way to differentiate from the [GF]PR structs below
than adding a redundant ARM64 prefix.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80105/new/
https://reviews.llvm.org/D80105
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
