labath added inline comments.
================ Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:91 + ArchSpec arch = process_sp->GetTarget().GetArchitecture(); + bool is64bit; + switch (arch.GetMachine()) { ---------------- I'd just do `is64bit = arch.GetAddressByteSize() == 8` ================ Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:124-190 + auto in_basenames = gpr_basenames.find(reg_name + 1); + if (in_basenames != gpr_basenames.end() && + new_index < gpr_base_reg_indices.size()) { + GPRReg ®_rec = gpr_base_reg_indices[new_index++]; + reg_rec.base_index = x.index(); + + // construct derived register names ---------------- What if we introduced helper functions like: ``` // returns 0 for [re]ax, 1 for [re]bx, ... Optional<unsigned> GetGPRNumber(llvm::StringRef name) { // either a lookup table or the fancy name matching } GPRReg MakeGPRReg(size_t base_index, unsigned gpr_number) { // I'd probably implement this via lookup table(s) } ``` .. or something similar? The general idea is to split this function into smaller chunks and also try to abstract away the funky register names as much as possible. ================ Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:211-213 + const char *reg_name = x.name.AsCString(); + auto found = subreg_name_set.find(reg_name); + if (found != subreg_name_set.end()) ---------------- `llvm::is_contained(haystack, needle)` 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