[LIBC] Implement `sched_yield()`

[goldstein.w.n]

Summary

Implements: https://linux.die.net/man/2/sched_yield

Possibly we don't need the return value check / errno as according to
both the manpage (and current linux source) sched_yield cannot fail.

[michaelrj]

The function looks good, but since you're adding a new function there's a bit of extra configuration that needs to be done. I'll walk you through it here, but if you want an example then this patch should also show what needs to be done: https://reviews.llvm.org/D147970.

First, we need the function to have a FunctionSpec in the spec/ folder. For this function it will be in spec/gnu_ext.td, alongside sched_getaffinity.
Next, to get the function to build on the target platform we need to add the entrypoint to the list in config/linux/x86_64/entrypoints.txt. If you want you can also add it to the entrypoints list for other CPU architectures (e.g. RISC-V), but x86_64 is the most important one for testing.
We require all of the functions to have unit tests, which will go in test/src/sched/sched_yield_test.cpp. Don't worry about this making this too complex. This is a simple syscall wrapper so it just needs to test that calling the function succeeds.
Finally, the cmake for the sched folder is a little unusual, so you'll also need to add an alias to src/sched/CMakeLists.txt.

I know this looks like a lot, but each part should be fairly quick. Feel free to reach out if you want more guidance on how to do it.

[goldstein.w.n]

Do the rest of the work

[goldstein.w.n]

In D147985#4259116, @michaelrj wrote:

The function looks good, but since you're adding a new function there's a bit of extra configuration that needs to be done. I'll walk you through it here, but if you want an example then this patch should also show what needs to be done: https://reviews.llvm.org/D147970.

First, we need the function to have a FunctionSpec in the spec/ folder. For this function it will be in spec/gnu_ext.td, alongside sched_getaffinity.
Next, to get the function to build on the target platform we need to add the entrypoint to the list in config/linux/x86_64/entrypoints.txt. If you want you can also add it to the entrypoints list for other CPU architectures (e.g. RISC-V), but x86_64 is the most important one for testing.
We require all of the functions to have unit tests, which will go in test/src/sched/sched_yield_test.cpp. Don't worry about this making this too complex. This is a simple syscall wrapper so it just needs to test that calling the function succeeds.
Finally, the cmake for the sched folder is a little unusual, so you'll also need to add an alias to src/sched/CMakeLists.txt.

I know this looks like a lot, but each part should be fairly quick. Feel free to reach out if you want more guidance on how to do it.

Done, thanks for the guidance.

