[PATCH] D145584: [libc] Add support for setjmp and longjmp in riscv

[Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f6fd1b704c0: [libc] Add support for setjmp and longjmp in 
riscv (authored by Mikhail R. Gadelha ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145584

Files:
  clang/docs/tools/clang-formatted-files.txt
  libc/config/linux/riscv64/entrypoints.txt
  libc/config/linux/riscv64/headers.txt
  libc/include/llvm-libc-types/jmp_buf.h
  libc/src/setjmp/CMakeLists.txt
  libc/src/setjmp/longjmp.cpp
  libc/src/setjmp/riscv64/CMakeLists.txt
  libc/src/setjmp/riscv64/longjmp.cpp
  libc/src/setjmp/riscv64/setjmp.cpp
  libc/src/setjmp/setjmp.cpp
  libc/src/setjmp/x86_64/CMakeLists.txt
  libc/src/setjmp/x86_64/longjmp.cpp
  libc/src/setjmp/x86_64/setjmp.cpp

Index: libc/src/setjmp/x86_64/setjmp.cpp
===
--- libc/src/setjmp/x86_64/setjmp.cpp
+++ libc/src/setjmp/x86_64/setjmp.cpp
@@ -7,15 +7,15 @@
 //===--===//
 
 #include "src/__support/common.h"
-#include "src/__support/macros/properties/architectures.h"
 #include "src/setjmp/setjmp_impl.h"
 
-#include 
+#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
+#error "Invalid file include"
+#endif
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
-#ifdef LIBC_TARGET_ARCH_IS_X86_64
   register __UINT64_TYPE__ rbx __asm__("rbx");
   register __UINT64_TYPE__ r12 __asm__("r12");
   register __UINT64_TYPE__ r13 __asm__("r13");
@@ -50,10 +50,6 @@
   buf->rsp = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_frame_address(0)) +
  sizeof(__UINTPTR_TYPE__) * 2;
   buf->rip = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_return_address(0));
-#else // LIBC_TARGET_ARCH_IS_X86_64
-#error "setjmp implementation not available for the target architecture."
-#endif
-
   return 0;
 }
 
Index: libc/src/setjmp/x86_64/longjmp.cpp
===
--- libc/src/setjmp/x86_64/longjmp.cpp
+++ libc/src/setjmp/x86_64/longjmp.cpp
@@ -8,14 +8,14 @@
 
 #include "src/setjmp/longjmp.h"
 #include "src/__support/common.h"
-#include "src/__support/macros/properties/architectures.h"
 
-#include 
+#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
+#error "Invalid file include"
+#endif
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(void, longjmp, (__jmp_buf * buf, int val)) {
-#ifdef LIBC_TARGET_ARCH_IS_X86_64
   register __UINT64_TYPE__ rbx __asm__("rbx");
   register __UINT64_TYPE__ rbp __asm__("rbp");
   register __UINT64_TYPE__ r12 __asm__("r12");
@@ -38,9 +38,6 @@
   LIBC_INLINE_ASM("mov %1, %0\n\t" : "=r"(r15) : "m"(buf->r15) :);
   LIBC_INLINE_ASM("mov %1, %0\n\t" : "=r"(rsp) : "m"(buf->rsp) :);
   LIBC_INLINE_ASM("jmp *%0\n\t" : : "m"(buf->rip));
-#else // LIBC_TARGET_ARCH_IS_X86_64
-#error "longjmp implementation not available for the target architecture."
-#endif
 }
 
 } // namespace __llvm_libc
Index: libc/src/setjmp/x86_64/CMakeLists.txt
===
--- libc/src/setjmp/x86_64/CMakeLists.txt
+++ libc/src/setjmp/x86_64/CMakeLists.txt
@@ -3,12 +3,12 @@
   SRCS
 setjmp.cpp
   HDRS
-setjmp_impl.h
-  COMPILE_OPTIONS
--O3 # We do not want any local variables in setjmp
--fno-omit-frame-pointer # The implementation assumes frame pointer on to the stack
+../setjmp_impl.h
   DEPENDS
 libc.include.setjmp
