[lldb] Generic support for caching register set reads/writes

[mgorny]

Summary

Introduce a new class for caching register set reads and writes
in the generic NativeRegisterContextRegisterInfo class. Use it
in the NetBSD plugin for initial testing.

The implementation provides a generic caching version of ReadRegister()
and WriteRegister() that call virtual methods implemented in actual
NativeRegisterContext* classes. It supports both caching reads,
and delayed writes. The cache is flushed whenever the thread resumes
execution or a new read follows writes.

The hook API consists of five methods that need to be implemented
in NativeRegisterContext* classes:

• GetRegisterSetForRegNum() to map register number to a register set number (the latter being opaque to NativeRegisterContextRegisterInfo).

• ReadRegisterSet() and WriteRegisterSet() to respectively read regset into cache and write it back. The actual cache storage is provided by final classes (m_gpr, m_fpr...).

• GetRegisterFromCache() and SetRegisterInCache() to respectively get and update individual register values inside the cache. The implementation of both methods can assume that ReadRegisterSet() has been called to fill the cache, and must not call either ReadRegisterSet() or WriteRegisterSet().

The NativeRegisterContextRegisterInfo class maintains state map of cache
for individual regsets, and issues appropriate ReadRegisterSet()
and WriteRegisterSet() calls to maintain it. It includes
a FlushRegisterSets() method that needs to be called whenever the thread
is about to resume execution.

[mgorny]

Ok, this is going to have to wait. I've found some hard bug in NetBSD's xstate support and don't have time+resources to figure it out at the moment.

[labath]

I'm wondering if this could be implemented as some kind of a separate class which you "mix" into your register context when you want to do caching. Possibly something like this:

class NRCLinux_MyArch: public NRCLinux, private RCCache::Backend {
  RCCache m_cache;

  NRCLinux_MyArch(...) : m_cache(this) {}

  // NRC interface
  ReadRegister(...) override { return m_cache.ReadRegister(...); }
  ...

  // Backend interface
  ReadReally(...) override { action happens here }
};

That would enable individual register contexts to explicitly opt into caching, which might be cleaner in the long term, as I anticipate it will take a very long time to convert all registers contexts to the new way of doing things.

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

25 ↗

