[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

[Pavel Labath via Phabricator via lldb-commits]

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 &reg_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