[libc-commits] [PATCH] D75818: [libc] Take 2: Add linux implementations of thrd_create and thrd_join functions.

[Alex Brachet via Phabricator via libc-commits]

abrachet accepted this revision.
abrachet added a comment.
This revision is now accepted and ready to land.

This looks good to me. Maybe worth waiting for @phosek.

FWIW the tests works fine on my machine but the last patch did as well. Also, exceptions can properly unwind the threads stack which I thought might have been an issue with calling clone in C and not being able to emit the correct cfi directives. Which is to say I'm fairly confident everything is set up properly now :)



================
Comment at: libc/src/threads/linux/thrd_create.cpp:81-82
+  long clone_result =
+      __llvm_libc::syscall(SYS_clone, clone_flags, adjusted_stack,
+                           &thread->__tid, clear_tid_address, 0);
+
----------------
Maybe put a TODO here because clone's arguments are different on other architectures. Perhaps we should just use the libc wrapper for clone here later?


================
Comment at: libc/src/threads/linux/thrd_join.cpp:31-33
+    // The kernel should set the value at the clear tid address to zero.
+    // If not, it is a spurious wake and we should continue to wait on
+    // the futex.
----------------
Maybe this comment should be moved up to the while loops condition


================
Comment at: libc/src/threads/linux/x86_64/thread_start_args.h.in:12
+__attribute__((always_inline)) inline uintptr_t get_start_args_addr() {
+  // NOTE: For __builtin_frame_address to workd reliably across compilers,
+  // architectures and various optimization levels, the TU including this file
----------------
workd -> work


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75818