[PATCH] D137010: [libunwind][LoongArch] Add 64-bit LoongArch support

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added inline comments.


================
Comment at: libunwind/include/__libunwind_config.h:172
+#if __loongarch_grlen == 64
+#define _LIBUNWIND_TARGET_LOONGARCH 1
+#define _LIBUNWIND_CONTEXT_SIZE 65
----------------
This definition could come before the GRLen switch, just after `__loongarch__` is matched? Because both GRLen=64 and the future GRLen=32 cases are LoongArch.


================
Comment at: libunwind/include/__libunwind_config.h:199
 # define _LIBUNWIND_TARGET_S390X 1
+#define _LIBUNWIND_TARGET_LOONGARCH 1
 # define _LIBUNWIND_CONTEXT_SIZE 167
----------------
One space before `define` for consistency with neighboring code.


================
Comment at: libunwind/src/Registers.hpp:5132
+  case UNW_REG_IP:
+    return "$pc";
+  case UNW_REG_SP:
----------------
Do we really want the `$` prefix? It's not for emitting assembly after all, and no architecture except MIPS does so. The RISCV port even features ABI names in their `getRegisterName` and I think it makes sense too, because it's actually clear we're dealing with a LP64* ABI here.


================
Comment at: libunwind/src/UnwindRegistersRestore.S:1303
+# if __loongarch_frlen == 64
+  fld.d    $f0, $r4, (8 * 33 + 8 * 0)
+  fld.d    $f1, $r4, (8 * 33 + 8 * 1)
----------------
Those `$r4` (at this position) could be just `$a0` for clarity.


================
Comment at: libunwind/src/UnwindRegistersRestore.S:1369
+  ld.d    $r31, $r4, (8 * 31)
+  ld.d    $r4,  $r4, (8 * 4)   // restore $a0
+  ld.d    $r1,  $r4, (8 * 32)  // load new pc into $ra
----------------
nit: `restore $a0 last` for a nice short explanation of its placement.


================
Comment at: libunwind/src/UnwindRegistersRestore.S:1372
+
+  jr      $r1                  // jump to $ra
+
----------------
`jr $ra` is enough. (We might even use `ret` if we don't care about binutils 2.39.) And the comment provides no new information than the code so it's better removed.


================
Comment at: libunwind/src/UnwindRegistersSave.S:1302
+
+  or       $r4, $r0, $r0  // return UNW_ESUCCESS
+  jr       $r1            // jump to $ra
----------------
`move $a0, $zero // UNW_ESUCCESS`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137010