clayborg added a comment.

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

> I am strongly opposed to ArchSpec owing an Architecture object. The latter is 
> far more complicated -- it reads bytes from target memory and disassembles 
> them -- whereas ArchSpec just returns a bunch of constants. If anything, it 
> should be the other way around. That way the code which just fiddles with 
> triples does not need to depend on the whole world.


I know part of this re-org was to keep the ArchSpec class from accessing any 
Target stuff to keep each directory with source code from accessing stuff in 
another tree. So from that perspective I can see your objection. I don't think 
it adds anything at all to the code by splitting this up other than achieving 
this separation though. It makes sense to ask an architecture to do 
architecture specific things. No sure why they need to be split IMHO.

> Having an Architecture instance in the Module object (to ensure it's 
> independent of the target) seems /ok/, though I am not really convinced that 
> it is needed, as the &~1 logic seems like a good candidate for ArchSpec 
> (which already knows a lot about arm vs. thumb etc.

I would rather keep all architecture specific code in the Architecture plug-ins 
somehow. It keeps code modifications clearer as only on file changes that is 
architecture specific. If we inline stuff into ArchSpec we end up with a big 
switch statement for the core and every adds their code to the same function.

So not sure what the right solution is here. I still like folding the 
Architecture into ArchSpec myself, but I am open to any and all opinions.


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