mgorny updated this revision to Diff 380058.
mgorny retitled this revision from "[lldb] [Process/gdb-remote] Support 
combining xmm* and ymm*h regs into ymm*" to "[lldb] [ABI/X86] Support combining 
xmm* and ymm*h regs into ymm*".
mgorny added a comment.

Rebase to use the new `ABIX86::AugmentRegisterInfo()` API.


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

https://reviews.llvm.org/D108937

Files:
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===================================================================
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -199,6 +199,29 @@
         self.match("register read st0",
                    ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
 
+        self.runCmd("register write xmm0 \"{0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 "
+                    "0xf8 0xf7 0xf6 0xf5 0xf4 0xf3 0xf2 0xf1 0xf0}\"")
+        self.match("register read ymm0",
+                   ["ymm0 = {0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 0xf8 0xf7 0xf6 "
+                    "0xf5 0xf4 0xf3 0xf2 0xf1 0xf0 0xb1 0xb2 0xb3 0xb4 0xb5 "
+                    "0xb6 0xb7 0xb8 0xb9 0xba 0xbb 0xbc 0xbd 0xbe 0xbf 0xc0}"])
+
+        self.runCmd("register write ymm0h \"{0xef 0xee 0xed 0xec 0xeb 0xea 0xe9 "
+                    "0xe8 0xe7 0xe6 0xe5 0xe4 0xe3 0xe2 0xe1 0xe0}\"")
+        self.match("register read ymm0",
+                   ["ymm0 = {0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 0xf8 0xf7 0xf6 "
+                    "0xf5 0xf4 0xf3 0xf2 0xf1 0xf0 0xef 0xee 0xed 0xec 0xeb "
+                    "0xea 0xe9 0xe8 0xe7 0xe6 0xe5 0xe4 0xe3 0xe2 0xe1 0xe0}"])
+
+        self.runCmd("register write ymm0 \"{0xd0 0xd1 0xd2 0xd3 0xd4 0xd5 0xd6 "
+                    "0xd7 0xd8 0xd9 0xda 0xdb 0xdc 0xdd 0xde 0xdf 0xe0 0xe1 "
+                    "0xe2 0xe3 0xe4 0xe5 0xe6 0xe7 0xe8 0xe9 0xea 0xeb 0xec "
+                    "0xed 0xee 0xef}\"")
+        self.match("register read ymm0",
+                   ["ymm0 = {0xd0 0xd1 0xd2 0xd3 0xd4 0xd5 0xd6 0xd7 0xd8 0xd9 "
+                    "0xda 0xdb 0xdc 0xdd 0xde 0xdf 0xe0 0xe1 0xe2 0xe3 0xe4 "
+                    "0xe5 0xe6 0xe7 0xe8 0xe9 0xea 0xeb 0xec 0xed 0xee 0xef}"])
+
     @skipIfXmlSupportMissing
     @skipIfRemote
     @skipIfLLVMTargetMissing("X86")
@@ -361,6 +384,29 @@
         self.match("register read st0",
                    ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
 
+        self.runCmd("register write xmm0 \"{0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 "
+                    "0xf8 0xf7 0xf6 0xf5 0xf4 0xf3 0xf2 0xf1 0xf0}\"")
+        self.match("register read ymm0",
+                   ["ymm0 = {0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 0xf8 0xf7 0xf6 "
+                    "0xf5 0xf4 0xf3 0xf2 0xf1 0xf0 0xb1 0xb2 0xb3 0xb4 0xb5 "
+                    "0xb6 0xb7 0xb8 0xb9 0xba 0xbb 0xbc 0xbd 0xbe 0xbf 0xc0}"])
+
+        self.runCmd("register write ymm0h \"{0xef 0xee 0xed 0xec 0xeb 0xea 0xe9 "
+                    "0xe8 0xe7 0xe6 0xe5 0xe4 0xe3 0xe2 0xe1 0xe0}\"")
+        self.match("register read ymm0",
+                   ["ymm0 = {0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 0xf8 0xf7 0xf6 "
+                    "0xf5 0xf4 0xf3 0xf2 0xf1 0xf0 0xef 0xee 0xed 0xec 0xeb "
+                    "0xea 0xe9 0xe8 0xe7 0xe6 0xe5 0xe4 0xe3 0xe2 0xe1 0xe0}"])
+
+        self.runCmd("register write ymm0 \"{0xd0 0xd1 0xd2 0xd3 0xd4 0xd5 0xd6 "
+                    "0xd7 0xd8 0xd9 0xda 0xdb 0xdc 0xdd 0xde 0xdf 0xe0 0xe1 "
+                    "0xe2 0xe3 0xe4 0xe5 0xe6 0xe7 0xe8 0xe9 0xea 0xeb 0xec "
+                    "0xed 0xee 0xef}\"")
+        self.match("register read ymm0",
+                   ["ymm0 = {0xd0 0xd1 0xd2 0xd3 0xd4 0xd5 0xd6 0xd7 0xd8 0xd9 "
+                    "0xda 0xdb 0xdc 0xdd 0xde 0xdf 0xe0 0xe1 0xe2 0xe3 0xe4 "
+                    "0xe5 0xe6 0xe7 0xe8 0xe9 0xea 0xeb 0xec 0xed 0xee 0xef}"])
+
     @skipIfXmlSupportMissing
     @skipIfRemote
     @skipIfLLVMTargetMissing("AArch64")
Index: lldb/source/Target/DynamicRegisterInfo.cpp
===================================================================
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -463,20 +463,11 @@
     m_sets[set].registers = m_set_reg_nums[set].data();
   }
 
-  // sort and unique all value registers and make sure each is terminated with
-  // LLDB_INVALID_REGNUM
+  // make sure value_regs are terminated with LLDB_INVALID_REGNUM
 
   for (reg_to_regs_map::iterator pos = m_value_regs_map.begin(),
                                  end = m_value_regs_map.end();
        pos != end; ++pos) {
-    if (pos->second.size() > 1) {
-      llvm::sort(pos->second.begin(), pos->second.end());
-      reg_num_collection::iterator unique_end =
-          std::unique(pos->second.begin(), pos->second.end());
-      if (unique_end != pos->second.end())
-        pos->second.erase(unique_end, pos->second.end());
-    }
-    assert(!pos->second.empty());
     if (pos->second.back() != LLDB_INVALID_REGNUM)
       pos->second.push_back(LLDB_INVALID_REGNUM);
   }
@@ -678,13 +669,16 @@
   // Now update all value_regs with each register info as needed
   for (auto &reg : m_regs) {
     if (reg.value_regs != nullptr) {
-      // Assign a valid offset to all pseudo registers if not assigned by stub.
-      // Pseudo registers with value_regs list populated will share same offset
-      // as that of their corresponding primary register in value_regs list.
+      // Assign a valid offset to all pseudo registers that have only a single
+      // parent register in value_regs list, if not assigned by stub.  Pseudo
+      // registers with value_regs list populated will share same offset as
+      // that of their corresponding parent register.
       if (reg.byte_offset == LLDB_INVALID_INDEX32) {
         uint32_t value_regnum = reg.value_regs[0];
-        if (value_regnum != LLDB_INVALID_INDEX32) {
-          reg.byte_offset = GetRegisterInfoAtIndex(value_regnum)->byte_offset;
+        if (value_regnum != LLDB_INVALID_INDEX32 &&
+            reg.value_regs[1] == LLDB_INVALID_INDEX32) {
+          reg.byte_offset =
+              GetRegisterInfoAtIndex(value_regnum)->byte_offset;
           auto it = m_value_reg_offset_map.find(reg.kinds[eRegisterKindLLDB]);
           if (it != m_value_reg_offset_map.end())
             reg.byte_offset += it->second;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4317,7 +4317,9 @@
             reg_info.encoding = eEncodingIEEE754;
           } else if (gdb_type == "aarch64v" ||
                      llvm::StringRef(gdb_type).startswith("vec") ||
-                     gdb_type == "i387_ext") {
+                     gdb_type == "i387_ext" || gdb_type == "uint128") {
+            // lldb doesn't handle 128-bit uints correctly (for ymm*h), so treat
+            // them as vector (similarly to xmm/ymm)
             reg_info.format = eFormatVectorOfUInt8;
             reg_info.encoding = eEncodingVector;
           }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -87,10 +87,34 @@
     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::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;
 }
 
Index: lldb/source/Plugins/ABI/X86/ABIX86.cpp
===================================================================
--- lldb/source/Plugins/ABI/X86/ABIX86.cpp
+++ lldb/source/Plugins/ABI/X86/ABIX86.cpp
@@ -40,6 +40,7 @@
   GPR8,
 
   MM = 0,
+  YMM = 0,
 };
 
 typedef llvm::SmallDenseMap<llvm::StringRef,
@@ -133,15 +134,56 @@
       RegPair("st7", {"mm7"}),
   }};
 
+  RegisterMap ymmh_regs{{
+      RegPair("ymm0h", {"ymm0"}),
+      RegPair("ymm1h", {"ymm1"}),
+      RegPair("ymm2h", {"ymm2"}),
+      RegPair("ymm3h", {"ymm3"}),
+      RegPair("ymm4h", {"ymm4"}),
+      RegPair("ymm5h", {"ymm5"}),
+      RegPair("ymm6h", {"ymm6"}),
+      RegPair("ymm7h", {"ymm7"}),
+      RegPair("ymm8h", {"ymm8"}),
+      RegPair("ymm9h", {"ymm9"}),
+      RegPair("ymm10h", {"ymm10"}),
+      RegPair("ymm11h", {"ymm11"}),
+      RegPair("ymm12h", {"ymm12"}),
+      RegPair("ymm13h", {"ymm13"}),
+      RegPair("ymm14h", {"ymm14"}),
+      RegPair("ymm15h", {"ymm15"}),
+  }};
+  llvm::DenseSet<llvm::StringRef> xmm_regs{{
+      "xmm0",
+      "xmm1",
+      "xmm2",
+      "xmm3",
+      "xmm4",
+      "xmm5",
+      "xmm6",
+      "xmm7",
+      "xmm8",
+      "xmm9",
+      "xmm10",
+      "xmm11",
+      "xmm12",
+      "xmm13",
+      "xmm14",
+      "xmm15",
+  }};
+
   // 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;
+  // xmm0..xmm15
+  std::vector<uint32_t> xmm_reg_indices;
+  // ymm0h..ymm15h
+  std::vector<uint32_t> ymmh_reg_indices;
   // map used for fast register lookups
   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 RegisterMap &regset : {gpr_regs, st_regs, ymmh_regs}) {
     for (const RegPair &kv : regset)
       subreg_name_set.insert(kv.second.begin(), kv.second.end());
   }
