[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.


================
Comment at: include/__libunwind_config.h:59-68
+# elif defined(__mips__) && defined(_ABIN32) && defined(__mips_soft_float)
+#  define _LIBUNWIND_TARGET_MIPS_N64 1
+#  define _LIBUNWIND_CONTEXT_SIZE 35
+#  define _LIBUNWIND_CURSOR_SIZE 46
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 66
 # elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
 #  define _LIBUNWIND_TARGET_MIPS_N64 1
----------------
Change the define of _LIBUNWIND_TARGET_MIPS_N64 to _LIBUNWIND_TARGET_MIPS_NEWABI, then these two elif branches can have the condition (defined(_ABIN32) && defined(_ABI64) and refactored into one elif branch.


================
Comment at: include/__libunwind_config.h:62
+#  define _LIBUNWIND_CONTEXT_SIZE 35
+#  define _LIBUNWIND_CURSOR_SIZE 46
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 66
----------------
Shouldn't this 46 be 47?


================
Comment at: src/AddressSpace.hpp:201
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if defined(__LP64__) || defined(__mips_n32)
+  return get64(addr);
----------------
   defined(__mips_n32) -> (defined(__mips__) && defined(_ABIN32))


================
Comment at: src/UnwindRegistersRestore.S:548
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) && defined(__mips_soft_float)
 
----------------
This line is overly long, break with '\' after the second &&.


================
Comment at: src/UnwindRegistersSave.S:146
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) && defined(__mips_soft_float)
 
----------------
This line looks overly long, break before the second &&.


================
Comment at: src/libunwind.cpp:63-66
+#elif defined(__mips__) && defined(_ABIN32) && defined(__mips_soft_float)
+# define REGISTER_KIND Registers_mips_n64
 #elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
 # define REGISTER_KIND Registers_mips_n64
----------------
Fold these two branches together and have the condition (defined(_ABIN32) || defined(_ABI64), then follow-up by renaming Registers_mips_n64 to Register_mips_NEWABI. Add a comment to the definition of that class stating that it covers both n32 and n64.


https://reviews.llvm.org/D39074