jrtc27 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: > 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`. FWIW 0xc is `tw 30, 0, 0` whereas 0xe is `tw 31, 0, 0` i.e. the canonical unconditional trap instruction, which `trap` is an alias for. 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