(On Diff #297917)

I think using Optional<integral> would be better. Although, I'm not sure what are the cases where a register does not belong to any register set.

64–83 ↗

(On Diff #297917)

I think new interfaces should use llvm::Error/Expected

[mgorny]

Moved stuff into mixin-style class. Changing return types is next on the list.

[mgorny]

Switched GetRegisterSetForRegNum to llvm::Optional. Also noticed that NativeRegisterContextNetBSD_x86_64::GetRegisterSetCount() is entirely meaningless.

[mgorny]

Rework error handling to use llvm::Error in the new methods.

[mgorny]

Now with actual caching support.

[labath]

I guess this slipped off my radar.... What's the state of this? Are you still interested in the patch? If yes, we can try again after rebasing.

[mgorny]

I don't think I really want to work on this right now. Even if it still works reliably, I'm not sure if the gain is really worth the added complexity (and risks).

lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

for (const auto &abs_thread : m_threads) {
 
     const ResumeAction *action =
         resume_actions.GetActionForThread(thread.GetID(), true);
     // we need to explicit issue suspend requests, so it is simpler to map it
     // into proper action
     ResumeAction suspend_action{thread.GetID(), eStateSuspended,
                                 LLDB_INVALID_SIGNAL_NUMBER};
 
+    auto &regctx = static_cast<NativeRegisterContextNetBSD &>(
+        thread.GetRegisterContext());
+    llvm::Error flush_error = regctx.FlushRegisterSets();
+    if (flush_error)
+      return Status(std::move(flush_error));
+
     if (action == nullptr) {
       LLDB_LOG(log, "no action specified for pid {0} tid {1}", GetID(),
                thread.GetID());
       action = &suspend_action;
     }
 
     LLDB_LOG(
         log,

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h

 //===-- NativeRegisterContextNetBSD.h ---------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef lldb_NativeRegisterContextNetBSD_h
 #define lldb_NativeRegisterContextNetBSD_h
 
 #include "lldb/Host/common/NativeThreadProtocol.h"
 
+#include "llvm/Support/Error.h"
+
+#include "Plugins/Process/Utility/NativeRegisterContextCache.h"
 #include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h"
 
 namespace lldb_private {
 namespace process_netbsd {
 
 class NativeProcessNetBSD;
 
 class NativeRegisterContextNetBSD
-    : public virtual NativeRegisterContextRegisterInfo {
+    : public virtual NativeRegisterContextRegisterInfo,
+      public NativeRegisterContextCache {
 public:
   // This function is implemented in the NativeRegisterContextNetBSD_*
   // subclasses to create a new instance of the host specific
   // NativeRegisterContextNetBSD. The implementations can't collide as only one
   // NativeRegisterContextNetBSD_* variant should be compiled into the final
   // executable.
   static NativeRegisterContextNetBSD *
   CreateHostNativeRegisterContextNetBSD(const ArchSpec &target_arch,
                                         NativeThreadProtocol &native_thread);
   virtual Status
   CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) = 0;
 
 protected:
-  Status DoRegisterSet(int req, void *buf);
+  llvm::Error DoRegisterSet(int req, void *buf);
   virtual NativeProcessNetBSD &GetProcess();
   virtual ::pid_t GetProcessPid();
 };
 
 } // namespace process_netbsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextNetBSD_h

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp

 using namespace lldb_private;
 using namespace lldb_private::process_netbsd;
 
 // clang-format off
 #include <sys/types.h>
 #include <sys/ptrace.h>
 // clang-format on
 
-Status NativeRegisterContextNetBSD::DoRegisterSet(int ptrace_req, void *buf) {
-  return NativeProcessNetBSD::PtraceWrapper(ptrace_req, GetProcessPid(), buf,
-                                            m_thread.GetID());
+llvm::Error NativeRegisterContextNetBSD::DoRegisterSet(int ptrace_req, void *buf) {
+  return NativeProcessNetBSD::PtraceWrapper(ptrace_req, GetProcessPid(), buf, m_thread.GetID()).ToError();
 }
 
 NativeProcessNetBSD &NativeRegisterContextNetBSD::GetProcess() {
   return static_cast<NativeProcessNetBSD &>(m_thread.GetProcess());
 }
 
 ::pid_t NativeRegisterContextNetBSD::GetProcessPid() {
   return GetProcess().GetID();
 }

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

 #include <machine/reg.h>
 // clang-format on
 
 #include "Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h"
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
 #include "Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h"
 #include "Plugins/Process/Utility/lldb-x86-register-enums.h"
 
+#include "llvm/Support/Error.h"
+
 namespace lldb_private {
 namespace process_netbsd {
 
 class NativeProcessNetBSD;
 
 class NativeRegisterContextNetBSD_x86_64
     : public NativeRegisterContextNetBSD,
       public NativeRegisterContextWatchpoint_x86 {
 public:
   NativeRegisterContextNetBSD_x86_64(const ArchSpec &target_arch,
                                      NativeThreadProtocol &native_thread);
   uint32_t GetRegisterSetCount() const override;
 
   const RegisterSet *GetRegisterSet(uint32_t set_index) const override;
 
-  Status ReadRegister(const RegisterInfo *reg_info,
-                      RegisterValue &reg_value) override;
-
-  Status WriteRegister(const RegisterInfo *reg_info,
-                       const RegisterValue &reg_value) override;
-
   Status ReadAllRegisterValues(lldb::DataBufferSP &data_sp) override;
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
   Status
   CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) override;
 
+protected:
+  llvm::Optional<uint32_t> GetRegisterSetForRegNum(uint32_t reg_index) const override;
+
+  llvm::Error ReadRegisterSet(uint32_t set) override;
+  llvm::Error WriteRegisterSet(uint32_t set) override;
+
+  llvm::Error GetRegisterFromCache(const RegisterInfo *reg_info,
+                              RegisterValue &reg_value,
+                              uint32_t set) override;
+  llvm::Error SetRegisterInCache(const RegisterInfo *reg_info,
+                            const RegisterValue &reg_value,
+                            uint32_t set) override;
+
 private:
   // Private member types.
   enum { GPRegSet, XStateRegSet, DBRegSet };
 
   // Private member variables.
   struct reg m_gpr;
   struct xstate m_xstate;
   struct dbreg m_dbr;
 
-  int GetSetForNativeRegNum(int reg_num) const;
   int GetDR(int num) const;
-
-  Status ReadRegisterSet(uint32_t set);
-  Status WriteRegisterSet(uint32_t set);
 };
 
 } // namespace process_netbsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextNetBSD_x86_64_h
 
 #endif // defined(__x86_64__)

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

 #include "NativeRegisterContextNetBSD_x86_64.h"
 
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 
+#include "llvm/Support/FormatVariadic.h"
+
 #include "Plugins/Process/Utility/RegisterContextNetBSD_i386.h"
 #include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h"
 
 // clang-format off
 #include <sys/types.h>
 #include <sys/ptrace.h>
 #include <sys/sysctl.h>
 #include <sys/uio.h>

 };
 static_assert((sizeof(g_dbr_regnums_i386) / sizeof(g_dbr_regnums_i386[0])) -
                       1 ==
                   k_num_dbr_registers_i386,
               "g_dbr_regnums_i386 has wrong number of register infos");
 
 
 // Number of register sets provided by this context.
-enum { k_num_register_sets = 4 };
+enum { k_num_register_sets = 3 };
 
 // Register sets for x86 32-bit.
 static const RegisterSet g_reg_sets_i386[k_num_register_sets] = {
     {"General Purpose Registers", "gpr", k_num_gpr_registers_i386,
      g_gpr_regnums_i386},
     {"Extended State Registers", "xstate",
      k_num_avx_registers_i386 + k_num_mpx_registers_i386,
      g_xstate_regnums_i386},
NativeRegisterContextNetBSD_x86_64::NativeRegisterContextNetBSD_x86_64(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
     : NativeRegisterContextRegisterInfo(
           native_thread, CreateRegisterInfoInterface(target_arch)),
       m_gpr(), m_xstate(), m_dbr() {}
 
 // CONSIDER after local and llgs debugging are merged, register set support can
 // be moved into a base x86-64 class with IsRegisterSetAvailable made virtual.
 uint32_t NativeRegisterContextNetBSD_x86_64::GetRegisterSetCount() const {
-  uint32_t sets = 0;
-  for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
-    if (GetSetForNativeRegNum(set_index) != -1)
-      ++sets;
-  }
-
-  return sets;
+  return k_num_register_sets;
 }
 
 const RegisterSet *
 NativeRegisterContextNetBSD_x86_64::GetRegisterSet(uint32_t set_index) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
     return &g_reg_sets_i386[set_index];
   case llvm::Triple::x86_64:
static constexpr int RegNumX86ToX86_64(int regnum) {
   case lldb_dr6_i386:
   case lldb_dr7_i386:
     return lldb_dr0_x86_64 + regnum - lldb_dr0_i386;
   default:
     llvm_unreachable("Unhandled i386 register.");
   }
 }
 
-int NativeRegisterContextNetBSD_x86_64::GetSetForNativeRegNum(
-    int reg_num) const {
+llvm::Optional<uint32_t> NativeRegisterContextNetBSD_x86_64::GetRegisterSetForRegNum(
+    uint32_t reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
     if (reg_num >= k_first_gpr_i386 && reg_num <= k_last_gpr_i386)
       return GPRegSet;
     if (reg_num >= k_first_fpr_i386 && reg_num <= k_last_fpr_i386)
       return XStateRegSet;
     if (reg_num >= k_first_avx_i386 && reg_num <= k_last_avx_i386)
       return XStateRegSet; // AVX
     if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxr_i386)
-      return -1; // MPXR
+      return llvm::None; // MPXR
     if (reg_num >= k_first_mpxc_i386 && reg_num <= k_last_mpxc_i386)
-      return -1; // MPXC
+      return llvm::None; // MPXC
     if (reg_num >= k_first_dbr_i386 && reg_num <= k_last_dbr_i386)
       return DBRegSet; // DBR
     break;
   case llvm::Triple::x86_64:
     if (reg_num >= k_first_gpr_x86_64 && reg_num <= k_last_gpr_x86_64)
       return GPRegSet;
     if (reg_num >= k_first_fpr_x86_64 && reg_num <= k_last_fpr_x86_64)
       return XStateRegSet;
     if (reg_num >= k_first_avx_x86_64 && reg_num <= k_last_avx_x86_64)
       return XStateRegSet; // AVX
     if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxr_x86_64)
-      return -1; // MPXR
+      return llvm::None; // MPXR
     if (reg_num >= k_first_mpxc_x86_64 && reg_num <= k_last_mpxc_x86_64)
-      return -1; // MPXC
+      return llvm::None; // MPXC
     if (reg_num >= k_first_dbr_x86_64 && reg_num <= k_last_dbr_x86_64)
       return DBRegSet; // DBR
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
   llvm_unreachable("Register does not belong to any register set");
 }
 
-Status NativeRegisterContextNetBSD_x86_64::ReadRegisterSet(uint32_t set) {
+llvm::Error NativeRegisterContextNetBSD_x86_64::ReadRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return DoRegisterSet(PT_GETREGS, &m_gpr);
   case DBRegSet:
     return DoRegisterSet(PT_GETDBREGS, &m_dbr);
   case XStateRegSet: {
     struct iovec iov = {&m_xstate, sizeof(m_xstate)};
     return DoRegisterSet(PT_GETXSTATE, &iov);
   }
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::ReadRegisterSet");
 }
 
-Status NativeRegisterContextNetBSD_x86_64::WriteRegisterSet(uint32_t set) {
+llvm::Error NativeRegisterContextNetBSD_x86_64::WriteRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return DoRegisterSet(PT_SETREGS, &m_gpr);
   case DBRegSet:
     return DoRegisterSet(PT_SETDBREGS, &m_dbr);
   case XStateRegSet: {
     struct iovec iov = {&m_xstate, sizeof(m_xstate)};
     return DoRegisterSet(PT_SETXSTATE, &iov);
   }
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::WriteRegisterSet");
 }
 
-Status
-NativeRegisterContextNetBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
-                                                 RegisterValue &reg_value) {
-  Status error;
-
-  if (!reg_info) {
-    error.SetErrorString("reg_info NULL");
-    return error;
-  }
+llvm::Error
+NativeRegisterContextNetBSD_x86_64::GetRegisterFromCache(
+    const RegisterInfo *reg_info, RegisterValue &reg_value, uint32_t set) {
+  assert(reg_info);
 
   uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
-  if (reg == LLDB_INVALID_REGNUM) {
-    // This is likely an internal register for lldb use only and should not be
-    // directly queried.
-    error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "
-                                   "register, cannot read directly",
-                                   reg_info->name);
-    return error;
-  }
-
-  int set = GetSetForNativeRegNum(reg);
-  if (set == -1) {
-    // This is likely an internal register for lldb use only and should not be
-    // directly queried.
-    error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
-                                   reg_info->name);
-    return error;
-  }
 
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86_64:
     break;
   case llvm::Triple::x86:
     reg = RegNumX86ToX86_64(reg);
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
-  error = ReadRegisterSet(set);
-  if (error.Fail())
-    return error;
-
   switch (reg) {
 #if defined(__x86_64__)
   case lldb_rax_x86_64:
     reg_value = (uint64_t)m_gpr.regs[_REG_RAX];
     break;
   case lldb_rbx_x86_64:
     reg_value = (uint64_t)m_gpr.regs[_REG_RBX];
     break;
#endif
   case lldb_xmm8_x86_64:
   case lldb_xmm9_x86_64:
   case lldb_xmm10_x86_64:
   case lldb_xmm11_x86_64:
   case lldb_xmm12_x86_64:
   case lldb_xmm13_x86_64:
   case lldb_xmm14_x86_64:
   case lldb_xmm15_x86_64:
-    if (!(m_xstate.xs_rfbm & XCR0_SSE)) {
-      error.SetErrorStringWithFormat(
-          "register \"%s\" not supported by CPU/kernel", reg_info->name);
-    } else {
-      reg_value.SetBytes(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64],
-                         reg_info->byte_size, endian::InlHostByteOrder());
-    }
+    if (!(m_xstate.xs_rfbm & XCR0_SSE))
+      return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str());
     break;
   case lldb_ymm0_x86_64:
   case lldb_ymm1_x86_64:
   case lldb_ymm2_x86_64:
   case lldb_ymm3_x86_64:
   case lldb_ymm4_x86_64:
   case lldb_ymm5_x86_64:
   case lldb_ymm6_x86_64:
   case lldb_ymm7_x86_64:
   case lldb_ymm8_x86_64:
   case lldb_ymm9_x86_64:
   case lldb_ymm10_x86_64:
   case lldb_ymm11_x86_64:
   case lldb_ymm12_x86_64:
   case lldb_ymm13_x86_64:
   case lldb_ymm14_x86_64:
   case lldb_ymm15_x86_64:
+    {
     if (!(m_xstate.xs_rfbm & XCR0_SSE) ||
-        !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) {
-      error.SetErrorStringWithFormat(
-          "register \"%s\" not supported by CPU/kernel", reg_info->name);
-    } else {
-      uint32_t reg_index = reg - lldb_ymm0_x86_64;
-      YMMReg ymm =
-          XStateToYMM(m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
-                      m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
-      reg_value.SetBytes(ymm.bytes, reg_info->byte_size,
-                         endian::InlHostByteOrder());
-    }
+        !(m_xstate.xs_rfbm & XCR0_YMM_Hi128))
+      return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str());
+
     break;
+    }
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
   case lldb_dr3_x86_64:
   case lldb_dr4_x86_64:
   case lldb_dr5_x86_64:
   case lldb_dr6_x86_64:
   case lldb_dr7_x86_64:
     reg_value = (uint64_t)m_dbr.dr[reg - lldb_dr0_x86_64];
     break;
   default:
     llvm_unreachable("Reading unknown/unsupported register");
   }
 
