[Lldb-commits] [PATCH] D90034: [lldb] [Process/NetBSD] Use XStateRegSet for all FPU registers

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:994-995
         !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) {
-      error.SetErrorStringWithFormat("register \"%s\" not supported by CPU/kernel",
-                                     reg_info->name);
+      error.SetErrorStringWithFormat(
+          "register \"%s\" not supported by CPU/kernel", reg_info->name);
     } else {
----------------
labath wrote:
> mgorny wrote:
> > labath wrote:
> > > Is this code actually reachable? Ideally, I'd expect this to be checked immediately after program startup, so that we can avoid even reporting the existence of [xy]mm registers..
> > Yes, it is. I've just run it inside qemu with an emulated non-AVX CPU, and the code definitely gets run. The user-visible result is apparently that the registers are reported as unavailable.
> > 
> > I suppose we could use sysctl (or cpuid) to check for supported instruction sets in constructor but I don't really know how to make LLDB consider them non-existing.
> > 
> > In any case, I think that's material for a separate patch.
> I think you can make them non-existent by returning a smaller number via `GetUserRegisterCount`. Linux code has something like that, though I'm not sure that it is entirely correct.
> 
> Yeah, this is for a separate patch, though it has some implications for the caching code. The current linux code works by assuming that each "register set" is either entirely present or entirely absent (a reasonable assumption I'd say). That might make things tricky of one wants to have a single register set for reading/caching all of xstate data.
> 
> I'm wondering if register sets (as presented to the user) and register sets for the purposes of caching (maybe these should have a different name) shouldn't be distinct concepts...
I've been thinking about having two levels of 'register sets' but I've came to the conclusion that the benefit is not worth the added complexity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90034