[libc-commits] [PATCH] D137147: [libc] Add x86_64 implementation of setjmp and longjmp.

[Michael Jones via Phabricator via libc-commits]

michaelrj added a comment.

overall LGTM with a few nits



================
Comment at: libc/include/llvm-libc-types/jmp_buf.h:23
+#else
+#error "__jmp_buf not available for you target architecture."
+#endif
----------------
nit: "your"


================
Comment at: libc/src/setjmp/longjmp.cpp:17
+
+LLVM_LIBC_FUNCTION(void, longjmp, (__jmp_buf * buf, int val)) {
+#ifdef LLVM_LIBC_ARCH_X86_64
----------------
this function should be marked `noreturn`


================
Comment at: libc/src/setjmp/longjmp.cpp:28
+
+  // ABI requires that the return value should be stord in rax. So, we store
+  // |val| in rax. Note that this has to happen before we restore the registers
----------------
nit: "stored"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137147