-  return error;
+  return llvm::Error::success();
 }
 
-Status NativeRegisterContextNetBSD_x86_64::WriteRegister(
-    const RegisterInfo *reg_info, const RegisterValue &reg_value) {
-
-  Status error;
-
-  if (!reg_info) {
-    error.SetErrorString("reg_info NULL");
-    return error;
-  }
+llvm::Error NativeRegisterContextNetBSD_x86_64::SetRegisterInCache(
+    const RegisterInfo *reg_info,
+    const RegisterValue &reg_value,
+    uint32_t set) {
+  assert(reg_info);
 
   uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
-  if (reg == LLDB_INVALID_REGNUM) {
-    // This is likely an internal register for lldb use only and should not be
-    // directly queried.
-    error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "
-                                   "register, cannot read directly",
-                                   reg_info->name);
-    return error;
-  }
-
-  int set = GetSetForNativeRegNum(reg);
-  if (set == -1) {
-    // This is likely an internal register for lldb use only and should not be
-    // directly queried.
-    error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
-                                   reg_info->name);
-    return error;
-  }
 
   uint64_t new_xstate_bv = XCR0_X87;  // the most common case
 
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86_64:
     break;
   case llvm::Triple::x86:
     reg = RegNumX86ToX86_64(reg);
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
-  error = ReadRegisterSet(set);
-  if (error.Fail())
-    return error;
-
   switch (reg) {
 #if defined(__x86_64__)
   case lldb_rax_x86_64:
     m_gpr.regs[_REG_RAX] = reg_value.GetAsUInt64();
     break;
   case lldb_rbx_x86_64:
     m_gpr.regs[_REG_RBX] = reg_value.GetAsUInt64();
     break;
#endif
   case lldb_xmm8_x86_64:
   case lldb_xmm9_x86_64:
   case lldb_xmm10_x86_64:
   case lldb_xmm11_x86_64:
   case lldb_xmm12_x86_64:
   case lldb_xmm13_x86_64:
   case lldb_xmm14_x86_64:
   case lldb_xmm15_x86_64:
-    if (!(m_xstate.xs_rfbm & XCR0_SSE)) {
-      error.SetErrorStringWithFormat(
-          "register \"%s\" not supported by CPU/kernel", reg_info->name);
-    } else {
-      ::memcpy(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64],
-               reg_value.GetBytes(), reg_value.GetByteSize());
-      new_xstate_bv = XCR0_SSE;
-    }
+    if (!(m_xstate.xs_rfbm & XCR0_SSE))
+      return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str());
     break;
   case lldb_ymm0_x86_64:
   case lldb_ymm1_x86_64:
   case lldb_ymm2_x86_64:
   case lldb_ymm3_x86_64:
   case lldb_ymm4_x86_64:
   case lldb_ymm5_x86_64:
   case lldb_ymm6_x86_64:
   case lldb_ymm7_x86_64:
   case lldb_ymm8_x86_64:
   case lldb_ymm9_x86_64:
   case lldb_ymm10_x86_64:
   case lldb_ymm11_x86_64:
   case lldb_ymm12_x86_64:
   case lldb_ymm13_x86_64:
   case lldb_ymm14_x86_64:
   case lldb_ymm15_x86_64:
