[Lldb-commits] [PATCH] D66934: [ARM64] Simplify RegisterInfos_arm64.h with macro based RegisterInfo array

Fri Aug 30 09:02:07 PDT 2019

clayborg added a comment.

Just a few nits around DEFINE_GPR64 and how the "alt" names are specified. See inline comments.

================
Comment at: source/Plugins/Process/Utility/RegisterInfos_arm64.h:492
+// Defines a 64-bit general purpose register
+#define DEFINE_GPR64(reg, alt, generic_kind)                                   \
+  {                                                                            \
----------------
Since most definitions don't have an alt, might be nice to make two:
```
DEFINE_GPR64(reg, generic_kind) ...
DEFINE_GPR64_ALT(reg, alt, generic_kind) ...
```
Also be nice to not assume "alt" is already a string and just add the #alt into the definition. See comment below for more details

================
Comment at: source/Plugins/Process/Utility/RegisterInfos_arm64.h:534
+    // DEFINE_GPR64(name, alternate name, GENERIC KIND)
+    DEFINE_GPR64(x0, nullptr, LLDB_REGNUM_GENERIC_ARG1),
+    DEFINE_GPR64(x1, nullptr, LLDB_REGNUM_GENERIC_ARG2),
----------------
If we make changes to remote "alt" from DEFINE_GPR64 this becomes:
```
DEFINE_GPR64(x0, LLDB_REGNUM_GENERIC_ARG1),
```

================
Comment at: source/Plugins/Process/Utility/RegisterInfos_arm64.h:563
+    DEFINE_GPR64(x28, nullptr, LLDB_INVALID_REGNUM),
+    DEFINE_GPR64(fp, "x29", LLDB_REGNUM_GENERIC_FP),
+    DEFINE_GPR64(lr, "x30", LLDB_REGNUM_GENERIC_RA),
----------------
If we make suggested changes to DEFINE_GPR64 by adding a DEFINE_GPR64_ALT this becomes:
```
DEFINE_GPR64_alt(fp, x29, LLDB_REGNUM_GENERIC_FP),
```
Note no quotes on the "x29"

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

https://reviews.llvm.org/D66934