Minor fixups to LLDB AArch64 register infos macros for SVE register infos

[omjavaid]

Summary

This patch adds some cosmetic changes to LLDB AArch64 register infos macros in order to use them in SVE register infos struct in follow up patches.
This patch initially added invalidate lists to register infos struct but that is no longer needed and problem disappeared after updating qemu testing environment.

old headline comments for reference:
AArch64 reigster X and V registers are primary GPR and vector registers respectively. If these registers are modified their corresponding children w regs or s/d regs should be invalidated. Specially when a register write fails it is important that failure gets reflected to all the registers which draw their value from a particular value register.

[labath]

This sounds like it could use a test case.

lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

593–594

What's up with the indirection?

[omjavaid]

In D77045#1954690, @labath wrote:

This sounds like it could use a test case.

Adding a testcase would be tricky as these register overlap in memory and we store them with overlapping offsets with their children we should not need to invalidate the children when we write the parent but for some strange unexplainable reason QEMU was behaving strangely and not updating the first half in certain random cases. I just thought invalidation of children will force a read after write for that case.

lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

593–594

We want to pass nullptr or register name as is then do stringize on regname only. According to C macro expansion rules if we want our macros expanded first and then do the # then we ll need double indirection.

[labath]

In D77045#1956879, @omjavaid wrote:

Adding a testcase would be tricky as these register overlap in memory and we store them with overlapping offsets with their children we should not need to invalidate the children when we write the parent but for some strange unexplainable reason QEMU was behaving strangely and not updating the first half in certain random cases. I just thought invalidation of children will force a read after write for that case.

Thanks for the explanation, but I'm afraid I still don't get what is going on here. Can you walk me through the individual steps here? Something like:

1. user does "register write x0 xxxxxx"
2. lldb translates that to the appropriate p packet
3. ???
4. user does "register read w0"
5. bad value comes out because...

lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

593–594

I've figured it's something like that, though it's still not clear to me why that is needed in this particular case.

However, now I have a different question. As the only thing you're using the alt_name argument for is to turn it into a string, why not just pass a string in the first place:

DEFINE_GPR64(x0, nullptr, ...)
...
DEFINE_GPR64(fp, "x29", ...)
...

597

This comment is out of date now.

[omjavaid]

In D77045#1963896, @labath wrote:

In D77045#1956879, @omjavaid wrote:

Adding a testcase would be tricky as these register overlap in memory and we store them with overlapping offsets with their children we should not need to invalidate the children when we write the parent but for some strange unexplainable reason QEMU was behaving strangely and not updating the first half in certain random cases. I just thought invalidation of children will force a read after write for that case.

Thanks for the explanation, but I'm afraid I still don't get what is going on here. Can you walk me through the individual steps here? Something like:

1. user does "register write x0 xxxxxx"
2. lldb translates that to the appropriate p packet
3. ???
4. user does "register read w0"
5. bad value comes out because...

Let me debug it separately from SVE and I will get back to you with an update.

[omjavaid]

In D77045#1966540, @omjavaid wrote:

In D77045#1963896, @labath wrote:

In D77045#1956879, @omjavaid wrote:

Adding a testcase would be tricky as these register overlap in memory and we store them with overlapping offsets with their children we should not need to invalidate the children when we write the parent but for some strange unexplainable reason QEMU was behaving strangely and not updating the first half in certain random cases. I just thought invalidation of children will force a read after write for that case.

Thanks for the explanation, but I'm afraid I still don't get what is going on here. Can you walk me through the individual steps here? Something like:

1. user does "register write x0 xxxxxx"
2. lldb translates that to the appropriate p packet
3. ???
4. user does "register read w0"
5. bad value comes out because...

Let me debug it separately from SVE and I will get back to you with an update.

I have tried to figure this out and eventually problem disappeared after updating test environment. I could not really figure out the exact issue.

lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

 static uint32_t g_d25_invalidates[] = {fpu_v25, fpu_s25, LLDB_INVALID_REGNUM};
 static uint32_t g_d26_invalidates[] = {fpu_v26, fpu_s26, LLDB_INVALID_REGNUM};
 static uint32_t g_d27_invalidates[] = {fpu_v27, fpu_s27, LLDB_INVALID_REGNUM};
 static uint32_t g_d28_invalidates[] = {fpu_v28, fpu_s28, LLDB_INVALID_REGNUM};
 static uint32_t g_d29_invalidates[] = {fpu_v29, fpu_s29, LLDB_INVALID_REGNUM};
 static uint32_t g_d30_invalidates[] = {fpu_v30, fpu_s30, LLDB_INVALID_REGNUM};
 static uint32_t g_d31_invalidates[] = {fpu_v31, fpu_s31, LLDB_INVALID_REGNUM};
 
-// Generates register kinds array for 64-bit general purpose registers
-#define GPR64_KIND(reg, generic_kind)                                          \
+// Generates register kinds array with DWARF, EH frame and generic kind
+#define MISC_KIND(reg, type, generic_kind)                                     \
   {                                                                            \
     arm64_ehframe::reg, arm64_dwarf::reg, generic_kind, LLDB_INVALID_REGNUM,   \
-        gpr_##reg                                                              \
+        type##_##reg                                                           \
   }
 
-// Generates register kinds array for registers with lldb kind
-#define MISC_KIND(lldb_kind)                                                   \
+// Generates register kinds array for registers with only lldb kind
+#define LLDB_KIND(lldb_kind)                                                   \
   {                                                                            \
     LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,             \
         LLDB_INVALID_REGNUM, lldb_kind                                         \
   }
 
 // Generates register kinds array for vector registers
