labath accepted this revision. labath added a comment. This revision is now accepted and ready to land.
I think we've managed to come to something that looks mostly reasonable. ================ Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:51 + std::vector<DynamicRegisterInfo::Register> ®s, + llvm::ArrayRef<uint32_t> base_reg_indices, RegisterMap reg_names, + uint32_t base_size, RegKind name_index, lldb::Encoding encoding, ---------------- const RegisterMap& -- it's expensive to copy ================ Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:59 + DynamicRegisterInfo::Register &full_reg = regs[base_index]; + const llvm::StringRef &subreg_name = + reg_names[full_reg.name.GetStringRef()][static_cast<int>(name_index)]; ---------------- drop const & -- it's cheap to copy :) ================ Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:92-94 + bool is64bit = + process_sp->GetTarget().GetArchitecture().GetAddressByteSize() == 8; + uint32_t gpr_base_size = is64bit ? 8 : 4; ---------------- how about you reverse this, so you set gpr_base_size first, and then use that to initialize is64bit? ================ Comment at: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py:343 + # test pseudo-registers + self.match("register read cx", ---------------- Maybe also do a `register read --all` and check that all (sub)registers are present and have the expected values (you can try using `self.filecheck` for that). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108831/new/ https://reviews.llvm.org/D108831 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits