[Lldb-commits] [PATCH] D74244: [lldb] Delete register info definitions in the x86_64 ABI classes

[Pavel Labath via Phabricator via lldb-commits]

labath marked 6 inline comments as done.
labath added inline comments.


================
Comment at: lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp:938
+      .Case("R8", LLDB_REGNUM_GENERIC_ARG5)
+      .Case("r9", LLDB_REGNUM_GENERIC_ARG6)
+      .Default(LLDB_INVALID_REGNUM);
----------------
tatyana-krasnukha wrote:
> Typo? It should be uppercase, I think.
Thanks for catching that. After some consideration, I've decided it would be less confusing if this function (and GetMCName)  worked on lowercase register names like everything else lldb. In the new version of this patch I've moved the uppercase transformation closer to the point where we interface with the MC layer.


================
Comment at: lldb/source/Target/ABI.cpp:249
+  int eh = -1;
+  int dwarf = -1;
+  for (unsigned reg = 0; reg < m_mc_register_info_up->getNumRegs(); ++reg) {
----------------
jasonmolenda wrote:
> Would it be simpler to assign these to `LLDB_INVALID_REGNUM` at the beginning, instead of -1 and setting it later?
Not really, because the MC interface uses -1 to denote registers which don't have a dwarf number. (In fact some of the things in the MC list are not even really registers, but more like combinations of registers that are sometimes handy when grouped (`Q0_Q1`, etc.)


================
Comment at: lldb/source/Target/ABI.cpp:267
+  if (name_ref.consume_front(from_prefix) && to_integer(name_ref, _, 10))
+    name = (to_prefix + name_ref).str();
+}
----------------
jasonmolenda wrote:
> will `to_integer` bit work with an all-letter reg like `cpsr`?
We didn't need this functionality yet because we weren't renaming any singular registers, but I've now added code to handle this too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74244/new/

https://reviews.llvm.org/D74244