mstorsjo added a comment.

In D70840#1763639 <https://reviews.llvm.org/D70840#1763639>, @labath wrote:

> - I think we ought to have some kind of a utility function to hold the logic 
> for the `&~1` stuff. there is something like that in 
> Architecture::GetOpcodeLoadAddress. The Architecture class is mostly 
> independent of a target, so we may be able create one instance for each 
> module or symbol file, but that feels quite heavy. I might even go for 
> putting something like that into the ArchSpec class. The raison d'être of the 
> Architecture class was to evict some heavy code out of ArchSpec -- this was 
> ArchitectureArm::OverrideStopInfo. That one is definitely too heavy, so still 
> don't thing it belongs in ArchSpec, but the `&~1` thingy seems like it could 
> find a place there.)


I'm trying to dig into this now even if @clayborg hasn't commented on your 
suggestions.

This was moved out from the Target class in D48623 
<https://reviews.llvm.org/D48623>/rG7aa9e7bc5739f468143b7b97060a9fbbfb94c6d2 
<https://reviews.llvm.org/rG7aa9e7bc5739f468143b7b97060a9fbbfb94c6d2>. In 
addition to GetOpcodeLoadAddress and GetCallableLoadAddress, there's also 
GetBreakableLoadAddress in ArchitectureMips, which is a rather big piece of 
code as well, so I guess that can't be motivated to be moved, but as you write, 
the `&~1` bit in itself might be ok.

If we add a GetOpcodeLoadAddress in ArchSpec, which does `&~1` on mips and arm 
- should we try to call this from the Architecture plugins (we don't have an 
ArchSpec stored there right now), or just see it as tolerable and justifiable 
duplication? The existing GetOpcodeLoadAddress takes an AddressClass as well, 
should we keep that, or just make a plain GetOpcodeLoadAddress that assumes 
that the provided address is a code address?


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

https://reviews.llvm.org/D70840



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

Reply via email to