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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/src/atomic.cpp:19-21
+#ifdef _LIBCPP_HAS_FUTEX_H
 #include <linux/futex.h>
+#endif
----------------
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
```


================
Comment at: libcxx/src/atomic.cpp:49
+    // FUTEX_WAIT_PRIVATE is implemented at 128.
+    syscall(SYS_futex, __ptr, 128, __val, &__timeout, 0, 0);
+#else
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94909