This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6f9d7b8fb2e: Cleanup and speedup 
NativeRegisterContextLinux_arm64 (authored by omjavaid).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D69371?vs=232501&id=232586#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69371

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp

Index: lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -241,6 +241,9 @@
   if (signo != LLDB_INVALID_SIGNAL_NUMBER)
     data = signo;
 
+  // Before thread resumes, clear any cached register data structures
+  GetRegisterContext().InvalidateAllRegisters();
+
   return NativeProcessLinux::PtraceWrapper(PTRACE_CONT, GetID(), nullptr,
                                            reinterpret_cast<void *>(data));
 }
@@ -262,6 +265,9 @@
   if (signo != LLDB_INVALID_SIGNAL_NUMBER)
     data = signo;
 
+  // Before thread resumes, clear any cached register data structures
+  GetRegisterContext().InvalidateAllRegisters();
+
   // If hardware single-stepping is not supported, we just do a continue. The
   // breakpoint on the next instruction has been setup in
   // NativeProcessLinux::Resume.
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -40,6 +40,8 @@
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
+  void InvalidateAllRegisters() override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;
@@ -77,11 +79,6 @@
   enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK };
 
 protected:
-  Status DoReadRegisterValue(uint32_t offset, const char *reg_name,
-                             uint32_t size, RegisterValue &value) override;
-
-  Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
-                              const RegisterValue &value) override;
 
   Status ReadGPR() override;
 
@@ -125,8 +122,17 @@
     uint32_t fpcr;
   };
 
-  uint64_t m_gpr_arm64[k_num_gpr_registers_arm64]; // 64-bit general purpose
-                                                   // registers.
+  struct GPR {
+    uint64_t x[31];
+    uint64_t sp;
+    uint64_t pc;
+    uint64_t pstate;
+  };
+
+  bool m_gpr_is_valid;
+  bool m_fpu_is_valid;
+
+  GPR m_gpr_arm64; // 64-bit general purpose registers.
   RegInfo m_reg_info;
   FPU m_fpr; // floating-point registers including extended register sets.
 
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -152,6 +152,9 @@
   m_max_hwp_supported = 16;
   m_max_hbp_supported = 16;
   m_refresh_hwdebug_info = true;