-#define VREG_KIND(reg)                                                         \
-  {                                                                            \
-    LLDB_INVALID_REGNUM, arm64_dwarf::reg, LLDB_INVALID_REGNUM,                \
-        LLDB_INVALID_REGNUM, fpu_##reg                                         \
-  }
-
-// Generates register kinds array for cpsr
-#define CPSR_KIND(lldb_kind)                                                   \
-  {                                                                            \
-    arm64_ehframe::cpsr, arm64_dwarf::cpsr, LLDB_REGNUM_GENERIC_FLAGS,         \
-        LLDB_INVALID_REGNUM, lldb_kind                                         \
-  }
-
-#define MISC_GPR_KIND(lldb_kind) CPSR_KIND(lldb_kind)
-#define MISC_FPU_KIND(lldb_kind) MISC_KIND(lldb_kind)
-#define MISC_EXC_KIND(lldb_kind) MISC_KIND(lldb_kind)
+#define GPR64_KIND(reg, generic_kind) MISC_KIND(reg, gpr, generic_kind)
+#define VREG_KIND(reg) MISC_KIND(reg, fpu, LLDB_INVALID_REGNUM)
+#define MISC_GPR_KIND(lldb_kind) MISC_KIND(cpsr, gpr, LLDB_REGNUM_GENERIC_FLAGS)
+#define MISC_FPU_KIND(lldb_kind) LLDB_KIND(lldb_kind)
+#define MISC_EXC_KIND(lldb_kind) LLDB_KIND(lldb_kind)
 
 // Defines a 64-bit general purpose register
 #define DEFINE_GPR64(reg, generic_kind)                                        \
   {                                                                            \
     #reg, nullptr, 8, GPR_OFFSET(gpr_##reg), lldb::eEncodingUint,              \
         lldb::eFormatHex, GPR64_KIND(reg, generic_kind), nullptr, nullptr,     \
         nullptr, 0                                                             \
   }
 
 // Defines a 64-bit general purpose register
 #define DEFINE_GPR64_ALT(reg, alt, generic_kind)                               \
   {                                                                            \
     #reg, #alt, 8, GPR_OFFSET(gpr_##reg), lldb::eEncodingUint,                 \
         lldb::eFormatHex, GPR64_KIND(reg, generic_kind), nullptr, nullptr,     \
         nullptr, 0                                                             \
   }
 
 // Defines a 32-bit general purpose pseudo register
 #define DEFINE_GPR32(wreg, xreg)                                               \
   {                                                                            \
     #wreg, nullptr, 4,                                                         \
         GPR_OFFSET(gpr_##xreg) + GPR_W_PSEUDO_REG_ENDIAN_OFFSET,               \
-        lldb::eEncodingUint, lldb::eFormatHex, MISC_KIND(gpr_##wreg),          \
+        lldb::eEncodingUint, lldb::eFormatHex, LLDB_KIND(gpr_##wreg),          \
         g_contained_##xreg, g_##wreg##_invalidates, nullptr, 0                 \
   }
 
 // Defines a vector register with 16-byte size
 #define DEFINE_VREG(reg)                                                       \
   {                                                                            \
     #reg, nullptr, 16, FPU_OFFSET(fpu_##reg - fpu_v0), lldb::eEncodingVector,  \
         lldb::eFormatVectorOfUInt8, VREG_KIND(reg), nullptr, nullptr, nullptr, \
         0                                                                      \
   }
 
 // Defines S and D pseudo registers mapping over corresponding vector register
 #define DEFINE_FPU_PSEUDO(reg, size, offset, vreg)                             \
   {                                                                            \
     #reg, nullptr, size, FPU_OFFSET(fpu_##vreg - fpu_v0) + offset,             \
-        lldb::eEncodingIEEE754, lldb::eFormatFloat, MISC_KIND(fpu_##reg),      \
+        lldb::eEncodingIEEE754, lldb::eFormatFloat, LLDB_KIND(fpu_##reg),      \
         g_contained_##vreg, g_##reg##_invalidates, nullptr, 0                  \
   }
 
 // Defines miscellaneous status and control registers like cpsr, fpsr etc
 #define DEFINE_MISC_REGS(reg, size, TYPE, lldb_kind)                           \
   {                                                                            \
     #reg, nullptr, size, TYPE##_OFFSET_NAME(reg), lldb::eEncodingUint,         \
         lldb::eFormatHex, MISC_##TYPE##_KIND(lldb_kind), nullptr, nullptr,     \
static lldb_private::RegisterInfo g_register_infos_arm64_le[] = {
     DEFINE_GPR32(w15, x15),
     DEFINE_GPR32(w16, x16),
     DEFINE_GPR32(w17, x17),
     DEFINE_GPR32(w18, x18),
     DEFINE_GPR32(w19, x19),
     DEFINE_GPR32(w20, x20),
     DEFINE_GPR32(w21, x21),
     DEFINE_GPR32(w22, x22),
     DEFINE_GPR32(w23, x23),
     DEFINE_GPR32(w24, x24),
     DEFINE_GPR32(w25, x25),
     DEFINE_GPR32(w26, x26),
     DEFINE_GPR32(w27, x27),
     DEFINE_GPR32(w28, x28),
 
     // DEFINE_VREG(name)
     DEFINE_VREG(v0),
     DEFINE_VREG(v1),
     DEFINE_VREG(v2),
     DEFINE_VREG(v3),
     DEFINE_VREG(v4),