[Lldb-commits] [PATCH] D95947: [lldb] [Process/FreeBSDRemote] Introduce powerpc support

[Michał Górny via Phabricator via lldb-commits]

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
 
----------------
mgorny wrote:
> 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`.
Sorry, I got the two exchanged. I meant I'm going to try `0xe0`.


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

https://reviews.llvm.org/D95947