mgorny added inline comments.

================
Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:525-526
   static const uint8_t g_s390x_opcode[] = {0x00, 0x01};
+  static const uint8_t g_ppc_opcode[] = {0x7f, 0xc0, 0x00, 0x08};
   static const uint8_t g_ppc64le_opcode[] = {0x08, 0x00, 0xe0, 0x7f}; // trap
 
----------------
jrtc27 wrote:
> krytarowski wrote:
> > jrtc27 wrote:
> > > Why are these two different? Should it not always be `trap` ie `tw 
> > > 31,0,0`? If not that should be explained here. These names also aren't 
> > > great as it's unclear which ppc64 is using unless you read the code below 
> > > (I'd expect either ppc and ppc64 or ppc and ppcle as the two "axes", but 
> > > ppc and ppc64le are on a diagonal in the 2x2 grid).
> > On PPC we assume Big-Endian unless specified otherwise, so no need to 
> > specify ppcbe or ppc64be.
> I realise why there is no `be` suffix, that's not what I was asking. 
> Currently there are four options (though ppcle isn't implemented in FreeBSD):
> |  | Big | Little |
> | 32 | ppc | ppcle |
> | 64 | ppc64 | ppc64le |
> 
> If the difference between the two encodings is solely endianness then they 
> should be called `g_ppc_opcode` and `g_ppcle_opcode`. If the difference 
> between the two encodings is solely machine word size then they should be 
> called `g_ppc_opcode` and g_ppc64_opcode`. But `g_ppc_opcode` vs 
> g_ppc64le_opcode` has *both* differences in the name, which tells you nothing 
> about *why* they are different, and thus does not obviously state which 
> encoding ppc64 uses, if either of them.
> 
> As for the encodings themselves, they obviously differ in endianness, but 
> there is also a difference in the second/third byte where `g_ppc_opcode[1]` 
> is `0xc0` but `g_ppc64le_opcode[2]` is `0xe0`. That does not make sense to 
> me, but if it's there for a reason it needs a comment.
I don't really know what's the difference between the `0xc0` and `0xe0` opcode. 
Both can be found in various places in LLDB sources. I've copied this one from 
`source/Target/Platform.cpp`). Maybe `0xc0` works on Big Endian ppc as well, 
I've copied `0xe0` because it was used by the relevant code before. I'll 
simplify the code as suggested and try `0xc0`.


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

https://reviews.llvm.org/D95947

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

Reply via email to