+
+  m_gpr_is_valid = false;
+  m_fpu_is_valid = false;
 }
 
 uint32_t NativeRegisterContextLinux_arm64::GetRegisterSetCount() const {
@@ -185,41 +188,39 @@
 
   const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 
-  if (IsFPR(reg)) {
-    error = ReadFPR();
-    if (error.Fail())
-      return error;
-  } else {
-    uint32_t full_reg = reg;
-    bool is_subreg = reg_info->invalidate_regs &&
-                     (reg_info->invalidate_regs[0] != LLDB_INVALID_REGNUM);
+  if (reg == LLDB_INVALID_REGNUM)
+    return Status("no lldb regnum for %s", reg_info && reg_info->name
+                                               ? reg_info->name
+                                               : "<unknown register>");
+
+  uint8_t *src;
+  uint32_t offset;
 
-    if (is_subreg) {
-      // Read the full aligned 64-bit register.
-      full_reg = reg_info->invalidate_regs[0];
+  if (IsGPR(reg)) {
+    if (!m_gpr_is_valid) {
+      error = ReadGPR();
+      if (error.Fail())
+        return error;
     }
 
-    error = ReadRegisterRaw(full_reg, reg_value);
+    offset = reg_info->byte_offset;
+    assert(offset < GetGPRSize());
+    src = (uint8_t *)GetGPRBuffer() + offset;
 
-    if (error.Success()) {
-      // If our read was not aligned (for ah,bh,ch,dh), shift our returned
-      // value one byte to the right.
-      if (is_subreg && (reg_info->byte_offset & 0x1))
-        reg_value.SetUInt64(reg_value.GetAsUInt64() >> 8);
+  } else if (IsFPR(reg)) {
+    if (!m_fpu_is_valid) {
 
-      // If our return byte size was greater than the return value reg size,
-      // then use the type specified by reg_info rather than the uint64_t
-      // default
-      if (reg_value.GetByteSize() > reg_info->byte_size)
-        reg_value.SetType(reg_info);
+      error = ReadFPR();
+      if (error.Fail())
+        return error;
     }
-    return error;
-  }
+    offset = CalculateFprOffset(reg_info);
+    assert(offset < GetFPRSize());
+    src = (uint8_t *)GetFPRBuffer() + offset;
+  } else
+    return Status("failed - register wasn't recognized to be a GPR or an FPR, "
+                  "write strategy unknown");
 
-  // Get pointer to m_fpr variable and set the data from it.
-  uint32_t fpr_offset = CalculateFprOffset(reg_info);
-  assert(fpr_offset < sizeof m_fpr);
-  uint8_t *src = (uint8_t *)&m_fpr + fpr_offset;
   reg_value.SetFromMemoryData(reg_info, src, reg_info->byte_size,
                               eByteOrderLittle, error);
 
@@ -228,48 +229,51 @@
 
 Status NativeRegisterContextLinux_arm64::WriteRegister(
     const RegisterInfo *reg_info, const RegisterValue &reg_value) {
+  Status error;
+
   if (!reg_info)
     return Status("reg_info NULL");
 
-  const uint32_t reg_index = reg_info->kinds[lldb::eRegisterKindLLDB];
-  if (reg_index == LLDB_INVALID_REGNUM)
+  const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+
+  if (reg == LLDB_INVALID_REGNUM)
     return Status("no lldb regnum for %s", reg_info && reg_info->name
                                                ? reg_info->name
                                                : "<unknown register>");
 
-  if (IsGPR(reg_index))
-    return WriteRegisterRaw(reg_index, reg_value);
-
-  if (IsFPR(reg_index)) {
-    // Get pointer to m_fpr variable and set the data to it.
-    uint32_t fpr_offset = CalculateFprOffset(reg_info);
-    assert(fpr_offset < sizeof m_fpr);
-    uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
-    switch (reg_info->byte_size) {
-    case 2:
-      *(uint16_t *)dst = reg_value.GetAsUInt16();
-      break;
-    case 4:
-      *(uint32_t *)dst = reg_value.GetAsUInt32();
-      break;
-    case 8:
-      *(uint64_t *)dst = reg_value.GetAsUInt64();
-      break;
-    default:
-      assert(false && "Unhandled data size.");
-      return Status("unhandled register data size %" PRIu32,
-                    reg_info->byte_size);
+  uint8_t *dst;
+  uint32_t offset;
+
+  if (IsGPR(reg)) {
+    if (!m_gpr_is_valid) {
+      error = ReadGPR();
+      if (error.Fail())
+        return error;
     }
 
-    Status error = WriteFPR();
-    if (error.Fail())
-      return error;
+    offset = reg_info->byte_offset;
+    assert(offset < GetGPRSize());
+    dst = (uint8_t *)GetGPRBuffer() + offset;
 
-    return Status();
+    ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+    return WriteGPR();
+  } else if (IsFPR(reg)) {
+    if (!m_fpu_is_valid) {
+      error = ReadFPR();
+      if (error.Fail())
+        return error;
+    }
+    offset = CalculateFprOffset(reg_info);
+    assert(offset < GetFPRSize());
+    dst = (uint8_t *)GetFPRBuffer() + offset;
+
+    ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+    return WriteFPR();
   }
 
-  return Status("failed - register wasn't recognized to be a GPR or an FPR, "
-                "write strategy unknown");
+  return error;
 }
 
 Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
@@ -277,18 +281,21 @@
   Status error;
 
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
-  error = ReadGPR();
-  if (error.Fail())
-    return error;
-
-  error = ReadFPR();
-  if (error.Fail())
-    return error;
+  if (!m_gpr_is_valid) {
+    error = ReadGPR();
+    if (error.Fail())
+      return error;
+  }
 
+  if (!m_fpu_is_valid) {
+    error = ReadFPR();
+    if (error.Fail())
+      return error;
+  }
   uint8_t *dst = data_sp->GetBytes();
-  ::memcpy(dst, &m_gpr_arm64, GetGPRSize());
+  ::memcpy(dst, GetGPRBuffer(), GetGPRSize());
   dst += GetGPRSize();
-  ::memcpy(dst, &m_fpr, sizeof(m_fpr));
+  ::memcpy(dst, GetFPRBuffer(), GetFPRSize());
 
   return error;
 }
@@ -320,14 +327,14 @@
                                    __FUNCTION__);
     return error;
   }
-  ::memcpy(&m_gpr_arm64, src, GetRegisterInfoInterface().GetGPRSize());
+  ::memcpy(GetGPRBuffer(), src, GetRegisterInfoInterface().GetGPRSize());
 
   error = WriteGPR();
   if (error.Fail())
     return error;
 
   src += GetRegisterInfoInterface().GetGPRSize();
-  ::memcpy(&m_fpr, src, sizeof(m_fpr));
+  ::memcpy(GetFPRBuffer(), src, GetFPRSize());
 
   error = WriteFPR();
   if (error.Fail())
@@ -832,117 +839,65 @@
                                            &hwbType, &ioVec, ioVec.iov_len);
 }
 
