[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis edited subscribers, added: cfe-commits; removed: llvm-commits.
sdardis added a comment.

+CC cfe-commits, -CC llvm-commits.

Some comments inlined.

I've managed to run the test-suite on one of my machines here and I'm seeing 3 failures:

  libunwind :: libunwind_01.pass.cpp
  libunwind :: libunwind_02.pass.cpp
  libunwind :: unw_getcontext.pass.cpp

This is with a GCC toolchain, 6.3. Did you test with clang?



================
Comment at: include/__libunwind_config.h:54
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 32
+# elif defined(__mips_o32)
+#  define _LIBUNWIND_TARGET_MIPS_O32 1
----------------
Can you avoid using the __mips_o32 macro and instead use defined(__mips__) && _MIPS_SIM == _ABIO32 ?

It appears the __mips_o32 is a FreeBSD specific macro. Also, test for __mips_soft_float .


================
Comment at: include/__libunwind_config.h:59
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 32
+# elif defined(__mips_n64)
+#  define _LIBUNWIND_TARGET_MIPS_N64 1
----------------
Likewise "defined(__mips__) && _MIPS_SIM == _ABI64 && __mips_soft_float"


================
Comment at: src/UnwindRegistersRestore.S:492
 
+#elif defined(__mips_o32)
+
----------------
defined(__mips_o32) -> defined(__mips__) && _MIPS_SIM == _ABIO32


================
Comment at: src/UnwindRegistersRestore.S:500
+//
+  .set noreorder
+  .set noat
----------------
Prefix this with:

   .set push
   .set nomacro

Also, the assembler directives should come after the function declaration / defines.


================
Comment at: src/UnwindRegistersRestore.S:539
+  lw    $4, (4 * 4)($4)
+
+#elif defined(__mips_n64)
----------------
   .set pop


================
Comment at: src/UnwindRegistersRestore.S:540
+
+#elif defined(__mips_n64)
+
----------------
defined(__mips__) && _MIPS_SIM == _ABI64


================
Comment at: src/UnwindRegistersRestore.S:550
+  .set noat
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind18Registers_mips_n646jumptoEv)
+  // r0 is zero
----------------
See my comments on the o32 assembly.


================
Comment at: src/UnwindRegistersSave.S:90
 
+#elif defined(__mips_o32)
+
----------------
See my previous comments about this preprocessor macro.


================
Comment at: src/UnwindRegistersSave.S:101
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set noreorder
+  # s0 - s7
----------------
Wrap this with

   .set push
   .set noat
   .set nomacro


================
Comment at: src/UnwindRegistersSave.S:120
+  sw    $30, (4 * 30)($4)
+
+#elif defined(__mips_n64)
----------------
and:
   .set pop


================
Comment at: src/UnwindRegistersSave.S:121
+
+#elif defined(__mips_n64)
+
----------------
Again, see my previous comments on this preprocessor define.


================
Comment at: src/UnwindRegistersSave.S:132
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set noreorder
+  # s0 - s7
----------------
Wrap this with

   .set push
   .set noat
   .set nomacro


================
Comment at: src/UnwindRegistersSave.S:151
+  sd    $30, (8 * 30)($4)
+
 # elif defined(__mips__)
----------------
   .set pop


================
Comment at: src/libunwind.cpp:66
 #elif defined(__mips__)
 # warning The MIPS architecture is not supported.
 #else
----------------
The MIPS architecture is not supported with this ABI and environment!


https://reviews.llvm.org/D38110