[lldb] [ABI/X86] Support combining xmm* and ymm*h regs into ymm*

gdbserver does not expose combined ymm* registers but rather XSAVE-style
split xmm* and ymm*h portions.  Extend value_regs to support combining
multiple registers and use it to create user-friendly ymm* registers
that are combined from split xmm* and ymm*h portions.

Differential Revision: https://reviews.llvm.org/D108937

Author: mgorny

lldb/include/lldb/lldb-private-types.h

@@ -51,8 +51,10 @@ struct RegisterInfo {
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not null, all registers in this list will be read first, at which point
   /// the value for this register will be valid. For example, the value list
-  /// for ah would be eax (x86) or rax (x64).
-  uint32_t *value_regs; //
+  /// for ah would be eax (x86) or rax (x64). Register numbers are
+  /// of eRegisterKindLLDB. If multiple registers are listed, the final
+  /// value will be the concatenation of them.
+  uint32_t *value_regs;
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not null, all registers in this list will be invalidated when the value of
   /// this register changes. For example, the invalidate list for eax would be

lldb/source/Plugins/ABI/X86/ABIX86.cpp

@@ -33,36 +33,40 @@ void ABIX86::Terminate() {
   ABIWindows_x86_64::Terminate();
 }
 
-enum class RegKind {
-  GPR32 = 0,
+namespace {
+enum RegKind {
+  GPR32,
   GPR16,
   GPR8h,
   GPR8,
+  MM,
+  YMM_YMMh,
+  YMM_XMM,
 
-  MM = 0,
+  RegKindCount
+};
+};
+
+struct RegData {
+  RegKind subreg_kind;
+  llvm::StringRef subreg_name;
+  llvm::Optional<uint32_t> base_index;
 };
 
-typedef llvm::SmallDenseMap<llvm::StringRef,
-                            llvm::SmallVector<llvm::StringRef, 4>, 16>
-    RegisterMap;
-
-static void addPartialRegisters(
-    std::vector<DynamicRegisterInfo::Register> &regs,
-    llvm::ArrayRef<uint32_t> base_reg_indices, const RegisterMap &reg_names,
-    uint32_t base_size, RegKind name_index, lldb::Encoding encoding,
-    lldb::Format format, uint32_t subreg_size, uint32_t subreg_offset = 0) {
-  for (uint32_t base_index : base_reg_indices) {
-    if (base_index == LLDB_INVALID_REGNUM)
-      break;
-    assert(base_index < regs.size());
+static void
+addPartialRegisters(std::vector<DynamicRegisterInfo::Register> &regs,
+                    llvm::ArrayRef<RegData *> subregs, uint32_t base_size,
+                    lldb::Encoding encoding, lldb::Format format,
+                    uint32_t subreg_size, uint32_t subreg_offset = 0) {
+  for (const RegData *subreg : subregs) {
+    assert(subreg);
+    uint32_t base_index = subreg->base_index.getValue();
     DynamicRegisterInfo::Register &full_reg = regs[base_index];
-    llvm::StringRef subreg_name = reg_names.lookup(
-        full_reg.name.GetStringRef())[static_cast<int>(name_index)];
-    if (subreg_name.empty() || full_reg.byte_size != base_size)
+    if (full_reg.byte_size != base_size)
       continue;
 
-    lldb_private::DynamicRegisterInfo::Register subreg{
-        lldb_private::ConstString(subreg_name),
+    lldb_private::DynamicRegisterInfo::Register new_reg{
+        lldb_private::ConstString(subreg->subreg_name),
         lldb_private::ConstString(),
         lldb_private::ConstString("supplementary registers"),
         subreg_size,
@@ -77,10 +81,112 @@ static void addPartialRegisters(
         {},
         subreg_offset};
 
-    addSupplementaryRegister(regs, subreg);
+    addSupplementaryRegister(regs, new_reg);
+  }
+}
+
+static void
+addCombinedRegisters(std::vector<DynamicRegisterInfo::Register> &regs,
+                     llvm::ArrayRef<RegData *> subregs1,
+                     llvm::ArrayRef<RegData *> subregs2, uint32_t base_size,
+                     lldb::Encoding encoding, lldb::Format format) {
+  for (auto it : llvm::zip(subregs1, subregs2)) {
+    RegData *regdata1, *regdata2;
+    std::tie(regdata1, regdata2) = it;
+    assert(regdata1);
+    assert(regdata2);
+
+    // verify that we've got matching target registers
+    if (regdata1->subreg_name != regdata2->subreg_name)
+      continue;
+
+    uint32_t base_index1 = regdata1->base_index.getValue();
+    uint32_t base_index2 = regdata2->base_index.getValue();
+    if (regs[base_index1].byte_size != base_size ||
+        regs[base_index2].byte_size != base_size)
+      continue;
+
+    lldb_private::DynamicRegisterInfo::Register new_reg{
+        lldb_private::ConstString(regdata1->subreg_name),
+        lldb_private::ConstString(),
+        lldb_private::ConstString("supplementary registers"),
+        base_size * 2,
+        LLDB_INVALID_INDEX32,
+        encoding,
+        format,
+        LLDB_INVALID_REGNUM,
+        LLDB_INVALID_REGNUM,
+        LLDB_INVALID_REGNUM,
+        LLDB_INVALID_REGNUM,
+        {base_index1, base_index2},
+        {}};
+
+    addSupplementaryRegister(regs, new_reg);
   }
 }
 
+typedef llvm::SmallDenseMap<llvm::StringRef, llvm::SmallVector<RegData, 4>, 64>
+    BaseRegToRegsMap;
+
+#define GPRh(l)                                                                \
+  {                                                                            \
+    is64bit ? BaseRegToRegsMap::value_type("r" l "x", {{GPR32, "e" l "x"},     \
+                                                       {GPR16, l "x"},         \
+                                                       {GPR8h, l "h"},         \
+                                                       {GPR8, l "l"}})         \
+            : BaseRegToRegsMap::value_type(                                    \
+                  "e" l "x", {{GPR16, l "x"}, {GPR8h, l "h"}, {GPR8, l "l"}})  \
+  }
+
+#define GPR(r16)                                                               \
+  {                                                                            \
+    is64bit ? BaseRegToRegsMap::value_type(                                    \
+                  "r" r16, {{GPR32, "e" r16}, {GPR16, r16}, {GPR8, r16 "l"}})  \
+            : BaseRegToRegsMap::value_type("e" r16,                            \
+                                           {{GPR16, r16}, {GPR8, r16 "l"}})    \
+  }
+
+#define GPR64(n)                                                               \
+  {                                                                            \
+    BaseRegToRegsMap::value_type(                                              \
+        "r" #n,                                                                \
+        {{GPR32, "r" #n "d"}, {GPR16, "r" #n "w"}, {GPR8, "r" #n "l"}})        \
+  }
+
+#define STMM(n)                                                                \
+  { BaseRegToRegsMap::value_type("st" #n, {{MM, "mm" #n}}) }
+
+#define YMM(n)                                                                 \
+  {BaseRegToRegsMap::value_type("ymm" #n "h", {{YMM_YMMh, "ymm" #n}})}, {      \
+    BaseRegToRegsMap::value_type("xmm" #n, {{YMM_XMM, "ymm" #n}})              \
+  }
+
+BaseRegToRegsMap makeBaseRegMap(bool is64bit) {
+  BaseRegToRegsMap out{
+      {// GPRs common to amd64 & i386
+       GPRh("a"), GPRh("b"), GPRh("c"), GPRh("d"), GPR("si"), GPR("di"),
+       GPR("bp"), GPR("sp"),
+
+       // ST/MM registers
+       STMM(0), STMM(1), STMM(2), STMM(3), STMM(4), STMM(5), STMM(6), STMM(7),
+
+       // lower YMM registers (common to amd64 & i386)
+       YMM(0), YMM(1), YMM(2), YMM(3), YMM(4), YMM(5), YMM(6), YMM(7)}};
+
+  if (is64bit) {
+    BaseRegToRegsMap amd64_regs{{// GPRs specific to amd64
+                                 GPR64(8), GPR64(9), GPR64(10), GPR64(11),
+                                 GPR64(12), GPR64(13), GPR64(14), GPR64(15),
+
+                                 // higher YMM registers (specific to amd64)
+                                 YMM(8), YMM(9), YMM(10), YMM(11), YMM(12),
+                                 YMM(13), YMM(14), YMM(15)}};
+    out.insert(amd64_regs.begin(), amd64_regs.end());
+  }
+
+  return out;
+}
+
 void ABIX86::AugmentRegisterInfo(
     std::vector<DynamicRegisterInfo::Register> &regs) {
   MCBasedABI::AugmentRegisterInfo(regs);
@@ -91,83 +197,54 @@ void ABIX86::AugmentRegisterInfo(
 
   uint32_t gpr_base_size =
       process_sp->GetTarget().GetArchitecture().GetAddressByteSize();
-  bool is64bit = gpr_base_size == 8;
-
-  typedef RegisterMap::value_type RegPair;
-#define GPR_BASE(basename) (is64bit ? "r" basename : "e" basename)
-  RegisterMap gpr_regs{{
-      RegPair(GPR_BASE("ax"), {"eax", "ax", "ah", "al"}),
-      RegPair(GPR_BASE("bx"), {"ebx", "bx", "bh", "bl"}),
-      RegPair(GPR_BASE("cx"), {"ecx", "cx", "ch", "cl"}),
-      RegPair(GPR_BASE("dx"), {"edx", "dx", "dh", "dl"}),
-      RegPair(GPR_BASE("si"), {"esi", "si", "", "sil"}),
-      RegPair(GPR_BASE("di"), {"edi", "di", "", "dil"}),
-      RegPair(GPR_BASE("bp"), {"ebp", "bp", "", "bpl"}),
-      RegPair(GPR_BASE("sp"), {"esp", "sp", "", "spl"}),
-  }};
-#undef GPR_BASE
-  if (is64bit) {
-#define R(base) RegPair(base, {base "d", base "w", "", base "l"})
-    RegisterMap amd64_regs{{
-        R("r8"),
-        R("r9"),
-        R("r10"),
-        R("r11"),
-        R("r12"),
-        R("r13"),
-        R("r14"),
-        R("r15"),
-    }};
-#undef R
-    gpr_regs.insert(amd64_regs.begin(), amd64_regs.end());
-  }
 
-  RegisterMap st_regs{{
-      RegPair("st0", {"mm0"}),
-      RegPair("st1", {"mm1"}),
-      RegPair("st2", {"mm2"}),
-      RegPair("st3", {"mm3"}),
-      RegPair("st4", {"mm4"}),
-      RegPair("st5", {"mm5"}),
-      RegPair("st6", {"mm6"}),
-      RegPair("st7", {"mm7"}),
-  }};
-
-  // regs from gpr_basenames, in list order
-  std::vector<uint32_t> gpr_base_reg_indices;
-  // st0..st7, in list order
-  std::vector<uint32_t> st_reg_indices;
-  // map used for fast register lookups
+  // primary map from a base register to its subregisters
+  BaseRegToRegsMap base_reg_map = makeBaseRegMap(gpr_base_size == 8);
+  // set used for fast matching of register names to subregisters
   llvm::SmallDenseSet<llvm::StringRef, 64> subreg_name_set;
-
-  // put all subreg names into the lookup set
-  for (const RegisterMap &regset : {gpr_regs, st_regs}) {
-    for (const RegPair &kv : regset)
-      subreg_name_set.insert(kv.second.begin(), kv.second.end());
+  // convenience array providing access to all subregisters of given kind,
+  // sorted by base register index
+  std::array<llvm::SmallVector<RegData *, 16>, RegKindCount> subreg_by_kind;
+
+  // prepare the set of all known subregisters
+  for (const auto &x : base_reg_map) {
+    for (const auto &subreg : x.second)
+      subreg_name_set.insert(subreg.subreg_name);
   }
 
+  // iterate over all registers
   for (const auto &x : llvm::enumerate(regs)) {
     llvm::StringRef reg_name = x.value().name.GetStringRef();
-    // find expected base registers
-    if (gpr_regs.find(reg_name) != gpr_regs.end())
-      gpr_base_reg_indices.push_back(x.index());
-    else if (st_regs.find(reg_name) != st_regs.end())
-      st_reg_indices.push_back(x.index());
     // abort if at least one sub-register is already present
-    else if (llvm::is_contained(subreg_name_set, reg_name))
+    if (llvm::is_contained(subreg_name_set, reg_name))
       return;
+
+    auto found = base_reg_map.find(reg_name);
+    if (found == base_reg_map.end())
+      continue;
+
+    for (auto &subreg : found->second) {
+      // fill in base register indices
+      subreg.base_index = x.index();
+      // fill subreg_by_kind map-array
+      subreg_by_kind[static_cast<size_t>(subreg.subreg_kind)].push_back(
+          &subreg);
+    }
   }
 
-  if (is64bit)
-    addPartialRegisters(regs, gpr_base_reg_indices, gpr_regs, gpr_base_size,
-                        RegKind::GPR32, eEncodingUint, eFormatHex, 4);
-  addPartialRegisters(regs, gpr_base_reg_indices, gpr_regs, gpr_base_size,
-                      RegKind::GPR16, eEncodingUint, eFormatHex, 2);
-  addPartialRegisters(regs, gpr_base_reg_indices, gpr_regs, gpr_base_size,
-                      RegKind::GPR8h, eEncodingUint, eFormatHex, 1, 1);
-  addPartialRegisters(regs, gpr_base_reg_indices, gpr_regs, gpr_base_size,
-                      RegKind::GPR8, eEncodingUint, eFormatHex, 1);
-
-  addPartialRegisters(regs, st_reg_indices, st_regs, 10, RegKind::MM,
-                      eEncodingUint, eFormatHex, 8);
+  // now add registers by kind
+  addPartialRegisters(regs, subreg_by_kind[GPR32], gpr_base_size, eEncodingUint,
+                      eFormatHex, 4);
+  addPartialRegisters(regs, subreg_by_kind[GPR16], gpr_base_size, eEncodingUint,
+                      eFormatHex, 2);
+  addPartialRegisters(regs, subreg_by_kind[GPR8h], gpr_base_size, eEncodingUint,
+                      eFormatHex, 1, 1);
+  addPartialRegisters(regs, subreg_by_kind[GPR8], gpr_base_size, eEncodingUint,
+                      eFormatHex, 1);
+
+  addPartialRegisters(regs, subreg_by_kind[MM], 10, eEncodingUint, eFormatHex,
+                      8);
+
+  addCombinedRegisters(regs, subreg_by_kind[YMM_XMM], subreg_by_kind[YMM_YMMh],
+                       16, eEncodingVector, eFormatVectorOfUInt8);
 }

lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

@@ -87,10 +87,34 @@ bool GDBRemoteRegisterContext::ReadRegister(const RegisterInfo *reg_info,
     const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
     if (m_reg_valid[reg] == false)
       return false;
-    const bool partial_data_ok = false;
-    Status error(value.SetValueFromData(
-        reg_info, m_reg_data, reg_info->byte_offset, partial_data_ok));
-    return error.Success();
+    if (reg_info->value_regs &&
+        reg_info->value_regs[0] != LLDB_INVALID_REGNUM &&
+        reg_info->value_regs[1] != LLDB_INVALID_REGNUM) {
+      std::vector<char> combined_data;
+      uint32_t offset = 0;
+      for (int i = 0; reg_info->value_regs[i] != LLDB_INVALID_REGNUM; i++) {
+        const RegisterInfo *parent_reg = GetRegisterInfo(
+            eRegisterKindLLDB, reg_info->value_regs[i]);
+        if (!parent_reg)
+          return false;
+        combined_data.resize(offset + parent_reg->byte_size);
+        if (m_reg_data.CopyData(parent_reg->byte_offset, parent_reg->byte_size,
+                                combined_data.data() + offset) !=
+            parent_reg->byte_size)
+          return false;
+        offset += parent_reg->byte_size;
+      }
+
+      Status error;
+      return value.SetFromMemoryData(
+                 reg_info, combined_data.data(), combined_data.size(),
+                 m_reg_data.GetByteOrder(), error) == combined_data.size();
+    } else {
+      const bool partial_data_ok = false;
+      Status error(value.SetValueFromData(
+          reg_info, m_reg_data, reg_info->byte_offset, partial_data_ok));
+      return error.Success();
+    }
   }
   return false;
 }
@@ -272,8 +296,38 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) {
 bool GDBRemoteRegisterContext::WriteRegister(const RegisterInfo *reg_info,
                                              const RegisterValue &value) {
   DataExtractor data;
-  if (value.GetData(data))
-    return WriteRegisterBytes(reg_info, data, 0);
+  if (value.GetData(data)) {
+    if (reg_info->value_regs &&
+        reg_info->value_regs[0] != LLDB_INVALID_REGNUM &&
+        reg_info->value_regs[1] != LLDB_INVALID_REGNUM) {
+      uint32_t combined_size = 0;
+      for (int i = 0; reg_info->value_regs[i] != LLDB_INVALID_REGNUM; i++) {
+        const RegisterInfo *parent_reg = GetRegisterInfo(
+            eRegisterKindLLDB, reg_info->value_regs[i]);
+        if (!parent_reg)
+          return false;
+        combined_size += parent_reg->byte_size;
+      }
+
+      if (data.GetByteSize() < combined_size)
+        return false;
+
+      uint32_t offset = 0;
+      for (int i = 0; reg_info->value_regs[i] != LLDB_INVALID_REGNUM; i++) {
+        const RegisterInfo *parent_reg = GetRegisterInfo(
+            eRegisterKindLLDB, reg_info->value_regs[i]);
+        assert(parent_reg);
+
+        DataExtractor parent_data{data, offset, parent_reg->byte_size};
+        if (!WriteRegisterBytes(parent_reg, parent_data, 0))
+          return false;
+        offset += parent_reg->byte_size;
+      }
+      assert(offset == combined_size);
+      return true;
+    } else
+      return WriteRegisterBytes(reg_info, data, 0);
+  }
   return false;
 }