[Lldb-commits] [PATCH] First cut of PowerPC(64) support in LLDB.

[Justin Hibbits]

There's still lots of work left to do on this, which I'll work on for future patches.  I'll fix the immediate problems, though, in a patch I'll submit tonight or tomorrow.

REPOSITORY
  rL LLVM

================
Comment at: source/Core/ArchSpec.cpp:22
@@ -21,2 +21,3 @@
 #include "lldb/Utility/SafeMachO.h"
+#include "lldb/Core/Log.h"
 #include "lldb/Core/RegularExpression.h"
----------------
emaste wrote:
> Leftover from earlier debugging?
> 
Yup.  Thought I had removed all the debug stuff. Guess I missed some

================
Comment at: source/Host/freebsd/Host.cpp:304
@@ +303,3 @@
+
+    ps_strings_addr = (void *)0xffffdff0;
+    pid.piod_op = PIOD_READ_D;
----------------
emaste wrote:
> Do we have a source for the magic constant?
This constant is probably only good for powerpc.  It's derived from SHARED_PAGE - sizeof(struct freebsd32_ps_strings).  It's probably better to add a sysctl to the kernel to get the information of any given process.  What do you think?

================
Comment at: source/Host/freebsd/Host.cpp:312
@@ +311,3 @@
+        buf_ap.release();
+        goto done;
+    }
----------------
emaste wrote:
> can we just return buf_sp here instead?
This is a direct copy of GetAuxData() below.  So they can both be updated the same.  I'm not sure the rationale of GetAuxData() doing it, so figured the best solution for now is to just copy it.

================
Comment at: source/Plugins/ABI/CMakeLists.txt:2-3
@@ -1,2 +1,4 @@
 add_subdirectory(SysV-hexagon)
+add_subdirectory(SysV-ppc64)
+add_subdirectory(SysV-ppc)
 add_subdirectory(SysV-x86_64)
----------------
emaste wrote:
> ppc should sort before ppc64
Oops.  Fixing in a new patch, with the others.

================
Comment at: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp:264
@@ +263,3 @@
+
+    if (args.size() > 8) // TODO handle more than 8 arguments
+        return false;
----------------
emaste wrote:
> Should we have a log for this?
Log or TODO/FIXME, yes.  I'll add something.

================
Comment at: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp:634
@@ +633,3 @@
+        {
+            if (type_flags & eTypeIsComplex)
+            {
----------------
emaste wrote:
> Debug log or error return for these cases?
Shamelessly copied from SysV-x86_64.  The else{} block should be logged, though.  Will add that.

================
Comment at: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp:1032
@@ +1031,3 @@
+        const char *name = reg_info->name;
+        if (name[0] == 'r')
+        {
----------------
emaste wrote:
> you should be able to use a register ID in kinds here rather than comparing strings, I'd think
That's a better way, yes.  I did it this way because ABISysV-x86_64 does this.  I'll fix it in a future patch.  Still lots of work left before this is really complete.

================
Comment at: source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp:331
@@ +330,3 @@
+	case llvm::Triple::ppc64:
+		{
+			static const uint8_t g_ppc_opcode[] = { 0x7f, 0xe0, 0x00, 0x08 };
----------------
emaste wrote:
> Maybe a phabricator issue, but whitespace seems off here - tabs vs. spaces?
Yup, this was one of the first changes I had made, and used tabs.  Fixed in next patch.

================
Comment at: source/Plugins/Process/elf-core/RegisterContextPOSIXCore_powerpc.cpp:51
@@ +50,3 @@
+{
+    assert(0);
+    return false;
----------------
emaste wrote:
> llvm_unreachable("powerpc::WriteFPR unimplemented"); perhaps?
Good idea.  I'll include it in a future patch, for both GPR and FPR.

================
Comment at: source/Target/Thread.cpp:2284-2285
@@ -2283,2 +2283,4 @@
             case llvm::Triple::mips64:
+            case llvm::Triple::ppc64:
+            case llvm::Triple::ppc:
             case llvm::Triple::hexagon:
----------------
emaste wrote:
> ppc should sort first
Oops, next patch.

================
Comment at: source/lldb.cpp:232
@@ -227,2 +231,3 @@
     ABISysV_x86_64::Terminate();
+    ABISysV_ppc64::Terminate();
     DisassemblerLLVMC::Terminate();
----------------
emaste wrote:
> ppc and ppc64 ::Initialize but only pp64 ::Terminate
Fixing in a new patch.

http://reviews.llvm.org/D5988