+    {
     if (!(m_xstate.xs_rfbm & XCR0_SSE) ||
-        !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) {
-      error.SetErrorStringWithFormat(
-          "register \"%s\" not supported by CPU/kernel", reg_info->name);
-    } else {
-      uint32_t reg_index = reg - lldb_ymm0_x86_64;
-      YMMReg ymm;
-      ::memcpy(ymm.bytes, reg_value.GetBytes(), reg_value.GetByteSize());
-      YMMToXState(ymm, m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
-                  m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
-      new_xstate_bv = XCR0_SSE | XCR0_YMM_Hi128;
-    }
+        !(m_xstate.xs_rfbm & XCR0_YMM_Hi128))
+      return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str());
+
     break;
+    }
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
   case lldb_dr3_x86_64:
   case lldb_dr4_x86_64:
   case lldb_dr5_x86_64:
   case lldb_dr6_x86_64:
   case lldb_dr7_x86_64:
     m_dbr.dr[reg - lldb_dr0_x86_64] = reg_value.GetAsUInt64();
     break;
   default:
     llvm_unreachable("Reading unknown/unsupported register");
   }
 
   if (set == XStateRegSet)
     m_xstate.xs_xstate_bv |= new_xstate_bv;
 
-  return WriteRegisterSet(set);
+  return llvm::Error::success();
 }
 
 Status NativeRegisterContextNetBSD_x86_64::ReadAllRegisterValues(
     lldb::DataBufferSP &data_sp) {
   Status error;
 
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
   error = ReadRegisterSet(GPRegSet);
int NativeRegisterContextNetBSD_x86_64::GetDR(int num) const {
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 }
 
 Status NativeRegisterContextNetBSD_x86_64::CopyHardwareWatchpointsFrom(
     NativeRegisterContextNetBSD &source) {
   auto &r_source = static_cast<NativeRegisterContextNetBSD_x86_64&>(source);
-  Status res = r_source.ReadRegisterSet(DBRegSet);
-  if (!res.Fail()) {
+  llvm::Error error = r_source.ReadRegisterSet(DBRegSet);
+  if (!error) {
     // copy dbregs only if any watchpoints were set
     if ((r_source.m_dbr.dr[7] & 0xFF) == 0)
-      return res;
+      return Status();
 
     m_dbr = r_source.m_dbr;
-    res = WriteRegisterSet(DBRegSet);
+    error = WriteRegisterSet(DBRegSet);
   }
-  return res;
+  return Status(std::move(error));
 }
 
 #endif // defined(__x86_64__)

lldb/source/Plugins/Process/Utility/CMakeLists.txt

 add_lldb_library(lldbPluginProcessUtility
   AuxVector.cpp
   DynamicRegisterInfo.cpp
   FreeBSDSignals.cpp
   GDBRemoteSignals.cpp
   HistoryThread.cpp
   HistoryUnwind.cpp
   InferiorCallPOSIX.cpp
   LinuxProcMaps.cpp
   LinuxSignals.cpp
   MipsLinuxSignals.cpp
+  NativeRegisterContextCache.cpp
   NativeRegisterContextRegisterInfo.cpp
   NativeRegisterContextWatchpoint_x86.cpp
   NetBSDSignals.cpp
   RegisterContextDarwin_arm.cpp
   RegisterContextDarwin_arm64.cpp
   RegisterContextDarwin_i386.cpp
   RegisterContextDarwin_x86_64.cpp
   RegisterContextDummy.cpp

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

+//===-- NativeRegisterContextCache.h ----------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef lldb_NativeRegisterContextCache_h
+#define lldb_NativeRegisterContextCache_h
+
+#include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h"
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Error.h"
+
+#include <cstdint>
+#include <map>
+
+namespace lldb_private {
+
+enum class RegSetCacheStatus {
+  Empty,
+  Read,
+  Written,
+};
+
+class NativeRegisterContextCache
+    : public virtual NativeRegisterContextRegisterInfo {
+public:
+  Status ReadRegister(const RegisterInfo *reg_info, RegisterValue &reg_value) override;
+  Status WriteRegister(const RegisterInfo *reg_info, const RegisterValue &reg_value) override;
+  llvm::Error FlushRegisterSets();
+
+protected:
+  virtual llvm::Optional<uint32_t> GetRegisterSetForRegNum(uint32_t reg_index) const = 0;
+
+  virtual llvm::Error ReadRegisterSet(uint32_t set) = 0;
+  virtual llvm::Error WriteRegisterSet(uint32_t set) = 0;
+
+  virtual llvm::Error GetRegisterFromCache(const RegisterInfo *reg_info, RegisterValue &reg_value, uint32_t set) = 0;
+  virtual llvm::Error SetRegisterInCache(const RegisterInfo *reg_info, const RegisterValue &reg_value, uint32_t set) = 0;
+
+  llvm::Error FlushRegisterSet(uint32_t set);
+
+private:
+  std::map<int, RegSetCacheStatus> m_regset_cache_status;
+};
+
+} // namespace lldb_private
+
+#endif // #ifndef lldb_NativeRegisterContextCache_h
+

lldb/source/Plugins/Process/Utility/NativeRegisterContextCache.cpp

+//===-- NativeRegisterContextCache.cpp ------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "NativeRegisterContextCache.h"
+
+using namespace lldb_private;
+
+Status
+NativeRegisterContextCache::ReadRegister(const RegisterInfo *reg_info,
+                                                RegisterValue &reg_value) {
+  Status error;
+
+  if (!reg_info) {
+    error.SetErrorString("reg_info NULL");
+    return error;
+  }
+
+  uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+  if (reg == LLDB_INVALID_REGNUM) {
+    // This is likely an internal register for lldb use only and should not be
+    // directly queried.
+    error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "
+                                   "register, cannot read directly",
+                                   reg_info->name);
+    return error;
+  }
+
+  llvm::Optional<uint32_t> maybe_set = GetRegisterSetForRegNum(reg);
+  if (!maybe_set) {
+    error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
+                                   reg_info->name);
+    return error;
+  }
+
+  uint32_t set = maybe_set.getValue();
+  switch (m_regset_cache_status[set]) {
+    case RegSetCacheStatus::Written: {
+      // if anything was written, flush it and reread
+      llvm::Error error2 = FlushRegisterSet(set);
+      if (error2)
+        return Status(std::move(error2));
+      assert(m_regset_cache_status[set] == RegSetCacheStatus::Empty);
+      LLVM_FALLTHROUGH;
+    }
+    case RegSetCacheStatus::Empty: {
+      llvm::Error error2 = ReadRegisterSet(set);
+      if (error2)
+        return Status(std::move(error2));
+      m_regset_cache_status[set] = RegSetCacheStatus::Read;
+      LLVM_FALLTHROUGH;
+    }
+    case RegSetCacheStatus::Read:
+      return Status(std::move(GetRegisterFromCache(reg_info, reg_value, set)));
+  }
+
+  llvm_unreachable("unknown register cache status");
+}
+
+Status NativeRegisterContextCache::WriteRegister(
+    const RegisterInfo *reg_info, const RegisterValue &reg_value) {
+
+  Status error;
+
+  if (!reg_info) {
+    error.SetErrorString("reg_info NULL");
+    return error;
+  }
+
+  uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+  if (reg == LLDB_INVALID_REGNUM) {
+    // This is likely an internal register for lldb use only and should not be
+    // directly queried.
+    error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "
+                                   "register, cannot read directly",
+                                   reg_info->name);
+    return error;
+  }
+
+  llvm::Optional<uint32_t> maybe_set = GetRegisterSetForRegNum(reg);
+  if (!maybe_set) {
+    error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
+                                   reg_info->name);
+    return error;
+  }
+
+  uint32_t set = maybe_set.getValue();
+  switch (m_regset_cache_status[set]) {
+    case RegSetCacheStatus::Empty: {
+      // Read the original data if not yet cached
+      llvm::Error error2 = ReadRegisterSet(set);
+      if (error2)
+        return Status(std::move(error2));
+      m_regset_cache_status[set] = RegSetCacheStatus::Read;
+      LLVM_FALLTHROUGH;
+    }
+    case RegSetCacheStatus::Read:
+    case RegSetCacheStatus::Written: {
+      llvm::Error error2 = SetRegisterInCache(reg_info, reg_value, set);
+      if (error2)
+        return Status(std::move(error2));
+      m_regset_cache_status[set] = RegSetCacheStatus::Written;
+      return Status();
+    }
+  }
+
+  llvm_unreachable("unknown register cache status");
+}
+
+llvm::Error NativeRegisterContextCache::FlushRegisterSets() {
+  for (auto kv : m_regset_cache_status) {
+    if (kv.second == RegSetCacheStatus::Empty)
+      continue;
+    llvm::Error error = FlushRegisterSet(kv.first);
+    if (error)
+      return error;
+  }
+
+  return llvm::Error::success();
+}
+
+llvm::Error NativeRegisterContextCache::FlushRegisterSet(uint32_t set) {
+  switch (m_regset_cache_status[set]) {
+    case RegSetCacheStatus::Written: {
+      llvm::Error error = WriteRegisterSet(set);
+      if (error)
+        return error;
+      LLVM_FALLTHROUGH;
+    }
+    case RegSetCacheStatus::Read:
+      m_regset_cache_status[set] = RegSetCacheStatus::Empty;
+      LLVM_FALLTHROUGH;
+    case RegSetCacheStatus::Empty:
+      return llvm::Error::success();
+  }
+
+  llvm_unreachable("unknown register cache status");
+}