@@ -153,6 +195,10 @@
       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());
+    else if (xmm_regs.find(reg_name) != xmm_regs.end())
+      xmm_reg_indices.push_back(x.index());
+    else if (ymmh_regs.find(reg_name) != ymmh_regs.end())
+      ymmh_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))
       return;
@@ -170,4 +216,39 @@
 
   addPartialRegisters(regs, st_reg_indices, st_regs, 10, RegKind::MM,
                       eEncodingUint, eFormatHex, 8);
+
+  for (auto it : llvm::zip(ymmh_reg_indices, xmm_reg_indices)) {
+    uint32_t ymmh_index, xmm_index;
+    std::tie(ymmh_index, xmm_index) = it;
+
+    DynamicRegisterInfo::Register &xmm_reg = regs[xmm_index];
+    DynamicRegisterInfo::Register &ymmh_reg = regs[ymmh_index];
+    if (xmm_reg.byte_size != 16 || ymmh_reg.byte_size != 16)
+      continue;
+
+    llvm::StringRef xmm_name = xmm_reg.name.GetStringRef();
+    llvm::StringRef ymmh_name = ymmh_reg.name.GetStringRef();
+    // make sure we got a matching index, i.e. xmmN matches ymmNh
+    if (xmm_name.drop_front(3) != ymmh_name.drop_front(3).drop_back(1))
+      continue;
+
+    llvm::StringRef subreg_name =
+        ymmh_regs.lookup(ymmh_name)[static_cast<int>(RegKind::YMM)];
+    lldb_private::DynamicRegisterInfo::Register subreg{
+        lldb_private::ConstString(subreg_name),
+        lldb_private::ConstString(),
+        lldb_private::ConstString("supplementary registers"),
+        32,
+        LLDB_INVALID_INDEX32,
+        eEncodingVector,
+        eFormatVectorOfUInt8,
+        LLDB_INVALID_REGNUM,
+        LLDB_INVALID_REGNUM,
+        LLDB_INVALID_REGNUM,
+        LLDB_INVALID_REGNUM,
+        {xmm_index, ymmh_index},
+        {}};
+
+    addSupplementaryRegister(regs, subreg);
+  }
 }
Index: lldb/include/lldb/lldb-private-types.h
===================================================================
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -51,8 +51,10 @@
   /// 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-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to