-Status NativeRegisterContextLinux_arm64::DoReadRegisterValue(
-    uint32_t offset, const char *reg_name, uint32_t size,
-    RegisterValue &value) {
-  Status error;
-  if (offset > sizeof(struct user_pt_regs)) {
-    offset -= sizeof(struct user_pt_regs);
-    if (offset > sizeof(struct user_fpsimd_state)) {
-      error.SetErrorString("invalid offset value");
-      return error;
-    }
-    elf_fpregset_t regs;
-    int regset = NT_FPREGSET;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(
-        PTRACE_GETREGSET, m_thread.GetID(), &regset, &ioVec, sizeof regs);
-    if (error.Success()) {
-      value.SetBytes((void *)(((unsigned char *)(&regs)) + offset), 16,
-                     m_thread.GetProcess().GetByteOrder());
-    }
-  } else {
-    elf_gregset_t regs;
-    int regset = NT_PRSTATUS;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(
-        PTRACE_GETREGSET, m_thread.GetID(), &regset, &ioVec, sizeof regs);
-    if (error.Success()) {
-      value.SetBytes((void *)(((unsigned char *)(regs)) + offset), 8,
-                     m_thread.GetProcess().GetByteOrder());
-    }
-  }
-  return error;
-}
-
-Status NativeRegisterContextLinux_arm64::DoWriteRegisterValue(
-    uint32_t offset, const char *reg_name, const RegisterValue &value) {
+Status NativeRegisterContextLinux_arm64::ReadGPR() {
   Status error;
-  ::pid_t tid = m_thread.GetID();
-  if (offset > sizeof(struct user_pt_regs)) {
-    offset -= sizeof(struct user_pt_regs);
-    if (offset > sizeof(struct user_fpsimd_state)) {
-      error.SetErrorString("invalid offset value");
-      return error;
-    }
-    elf_fpregset_t regs;
-    int regset = NT_FPREGSET;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, &regset,
-                                              &ioVec, sizeof regs);
-
-    if (error.Success()) {
-      ::memcpy((void *)(((unsigned char *)(&regs)) + offset), value.GetBytes(),
-               16);
-      error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, tid, &regset,
-                                                &ioVec, sizeof regs);
-    }
-  } else {
-    elf_gregset_t regs;
-    int regset = NT_PRSTATUS;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, &regset,
-                                              &ioVec, sizeof regs);
-    if (error.Success()) {
-      ::memcpy((void *)(((unsigned char *)(&regs)) + offset), value.GetBytes(),
-               8);
-      error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, tid, &regset,
-                                                &ioVec, sizeof regs);
-    }
-  }
-  return error;
-}
 
-Status NativeRegisterContextLinux_arm64::ReadGPR() {
   struct iovec ioVec;
+
   ioVec.iov_base = GetGPRBuffer();
   ioVec.iov_len = GetGPRSize();
-  return ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
+
+  error = ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
+
+  if (error.Success())
+    m_gpr_is_valid = true;
+
+  return error;
 }
 
 Status NativeRegisterContextLinux_arm64::WriteGPR() {
   struct iovec ioVec;
+
+  m_gpr_is_valid = false;
+
   ioVec.iov_base = GetGPRBuffer();
   ioVec.iov_len = GetGPRSize();
+
   return WriteRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
 }
 
 Status NativeRegisterContextLinux_arm64::ReadFPR() {
+  Status error;
+
   struct iovec ioVec;
+
   ioVec.iov_base = GetFPRBuffer();
   ioVec.iov_len = GetFPRSize();
-  return ReadRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
+
+  error = ReadRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
+
+  if (error.Success())
+    m_fpu_is_valid = true;
+
+  return error;
 }
 
 Status NativeRegisterContextLinux_arm64::WriteFPR() {
   struct iovec ioVec;
+
+  m_fpu_is_valid = false;
+
   ioVec.iov_base = GetFPRBuffer();
   ioVec.iov_len = GetFPRSize();
+
   return WriteRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
 }
 
+void NativeRegisterContextLinux_arm64::InvalidateAllRegisters() {
+  m_gpr_is_valid = false;
+  m_fpu_is_valid = false;
+}
+
 uint32_t NativeRegisterContextLinux_arm64::CalculateFprOffset(
     const RegisterInfo *reg_info) const {
   return reg_info->byte_offset -
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
@@ -28,6 +28,9 @@
   CreateHostNativeRegisterContextLinux(const ArchSpec &target_arch,
                                        NativeThreadProtocol &native_thread);
 
+  // Invalidates cached values in register context data structures
+  virtual void InvalidateAllRegisters(){}
+
 protected:
   lldb::ByteOrder GetByteOrder() const;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to