+  COMPILE_OPTIONS
+-O3
+-fno-omit-frame-pointer
 )
 
 add_entrypoint_object(
@@ -16,9 +16,10 @@
   SRCS
 longjmp.cpp
   HDRS
-longjmp.h
-  COMPILE_OPTIONS
--O3 # We do not want any local variables in longjmp
+../longjmp.h
   DEPENDS
 libc.include.setjmp
+  COMPILE_OPTIONS
+-O3
+-fno-omit-frame-pointer
 )
Index: libc/src/setjmp/riscv64/setjmp.cpp
===
--- /dev/null
+++ libc/src/setjmp/riscv64/setjmp.cpp
@@ -0,0 +1,56 @@
+//===-- Implementation of setjmp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "src/__support/common.h"
+#include "src/setjmp/setjmp_impl.h"
+
+#include 
+
+#if !defined(LIBC_TARGET_ARCH_IS_RISCV64)
+#error "Invalid file include"
+#endif
+
+namespace __llvm_libc {
+
+LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
+  LIBC_INLINE_ASM("sd ra, %0\n\t" : : "m"(buf->__pc) :);
+  LIBC_INLINE_ASM("sd s0, %0\n\t" : : "m"(buf->__regs[0]) :);
+  LIBC_INLINE_ASM("sd s1, %0\n\t" : : "m"(buf->__regs[1]) :);
+  LIBC_INLINE_ASM("sd s2, %0\n\t" : : "m"(buf->__regs[2]) :);
+ 

[PATCH] D145584: [libc] Add support for setjmp and longjmp in riscv

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho updated this revision to Diff 508187.
mikhail.ramalho added a comment.

Update return


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145584

Files:
  clang/docs/tools/clang-formatted-files.txt
  libc/config/linux/riscv64/entrypoints.txt
  libc/config/linux/riscv64/headers.txt
  libc/include/llvm-libc-types/jmp_buf.h
  libc/src/setjmp/CMakeLists.txt
  libc/src/setjmp/longjmp.cpp
  libc/src/setjmp/riscv64/CMakeLists.txt
  libc/src/setjmp/riscv64/longjmp.cpp
  libc/src/setjmp/riscv64/setjmp.cpp
  libc/src/setjmp/setjmp.cpp
  libc/src/setjmp/x86_64/CMakeLists.txt
  libc/src/setjmp/x86_64/longjmp.cpp
  libc/src/setjmp/x86_64/setjmp.cpp

Index: libc/src/setjmp/x86_64/setjmp.cpp
===
--- libc/src/setjmp/x86_64/setjmp.cpp
+++ libc/src/setjmp/x86_64/setjmp.cpp
@@ -7,15 +7,15 @@
 //===--===//
 
 #include "src/__support/common.h"
-#include "src/__support/macros/properties/architectures.h"
 #include "src/setjmp/setjmp_impl.h"
 
-#include 
+#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
+#error "Invalid file include"
+#endif
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
-#ifdef LIBC_TARGET_ARCH_IS_X86_64
   register __UINT64_TYPE__ rbx __asm__("rbx");
   register __UINT64_TYPE__ r12 __asm__("r12");
   register __UINT64_TYPE__ r13 __asm__("r13");
@@ -50,10 +50,6 @@
   buf->rsp = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_frame_address(0)) +
  sizeof(__UINTPTR_TYPE__) * 2;
   buf->rip = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_return_address(0));
-#else // LIBC_TARGET_ARCH_IS_X86_64
-#error "setjmp implementation not available for the target architecture."
-#endif
-
   return 0;
 }
 
Index: libc/src/setjmp/x86_64/longjmp.cpp
===
--- libc/src/setjmp/x86_64/longjmp.cpp
+++ libc/src/setjmp/x86_64/longjmp.cpp
@@ -8,14 +8,14 @@
 
 #include "src/setjmp/longjmp.h"
 #include "src/__support/common.h"
-#include "src/__support/macros/properties/architectures.h"
 