As an aside, are you planning to implement anything along the lines of gnu ifunc? If not is the method
for arch-specific just with higher isa builds and compiler flags (i.e #ifdef __AVX2__ and stuff like that).

Also are you guys planning on implementing dynamic loader?

[dxf]

As an aside, are you planning to implement anything along the lines of gnu ifunc? If not is the method
for arch-specific just with higher isa builds and compiler flags (i.e #ifdef __AVX2__ and stuff like that).

Yes, the method is to build with architecture-specific flags. You can see examples of this in files like https://github.com/llvm/llvm-project/blob/main/libc/src/string/memory_utils/op_x86.h and https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/sinf.cpp

The model is to consider the libc functions as just another part of your application code that gets statically linked in. So you build the libc with the same optimization level, sanitizers, architecture defines, etc. you build your application with, and then deploy appropriately.

Also are you guys planning on implementing dynamic loader?

At some point, though it's not a current area of investment for anyone. If this is an area that interests you, patches are welcome!

[michaelrj]

You're just missing the test file then I'll approve.

libc/test/src/sched/CMakeLists.txt

24

did you forget to include the test file?

[goldstein.w.n]

libc/test/src/sched/CMakeLists.txt

24

rip untracked files. fixed.

[goldstein.w.n]

Add tests

[michaelrj]

LGTM with nit

libc/test/src/sched/yield_test.cpp

10

Nit: I'm not sure you need to include syscall here.

[goldstein.w.n]

Remove unneeded dep

[goldstein.w.n]

libc/test/src/sched/yield_test.cpp

10

Fixed. 1 more unrelated question if you have the time.

I'm working on adding support for proper attr handling in pthread_create, but am finding normal libc tests / build doesn't include pthread (on linux).

Any idea for the extra steps to build/test pthread?

[goldstein.w.n]

libc/spec/gnu_ext.td

52

@michaelrj are you sure this is the right place for sched_yield. Seems to be part of POSIX specification:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sched.h.html

[sivachandra]

libc/spec/gnu_ext.td

52

You are correct - this should be spec/posix.td.

libc/test/src/sched/yield_test.cpp

10

I'm working on adding support for proper attr handling in pthread_create, but am finding normal libc tests / build doesn't include pthread (on linux).

Any idea for the extra steps to build/test pthread?

Depends on what you mean by "normal libc tests". If you are running check-libc on linux, it should already be including the pthread tests. But, you can run pthread tests by running libc-integration-tests or use a more focused target libc-pthread-integration-tests. The tests live here: https://github.com/llvm/llvm-project/tree/main/libc/test/integration/src/pthread

[goldstein.w.n]

Move to posix.td

[goldstein.w.n]

libc/test/src/sched/yield_test.cpp

10

I'm working on adding support for proper attr handling in pthread_create, but am finding normal libc tests / build doesn't include pthread (on linux).

Any idea for the extra steps to build/test pthread?

Depends on what you mean by "normal libc tests". If you are running check-libc on linux, it should already be including the pthread tests.

Hmm, I'm noticing in a variety of cases some tests are skipped (including the entire pthread suite).
Building:

cmake -S llvm -B build -G Ninja -DLLVM_ENABLE_PROJECTS='libc;llvm;clang' -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXX_FLAGS="-O1 -Wno-address" -DCMAKE_C_FLAGS="-O1 -Wno-address" -DLLVM_ENABLE_RTTI=ON -DLLVM_ENABLE_EH=ON -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_INSTALL_PREFIX=/home/noah/programs/opensource/llvm-dev/build -DLLVM_CCACHE_BUILD=ON -DLLVM_BINUTILS_INCDIR=/home/noah/programs/opensource/binutils-dev/src/binutils-gdb/include/plugin-api.h

and testing with ninja check-libc

Anything inherently wrong with that?

But, you can run pthread tests by running libc-integration-tests or use a more focused target libc-pthread-integration-tests. The tests live here: https://github.com/llvm/llvm-project/tree/main/libc/test/integration/src/pthread

[sivachandra]

OK from my side but please wait for @michaelrj who is leading this review.

libc/test/src/sched/yield_test.cpp

10

Ah! Parts like pthread which depend on type definitions are only available in the full build mode: -DLLVM_LIBC_FULL_BUILD=ON.

[michaelrj]

good catch that this should be in posix, it should be good to go once you make this change, if you want to make sure that everything is working then run ninja libc-integration-tests as well as check-libc

libc/spec/posix.td

1319

you need to add Sched to the list of headers here.

[goldstein.w.n]

Add Sched to posix.td header

[goldstein.w.n]

In D147985#4262233, @michaelrj wrote:

good catch that this should be in posix, it should be good to go once you make this change, if you want to make sure that everything is working then run ninja libc-integration-tests as well as check-libc

Done, all pass. Likewise for D148069

[michaelrj]

Looks good to me, feel free to land it.

libc/config/linux/aarch64/entrypoints.txt

set(TARGET_LIBC_ENTRYPOINTS
     # fcntl.h entrypoints
     libc.src.fcntl.creat
     libc.src.fcntl.open
     libc.src.fcntl.openat
 
     # sched.h entrypoints
     libc.src.sched.sched_getaffinity
     libc.src.sched.sched_setaffinity
+    libc.src.sched.sched_yield
 
     # string.h entrypoints
     libc.src.string.bcmp
     libc.src.string.bcopy
     libc.src.string.bzero
     libc.src.string.index
     libc.src.string.memccpy
     libc.src.string.memchr

libc/config/linux/riscv64/entrypoints.txt

set(TARGET_LIBC_ENTRYPOINTS
     # fcntl.h entrypoints
     libc.src.fcntl.creat
     libc.src.fcntl.open
     libc.src.fcntl.openat
 
     # sched.h entrypoints
     libc.src.sched.sched_getaffinity
     libc.src.sched.sched_setaffinity
+    libc.src.sched.sched_yield
 
     # string.h entrypoints
     libc.src.string.bcmp
     libc.src.string.bcopy
     libc.src.string.bzero
     libc.src.string.index
     libc.src.string.memccpy
     libc.src.string.memchr

libc/config/linux/x86_64/entrypoints.txt

set(TARGET_LIBC_ENTRYPOINTS
     # fcntl.h entrypoints
     libc.src.fcntl.creat
     libc.src.fcntl.open
     libc.src.fcntl.openat
 
     # sched.h entrypoints
     libc.src.sched.sched_getaffinity
     libc.src.sched.sched_setaffinity
+    libc.src.sched.sched_yield
 
     # string.h entrypoints
     libc.src.string.bcmp
     libc.src.string.bcopy
     libc.src.string.bzero
     libc.src.string.index
     libc.src.string.memccpy
     libc.src.string.memchr

libc/spec/gnu_ext.td

HeaderSpec Math = HeaderSpec<
         FunctionSpec<
             "sincosf",
             RetValSpec<VoidType>,
             [ArgSpec<FloatType>, ArgSpec<FloatPtr>, ArgSpec<FloatPtr>]
         >,
         FunctionSpec<"exp10f", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
       ]
   >;
- 
   HeaderSpec Sched = HeaderSpec<
       "sched.h",
       [], // Macros
       [PidT, SizeTType, CpuSetT], // Types
       [], // Enumerations
       [
         FunctionSpec<
             "sched_getaffinity",
             RetValSpec<IntType>,
             [ArgSpec<PidT>, ArgSpec<SizeTType>, ArgSpec<CpuSetPtr>]
         >,
         FunctionSpec<
             "sched_setaffinity",
             RetValSpec<IntType>,
             [ArgSpec<PidT>, ArgSpec<SizeTType>, ArgSpec<ConstCpuSetPtr>]
         >,
       ]
   >;
- 
   HeaderSpec String = HeaderSpec<
       "string.h",
       [], // Macros
       [], // Types
       [], // Enumerations
       [
          FunctionSpec<
             "memmem",

libc/spec/posix.td

[
         FunctionSpec<
           "getenv",
           RetValSpec<CharPtr>,
           [ArgSpec<ConstCharPtr>]
         >,
     ]
   >;
 
+  HeaderSpec Sched = HeaderSpec<
+      "sched.h",
+      [], // Macros
+      [PidT, SizeTType, CpuSetT], // Types
+      [], // Enumerations
+      [
+        FunctionSpec<
+            "sched_yield",
+            RetValSpec<IntType>,
+            []
+        >,
+      ]
+  >;
+
   HeaderSpec String = HeaderSpec<
     "string.h",
     [
         Macro<"NULL">,
     ],
     [
         SizeTType,
     ],

 
   let Headers = [
     ArpaInet,
     CType,
     Dirent,
     Errno,
     FCntl,
     PThread,
+    Sched,
     Signal,
     Spawn,
     StdIO,
     StdLib,
     SysIOctl,
     SysMMan,
     SysResource,
     SysSelect,
     SysStat,
     SysTypes,
     SysUtsName,
     SysWait,
     Time,
     Termios,
     UniStd,
     String
   ];
 }

libc/src/sched/CMakeLists.txt

 add_entrypoint_object(
   sched_setaffinity
   ALIAS
   DEPENDS
     .${LIBC_TARGET_OS}.sched_setaffinity
 )
 
 add_entrypoint_object(
+  sched_yield
+  ALIAS
+  DEPENDS
+    .${LIBC_TARGET_OS}.sched_yield
+)
+
+add_entrypoint_object(
   __sched_getcpucount
   ALIAS
   DEPENDS
     .${LIBC_TARGET_OS}.__sched_getcpucount
 )

libc/src/sched/linux/CMakeLists.txt

 add_entrypoint_object(
   __sched_getcpucount
   SRCS
     sched_getcpucount.cpp
   HDRS
     ../sched_getcpucount.h
   DEPENDS
     libc.include.sched
   )
+
+add_entrypoint_object(
+  sched_yield
+  SRCS
+    sched_yield.cpp
+  HDRS
+    ../sched_yield.h
+  DEPENDS
+    libc.include.sys_syscall
+    libc.src.__support.OSUtil.osutil
+    libc.src.errno.errno
+)

libc/src/sched/linux/sched_yield.cpp

+//===-- Implementation of sched_yield -------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/sched/sched_yield.h"
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
+#include "src/errno/libc_errno.h"
+
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace __llvm_libc {
+
+LLVM_LIBC_FUNCTION(int, sched_yield, ()) {
+  long ret = __llvm_libc::syscall_impl(SYS_sched_yield);
+  // As of writing this, yield() cannot fail
+  if (ret < 0) {
+    libc_errno = -ret;
+    return -1;
+  }
+  return 0;
+}
+
+} // namespace __llvm_libc

libc/src/sched/sched_yield.h

+//===-- Implementation header for sched_yield -------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SCHED_SCHED_YIELD_H
+#define LLVM_LIBC_SRC_SCHED_SCHED_YIELD_H
+
+namespace __llvm_libc {
+
+int sched_yield(void);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_SCHED_SCHED_YIELD_H

libc/test/src/sched/CMakeLists.txt

DEPENDS
     libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
     libc.src.sched.sched_getaffinity
     libc.src.sched.sched_setaffinity
     libc.test.errno_setter_matcher
 )
 
 add_libc_unittest(
+  yield_test
+  SUITE
+    libc_sched_unittests
+  SRCS
+    yield_test.cpp
+  DEPENDS
+    libc.src.errno.errno
+    libc.src.sched.sched_yield
+)
+
+add_libc_unittest(
   cpu_count_test
   SUITE
     libc_sched_unittests
   SRCS
     cpu_count_test.cpp
   DEPENDS
     libc.include.sched
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
     libc.src.sched.sched_getaffinity
     libc.src.sched.__sched_getcpucount
     libc.test.errno_setter_matcher
 )

libc/test/src/sched/yield_test.cpp

+//===-- Unittests for sched_yield -----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/errno/libc_errno.h"
+#include "src/sched/sched_yield.h"
+#include "test/UnitTest/Test.h"
+
+TEST(LlvmLibcSchedYieldTest, SmokeTest) {
+  libc_errno = 0;
+  // sched_yield() always succeeds, just do a basic test that errno/ret are
+  // properly 0.
+  ASSERT_EQ(__llvm_libc::sched_yield(), 0);
+  ASSERT_EQ(libc_errno, 0);
+}