[libcxx-commits] [PATCH] D94909: [libc++][VE] Support futex system call on VE

[Kazushi Marukawa via Phabricator via libcxx-commits]

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

In D94909#3178020 <https://reviews.llvm.org/D94909#3178020>, @ldionne wrote:

> Also, are you able to provide a builder that would run CI on VE?

Yes.  We have CI at https://lab.llvm.org/buildbot/#/builders/91.  It runs builtin's tests at the moment.  We will run libcxxabi tests, libcxx tests, and more soon.



================
Comment at: libcxx/src/atomic.cpp:19-21
+#ifdef _LIBCPP_HAS_FUTEX_H
 #include <linux/futex.h>
+#endif
----------------
ldionne wrote:
> You should be able to remove all the `CMake` changes and replace this by
> 
> ```
> #if __has_include(<linux/futex.h>)
> #  include <linux/futex.h>
> #endif
> ```
Thank you for suggestion.  I modified code.


================
Comment at: libcxx/src/atomic.cpp:49
+    // FUTEX_WAIT_PRIVATE is implemented at 128.
+    syscall(SYS_futex, __ptr, 128, __val, &__timeout, 0, 0);
+#else
----------------
ldionne wrote:
> This is incredibly brittle and not self-documenting. Is there no way that VE can implement `linux/futex.h`? After all, it does provide the `SYS_futex` syscall and it does support `FUTEX_WAIT_PRIVATE`, only not with that macro name. This seems to me like a classic "workaround for something missing on the platform side", and usually it's better to round those rough edges on the platform side instead of working around them in a bunch of places (like here).
I understand how it looks like, but it is simply difficult to let OS peeople add `linux/futex.h` to their distribution.  I tried, but it won't work.  I can try it again and again...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94909