-#include 
+#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
+#error "Invalid file include"
+#endif
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(void, longjmp, (__jmp_buf * buf, int val)) {
-#ifdef LIBC_TARGET_ARCH_IS_X86_64
   register __UINT64_TYPE__ rbx __asm__("rbx");
   register __UINT64_TYPE__ rbp __asm__("rbp");
   register __UINT64_TYPE__ r12 __asm__("r12");
@@ -38,9 +38,6 @@
   LIBC_INLINE_ASM("mov %1, %0\n\t" : "=r"(r15) : "m"(buf->r15) :);
   LIBC_INLINE_ASM("mov %1, %0\n\t" : "=r"(rsp) : "m"(buf->rsp) :);
   LIBC_INLINE_ASM("jmp *%0\n\t" : : "m"(buf->rip));
-#else // LIBC_TARGET_ARCH_IS_X86_64
-#error "longjmp implementation not available for the target architecture."
-#endif
 }
 
 } // namespace __llvm_libc
Index: libc/src/setjmp/x86_64/CMakeLists.txt
===
--- libc/src/setjmp/x86_64/CMakeLists.txt
+++ libc/src/setjmp/x86_64/CMakeLists.txt
@@ -3,12 +3,12 @@
   SRCS
 setjmp.cpp
   HDRS
-setjmp_impl.h
-  COMPILE_OPTIONS
--O3 # We do not want any local variables in setjmp
--fno-omit-frame-pointer # The implementation assumes frame pointer on to the stack
+../setjmp_impl.h
   DEPENDS
 libc.include.setjmp
+  COMPILE_OPTIONS
+-O3
+-fno-omit-frame-pointer
 )
 
 add_entrypoint_object(
@@ -16,9 +16,10 @@
   SRCS
 longjmp.cpp
   HDRS
-longjmp.h
-  COMPILE_OPTIONS
--O3 # We do not want any local variables in longjmp
+../longjmp.h
   DEPENDS
 libc.include.setjmp
+  COMPILE_OPTIONS
+-O3
+-fno-omit-frame-pointer
 )
