[libc-commits] [PATCH] D143261: [libc][AArch64] Fix fullbuild when using G++/GCC

Fri Feb 3 08:23:48 PST 2023

DavidSpickett marked an inline comment as done.
DavidSpickett added a comment.

Side note, I suspect that `__arm_wsr64("x29", __arm_rsr64("sp"))` could end up optimised out in some scenarios. Checking a clang build, it is there so maybe I am wrong but I will do some more investigation there.

Shouldn't block this though.

================
Comment at: libc/src/__support/threads/linux/thread.cpp:221
+#else
+    asm volatile("mov x29, sp");
+#endif
----------------
sivachandra wrote:
> DavidSpickett wrote:
> > This I maybe don't need volatile and might need to describe the side effects? If you're ok with inline asm here in general, I'll check if I need those.
> Yes, this is OK. I am not an inline assembly expert, but I suspect you do need `volatile` (because the instruction is not touching any variables in C++), and that you don't need to describe side effects as it is clear that `x29` is being updated. I will let you tell me what is the most appropriate thing to do here.
Volatile is needed so the compiler doesn't move it about.

x29 (frame pointer) is set here so that in start_thread you can read the arguments. So in fact we don't want to tell the compiler that x29 is updated. 

If we did it would save and restore the value around the statement and when we went to start thread, it would be incorrect.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143261