clayborg added a comment. In https://reviews.llvm.org/D31172#808105, @labath wrote:
> In https://reviews.llvm.org/D31172#808038, @clayborg wrote: > > > Added a suggestion to make Target::GetArchitecturePlugin lazy and to ask if > > the arch is supported. Most of the time we will set this once and never > > need to change it, even when we attach, change arch, etc. The new > > suggestion will allow us to use the same arch plug-in without re-fetching > > it. Let me know what you think as the changes are thrown out there to see > > what people's opinions are. > > > Well... I don't feel strongly about it, but it's not my preferred solution > (the IsSupported() call feels hacky). But since we're already throwing ideas > out there, here are two of mine: > > - confine all the changes to m_arch to the SetArchitecture method. Right now > only one of the assignments to m_arch is done outside of this method, and > that one can be easily converted to a call to SetArchitecture(). This should > make sure the two values never get out of sync. That would be fine too. > - (a more extreme version of the first idea) store the ArchSpec inside the > architecture plugin (then use m_architecture_up->GetArchSpec() instead of > m_arch). This will make it impossible for the values to go out of sync, as we > will have a single source of truth. (We will need a default arch plugin for > cases where we don't have a specific plugin) It would need to be the other way around. Store the Architecture plug-in inside of the ArchSpec since we only have the "arm" Architecture plug-in and architectures don't need to make one of these unless the arch has special things it needs to do. I am fine with either approach and fine not doing that approach I suggested. I just don't want this to ever get out of sync. So maybe the easiest way it to ask the ArchSpec object for it's architecture plug-in? If there is no state maintained in these plug-ins, we might just have a single instance of each Architecture plug-in that everyone uses (across targets)? Then everyone doesn't need to hold onto a shared/unique_ptr and can just fetch it from the Plugin manager each time? https://reviews.llvm.org/D31172 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