Index: libc/src/setjmp/riscv64/setjmp.cpp
===
--- /dev/null
+++ libc/src/setjmp/riscv64/setjmp.cpp
@@ -0,0 +1,56 @@
+//===-- Implementation of setjmp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "src/__support/common.h"
+#include "src/setjmp/setjmp_impl.h"
+
+#include 
+
+#if !defined(LIBC_TARGET_ARCH_IS_RISCV64)
+#error "Invalid file include"
+#endif
+
+namespace __llvm_libc {
+
+LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
+  LIBC_INLINE_ASM("sd ra, %0\n\t" : : "m"(buf->__pc) :);
+  LIBC_INLINE_ASM("sd s0, %0\n\t" : : "m"(buf->__regs[0]) :);
+  LIBC_INLINE_ASM("sd s1, %0\n\t" : : "m"(buf->__regs[1]) :);
+  LIBC_INLINE_ASM("sd s2, %0\n\t" : : "m"(buf->__regs[2]) :);
+  LIBC_INLINE_ASM("sd s3, %0\n\t" : : "m"(buf->__regs[3]) :);
+  LIBC_INLINE_ASM("sd s4, %0\n\t" : : "m"(buf->__regs[4]) :);
+  LIBC_INLINE_ASM("sd s5, %0\n\t" : : "m"(

[PATCH] D145584: [libc] Add support for setjmp and longjmp in riscv

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho added inline comments.



Comment at: libc/src/setjmp/riscv64/longjmp.cpp:54-55
+
+  LIBC_INLINE_ASM("seqz %0, %1" : "+r"(buf) : "r"(val) :);
+  LIBC_INLINE_ASM("add %0, %0, %1" : "+r"(buf) : "r"(val), "r"(buf) :);
+}

sivachandra wrote:
> Your comment is in the previous diff but thanks for the explanation. I think 
> we have missed the `val` check for zero in the x86_64 case and should be 
> fixed separately.
> 
> For the above two instructions, in the interest of reducing the amount of 
> logic in inline assembly, can we do:
> 
> ```
>   val = val == 0 ? 1 : val;
>   LIBC_INLINE_ASM("add a0, %0, zero\n\t" : : "r"(val) :);
> ```
> Your comment is in the previous diff but thanks for the explanation. I think 
> we have missed the `val` check for zero in the x86_64 case and should be 
> fixed separately.

no problem, do you want me to fix the x86_64 version?

> 
> For the above two instructions, in the interest of reducing the amount of 
> logic in inline assembly, can we do:
> 
> ```
>   val = val == 0 ? 1 : val;
>   LIBC_INLINE_ASM("add a0, %0, zero\n\t" : : "r"(val) :);
> ```

Done, I'll rerun the tests now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145584

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145584: [libc] Add support for setjmp and longjmp in riscv

[Siva Chandra via Phabricator via cfe-commits]

sivachandra accepted this revision.
sivachandra added inline comments.
This revision is now accepted and ready to land.



Comment at: libc/src/setjmp/riscv64/longjmp.cpp:54-55
+
+  LIBC_INLINE_ASM("seqz %0, %1" : "+r"(buf) : "r"(val) :);
+  LIBC_INLINE_ASM("add %0, %0, %1" : "+r"(buf) : "r"(val), "r"(buf) :);
+}

Your comment is in the previous diff but thanks for the explanation. I think we 
have missed the `val` check for zero in the x86_64 case and should be fixed 
separately.

For the above two instructions, in the interest of reducing the amount of logic 
in inline assembly, can we do:

```
  val = val == 0 ? 1 : val;
  LIBC_INLINE_ASM("add a0, %0, zero\n\t" : : "r"(val) :);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145584

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145584: [libc] Add support for setjmp and longjmp in riscv

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho added inline comments.



Comment at: libc/src/setjmp/riscv64/longjmp.h:56
+
+  LIBC_INLINE_ASM("seqz %0, %1" : "+r"(buf) : "r"(val) :);
+  LIBC_INLINE_ASM("add %0, %0, %1" : "+r"(buf) : "r"(val), "r"(buf) :);

sivachandra wrote:
> Why is this required? Can we just copy `val` to `a0` and return? Or, even 
> better would be just `return val;` if we can make this a `naked` function?
This is the fake return mentioned on the Linux man page:
```
Following  a  successful  longjmp(), execution continues as if setjmp()
had returned for a second time.  This  "fake"  return  can  be  distin-
guished from a true setjmp() call because the "fake" return returns the
value provided in val.  If the programmer mistakenly passes the value 0
in val, the "fake" return will instead return 1.
```

We can't return normally here, since `longjmp` has no return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145584

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145584: [libc] Add support for setjmp and longjmp in riscv

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho updated this revision to Diff 507700.
mikhail.ramalho marked 6 inline comments as done.
mikhail.ramalho added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145584

Files:
  clang/docs/tools/clang-formatted-files.txt
  libc/config/linux/riscv64/entrypoints.txt
  libc/config/linux/riscv64/headers.txt
  libc/include/llvm-libc-types/jmp_buf.h
  libc/src/setjmp/CMakeLists.txt
  libc/src/setjmp/longjmp.cpp
  libc/src/setjmp/riscv64/CMakeLists.txt
  libc/src/setjmp/riscv64/longjmp.cpp
  libc/src/setjmp/riscv64/setjmp.cpp
  libc/src/setjmp/setjmp.cpp
  libc/src/setjmp/x86_64/CMakeLists.txt
  libc/src/setjmp/x86_64/longjmp.cpp
  libc/src/setjmp/x86_64/setjmp.cpp

Index: libc/src/setjmp/x86_64/setjmp.cpp
===
--- libc/src/setjmp/x86_64/setjmp.cpp
+++ libc/src/setjmp/x86_64/setjmp.cpp
@@ -7,15 +7,15 @@
 //===--===//
 
 #include "src/__support/common.h"
-#include "src/__support/macros/properties/architectures.h"
 #include "src/setjmp/setjmp_impl.h"
 
-#include 
+#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
+#error "Invalid file include"
+#endif
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
-#ifdef LIBC_TARGET_ARCH_IS_X86_64
   register __UINT64_TYPE__ rbx __asm__("rbx");
   register __UINT64_TYPE__ r12 __asm__("r12");
   register __UINT64_TYPE__ r13 __asm__("r13");
@@ -50,10 +50,6 @@
   buf->rsp = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_frame_address(0)) +
  sizeof(__UINTPTR_TYPE__) * 2;
   buf->rip = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_return_address(0));
-#else // LIBC_TARGET_ARCH_IS_X86_64
-#error "setjmp implementation not available for the target architecture."
-#endif
-
   return 0;
 }
 
Index: libc/src/setjmp/x86_64/longjmp.cpp
===
--- libc/src/setjmp/x86_64/longjmp.cpp
+++ libc/src/setjmp/x86_64/longjmp.cpp
@@ -8,14 +8,14 @@
 
 #include "src/setjmp/longjmp.h"
 #include "src/__support/common.h"
-#include "src/__support/macros/properties/architectures.h"
 
-#include 
+#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
+#error "Invalid file include"
+#endif
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(void, longjmp, (__jmp_buf * buf, int val)) {
-#ifdef LIBC_TARGET_ARCH_IS_X86_64
   register __UINT64_TYPE__ rbx __asm__("rbx");
   register __UINT64_TYPE__ rbp __asm__("rbp");
   register __UINT64_TYPE__ r12 __asm__("r12");
@@ -38,9 +38,6 @@
   LIBC_INLINE_ASM("mov %1, %0\n\t" : "=r"(r15) : "m"(buf->r15) :);
   LIBC_INLINE_ASM("mov %1, %0\n\t" : "=r"(rsp) : "m"(buf->rsp) :);
   LIBC_INLINE_ASM("jmp *%0\n\t" : : "m"(buf->rip));
-#else // LIBC_TARGET_ARCH_IS_X86_64
-#error "longjmp implementation not available for the target architecture."
-#endif
 }
 
 } // namespace __llvm_libc
Index: libc/src/setjmp/x86_64/CMakeLists.txt
===
--- libc/src/setjmp/x86_64/CMakeLists.txt
+++ libc/src/setjmp/x86_64/CMakeLists.txt
@@ -3,12 +3,12 @@
   SRCS
 setjmp.cpp
   HDRS
-setjmp_impl.h
-  COMPILE_OPTIONS
--O3 # We do not want any local variables in setjmp
--fno-omit-frame-pointer # The implementation assumes frame pointer on to the stack
+../setjmp_impl.h
   DEPENDS
 libc.include.setjmp
+  COMPILE_OPTIONS
+-O3
+-fno-omit-frame-pointer
 )
 
 add_entrypoint_object(
@@ -16,9 +16,10 @@
   SRCS
 longjmp.cpp
   HDRS
-longjmp.h
-  COMPILE_OPTIONS
--O3 # We do not want any local variables in longjmp
+../longjmp.h
   DEPENDS
 libc.include.setjmp
+  COMPILE_OPTIONS
+-O3
+-fno-omit-frame-pointer
 )
Index: libc/src/setjmp/riscv64/setjmp.cpp
===
--- /dev/null
+++ libc/src/setjmp/riscv64/setjmp.cpp
@@ -0,0 +1,56 @@
+//===-- Implementation of setjmp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "src/__support/common.h"
+#include "src/setjmp/setjmp_impl.h"
+
+#include 
+
+#if !defined(LIBC_TARGET_ARCH_IS_RISCV64)
+#error "Invalid file include"
+#endif
+
+namespace __llvm_libc {
+
+LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
+  LIBC_INLINE_ASM("sd ra, %0\n\t" : : "m"(buf->__pc) :);
+  LIBC_INLINE_ASM("sd s0, %0\n\t" : : "m"(buf->__regs[0]) :);
+  LIBC_INLINE_ASM("sd s1, %0\n\t" : : "m"(buf->__regs[1]) :);
+  LIBC_INLINE_ASM("sd s2, %0\n\t" : : "m"(buf->__regs[2]) :);
+  LIBC_INLINE_ASM("sd s3, %0\n\t" : : "m"(bu