DavidSpickett added a comment.

> I think the main reason that the RegisterInfo struct is such as it is, is 
> because it wants to be trivially constructible (or whatever the c++ term for 
> that is). I'm pretty sure that adding a std::string member will make it stop 
> being that, and I don't think we suddenly want to run global constructors for 
> all of the global RegisterInfo instances we have (and we have a lot of them).
Agreed. I think I stumbled into a solution for this for different reasons.

I might have mentioned adding a std::string somewhere in this review but I only 
did that as a way to test this patch actually would work.

> If you wanted to preserve the existing patterns, the right way to store a 
> string/array member inside a static struct would be to copy what is being 
> done in the value_regs/invalidate_regs field, i.e., store the value itself in 
> another static object, and then store the pointer to that object into 
> RegisterInfo. > For dynamic RegisterInfos (like those received from the 
> lldb-server), you'd use DynamicRegisterInfo and 
> DynamicRegisterInfo::Register, which have logic to persist/create storage for 
> these values (that logic would have to be extended to handle the new fields 
> as well).

Agreed.

> That said I would *love* is someone changed the RegisterInfo structs into 
> something saner, but I think that will need to be more elaborate than simply 
> stuffing a std::vector member into it. I can give you my idea of how this 
> could work, if you're interested in trying this out.

Sure I'd be interested in that. I've just been hacking on this small part of it 
so I don't have the full picture yet.

> So I might recommend adding a RegisterInfo member that is a "Fields *fields" 
> that can be easily set to NULL in POD initializers, but then stored in the 
> dynamic register info class in its own vector and then have the RegisterInfo 
> struct point to another POD definition of the Fields.

I though along the same lines but a different reason, I didn't want to store 
empty <register flags type> in every register info that didn't have flags and 
balloon the total size.

So I have:

  struct RegisterInfo {
  <...>
    /// If not nullptr, a custom type defined by XML descriptions.
    /// Using shared_ptr here means that the majority of registers that don't 
have
    /// custom types do not have to waste sizeof(RegisterFlags) bytes.
    std::shared_ptr<RegisterFlags> flags_type = nullptr;

But shared_ptr still has a destructor. It could be `RegisterFlags*`, I just 
have to work out when all that data needs to be dealocated. Probably when the 
target itself destructs but I need to figure that out, in general i don't know 
yet where the best place for these flags/fields to live is.

If it is a plain pointer then this patch is redundant but hey I learned how to 
use placement new :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134041

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to