[Lldb-commits] [PATCH] D40133: elf-core: Convert remaining register context to use register set maps

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: source/Plugins/Process/elf-core/elf-core-enums.h:58
 
+enum class CoreRegset : uint8_t { GPR, FPR, PPC_VMX, PPC_VSX };
+
----------------
labath wrote:
> clayborg wrote:
> > Seems weird to have PPC_VMX and PPC_VSX define in a CoreRegSet? Do these need to be specific for each arch? Why is everyone trying to use these? 
> > 
> > 
> This enum represents all possible register sets that we can find in the elf core files. Not all register contexts are expected to use all of them -- each register context just plucks out those that he knows about (if they were present in the core file).
> 
> The enum (and the corresponding map) are intended as a replacement for the list of member variable register sets in the ThreadData struct. `vregset` (and the previous ppc64 patch was about to add one more) is already quite architecture-specific.
> 
> While I do think that this is an improvement over the list-of-members solution, I am also not entirely happy with having these arch-specific sets in the enum.
> 
> The alternative I thought of just now is to have a a plain vector instead of a map for register set data. Then, at the generic level, we will just be dealing with "register set 0", "register set 1", etc.. and when we consult the  RegisterInfoInterface class, it will tell us that "register set 0" is in fact GPR, register set 2 is VMX, etc.
> 
> What do you think?
I like the vector idea a bit better and I like the idea of the RegisterInfoInterface telling us that "register set at index 0" is GPR, etc.


https://reviews.llvm.org/D40133