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

[Mikhail Ramalho via Phabricator via libc-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