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