[libc] customizable namespace 1/4

[gchatelet]

Summary

This implements the first step of https://discourse.llvm.org/t/rfc-customizable-namespace-to-allow-testing-the-libc-when-the-system-libc-is-also-llvms-libc/73079

Make required Bazel and CMake preparatory changes which define the var for the customizable namespace and use it (pass it as a compile option -DLIBC_NAMESPACE=<...>).

[sivachandra]

libc/CMakeLists.txt

58

Can we replace lines 51 to 58 with:

set(LIBC_NAMESPACE
  "__llvm_libc_${LLVM_VERSION_MAJOR}_${LLVM_VERSION_MINOR}_${LLVM_VERSION_PATCH}_${LLVM_VERSION_SUFFIX}"
  CACHE STRING "The namespace to use to enclose internal implementations. Must start with '__llvm_libc'."
)

The effective changes are:

1. Call the CMake variable just LIBC_NAMESPACE.
2. Since it is a CACHE var, it will be set only if it is not being set on the command line.
3. We don't need FORCE as we do not really want to change the cached value.

libc/cmake/modules/LLVMLibCCompileOptions.cmake

10

Instead of adding a compile option to every target, can we just do one add_definitions call in libc/CMakeLists.txt?

[gchatelet]

libc/CMakeLists.txt

58

I initially did the two step version with FORCE to make sure that CMake does not retain an old value.

i.e., if we run cmake -DLIBC_NAMESPACE=__llvm_libc ... then cmake ... I would expect the second run to use the autogenerated namespace value. WDYT?

libc/cmake/modules/LLVMLibCCompileOptions.cmake

10

We could. For large projects I tend to prefer being explicit rather than implicit and local rather than global.
LIBC_NAMESPACE might be a could use of add_definitions though.

I think we should still have a file like LLVMLibCCompileOptions.cmake for other options like -fno-expections, -ffreestanding or -fno-rtti. We currently have ad-hoc target_compile_options all over the place which will fail when compiling with MSVC for instance.

[sivachandra]

libc/CMakeLists.txt

58

I initially did the two step version with FORCE to make sure that CMake does not retain an old value.

The way I understand it, the FORCE on line 52 just clears out the command line arg also?

i.e., if we run cmake -DLIBC_NAMESPACE=__llvm_libc ... then cmake ... I would expect the second run to use the autogenerated namespace value. WDYT?

I don't think this is possible in general at all? As in, I am not sure there is a way to overwrite cache variables that way at all other than FORCE of course, but FORCE erases the command line value the first time as well?

If one is making another CMake run with a different set of command line options, they should cleanup the files from the previous run. I went looking for an option and found the --fresh option but it is available only from 3.24: https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-fresh.

libc/cmake/modules/LLVMLibCCompileOptions.cmake

10

We could. For large projects I tend to prefer being explicit rather than implicit and local rather than global.
LIBC_NAMESPACE might be a could use of add_definitions though.

There cannot be two different namespaces during a single build so I think just adding via add_definitions would be sufficient. If we hit problems, we can always make it more complicated at that time.

I think we should still have a file like LLVMLibCCompileOptions.cmake for other options like -fno-expections, -ffreestanding or -fno-rtti. We currently have ad-hoc target_compile_options all over the place which will fail when compiling with MSVC for instance.

Acknowledged. I think we can do it irrespective of this POC.

[gchatelet]

• Address comments
• Support bazel as well

[gchatelet]

• fix typo

[gchatelet]

• make bazel load statement a one liner

libc/CMakeLists.txt

58

Alright, let's make it simple then.

[gchatelet]

• fail on unsupported compiler

[gchatelet]

• fix auto-formatting

[gchatelet]

• Fixing messed up file

[sivachandra]

How do you want to take this forward?

libc/CMakeLists.txt

67

If we use add_definitions, we shouldn't use -D or the /D syntax. CMake will use the appropriate way to add a macro-def based on the compiler we use.

[sivachandra]

libc/CMakeLists.txt

67

I think I am wrong here. Scrub this!

[gchatelet]

• use add_compile_definitions instead of add_definitions

[gchatelet]

libc/CMakeLists.txt

67

That's actually the case when using add_compile_definitions (doc) instead of add_definitions (doc). I've used the more modern version since we can support it (3.20 > 3.12)

[gchatelet]

In D159112#4632351, @sivachandra wrote:

How do you want to take this forward?

Let's stick to the plan in https://discourse.llvm.org/t/rfc-customizable-namespace-to-allow-testing-the-libc-when-the-system-libc-is-also-llvms-libc/73079.
I will update this patch to conform to the first step (removing the guard that is) and submit.
Then I'll create a second patch to add the guard.

[gchatelet]

• Remove guard from libc/src/__support/common.h

[sivachandra]

Have an inline question but LGTM.

libc/CMakeLists.txt

56

May be I am missing something. Wouldn't the FORCE make it take the else path always?

[gchatelet]

libc/CMakeLists.txt

56

Hmm I thought I used your previous suggestion but it seems I messed up. Thx for noticing.

[gchatelet]

• Addess comments

[gchatelet]

FYI I'll submit this patch as a github PR.

[gchatelet]

Abandoning the review in Phabricator.
Review in github : https://github.com/llvm/llvm-project/pull/65321

libc/CMakeLists.txt

if(("libc" IN_LIST LLVM_ENABLE_RUNTIMES AND NOT LLVM_RUNTIMES_BUILD) OR
   return()
 endif()
 
 option(LIBC_CMAKE_VERBOSE_LOGGING
        "Log details warnings and notifications during CMake configuration." OFF)
 # Path libc/scripts directory.
 set(LIBC_BUILD_SCRIPTS_DIR "${LIBC_SOURCE_DIR}/utils/build_scripts")
 
+# Defining a global namespace for enclose all libc functions.
+set(LIBC_CMAKE_NAMESPACE "" CACHE STRING "The namespace to use to enclose internal implementations. Must start with '__llvm_libc'." FORCE)
+
+if(LIBC_CMAKE_NAMESPACE)
+  set(LLVM_LIBC_NAMESPACE "${LIBC_CMAKE_NAMESPACE}")
+else()
+  set(LLVM_LIBC_NAMESPACE "__llvm_libc_${LLVM_VERSION_MAJOR}_${LLVM_VERSION_MINOR}_${LLVM_VERSION_PATCH}_${LLVM_VERSION_SUFFIX}")
+endif()
+
+if(NOT LLVM_LIBC_NAMESPACE MATCHES "^__llvm_libc")
+  message(FATAL_ERROR "Invalid LLVM_LIBC_NAMESPACE. Must start with '__llvm_libc' was '${LLVM_LIBC_NAMESPACE}'")
+endif()
+
+message(STATUS "Setting LLVM_LIBC_NAMESPACE namespace to '${LLVM_LIBC_NAMESPACE}'")
+
 # Flags to pass down to the compiler while building the libc functions.
 set(LIBC_COMPILE_OPTIONS_DEFAULT "" CACHE STRING "Architecture to tell clang to optimize for (e.g. -march=... or -mcpu=...)")
 
 include(common_libc_tuners.cmake)
 
 list(APPEND LIBC_COMPILE_OPTIONS_DEFAULT ${LIBC_COMMON_TUNE_OPTIONS})
 
 # Check --print-resource-dir to find the compiler resource dir if this flag
 # is supported by the compiler.
 execute_process(

libc/cmake/modules/LLVMLibCCompileOptions.cmake

+function(add_namespace_compile_option target_name)
+    if(NOT LLVM_LIBC_NAMESPACE)
+        message(FATAL_ERROR "LLVM_LIBC_NAMESPACE is not defined.")
+    endif()
+
+    if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
+        target_compile_options(${target_name} PRIVATE "-DLIBC_NAMESPACE=${LLVM_LIBC_NAMESPACE}")
+    elseif(MSVC)
+        target_compile_options(${target_name} PRIVATE "/DLIBC_NAMESPACE=${LLVM_LIBC_NAMESPACE}")
+    endif()
+endfunction()

libc/cmake/modules/LLVMLibCObjectRules.cmake

foreach(gpu_arch ${LIBC_GPU_ARCHITECTURES})
       # packaged GPU binaries.
       add_library(${gpu_target_name}
         EXCLUDE_FROM_ALL
         OBJECT
         ${add_gpu_obj_src}
         ${ADD_GPU_OBJ_HDRS}
       )
 
+      add_namespace_compile_option(${gpu_target_name})
       target_compile_options(${gpu_target_name} PRIVATE ${compile_options})
       target_include_directories(${gpu_target_name} PRIVATE ${include_dirs})
       target_compile_definitions(${gpu_target_name} PRIVATE LIBC_COPT_PUBLIC_PACKAGING)
       set_target_properties(
         ${gpu_target_name}
         PROPERTIES
           CXX_STANDARD ${ADD_GPU_OBJ_CXX_STANDARD}
       )
function(_build_gpu_objects fq_target_name internal_target_name)
   add_library(
     ${fq_target_name}
     # We want an object library as the objects will eventually get packaged into
     # an archive (like libcgpu.a).
     EXCLUDE_FROM_ALL
     OBJECT
     ${CMAKE_CURRENT_BINARY_DIR}/stubs/${stub_filename}
   )
+  add_namespace_compile_option(${fq_target_name})
   target_compile_options(${fq_target_name} BEFORE PRIVATE
                          ${common_compile_options} -nostdlib)
   foreach(packaged_gpu_binary ${packaged_gpu_binaries})
     target_compile_options(${fq_target_name} PRIVATE
                            "SHELL:-Xclang -fembed-offload-object=${packaged_gpu_binary}")
   endforeach()
   target_include_directories(${fq_target_name} PRIVATE ${include_dirs})
   add_dependencies(${fq_target_name}
                    ${full_deps_list} ${packaged_gpu_names} ${stub_target_name})
 
   # We only build the internal target for a single supported architecture.
   if(LIBC_GPU_TARGET_ARCHITECTURE_IS_AMDGPU OR
      LIBC_GPU_TARGET_ARCHITECTURE_IS_NVPTX)
     add_library(
       ${internal_target_name}
       EXCLUDE_FROM_ALL
       OBJECT
       ${ADD_GPU_OBJ_SRCS}
       ${ADD_GPU_OBJ_HDRS}
     )
+    add_namespace_compile_option(${internal_target_name})
     target_compile_options(${internal_target_name} BEFORE PRIVATE
                            ${common_compile_options} --target=${LIBC_GPU_TARGET_TRIPLE})
     if(LIBC_GPU_TARGET_ARCHITECTURE_IS_AMDGPU)
       target_compile_options(${internal_target_name} PRIVATE -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto)
     elseif(LIBC_GPU_TARGET_ARCHITECTURE_IS_NVPTX)
       get_nvptx_compile_options(nvptx_options ${LIBC_GPU_TARGET_ARCHITECTURE})
       target_compile_options(${internal_target_name} PRIVATE ${nvptx_options})
     endif()
else()
     )
     target_include_directories(
       ${fq_target_name}
       PRIVATE
         ${LIBC_SOURCE_DIR}
         ${LIBC_INCLUDE_DIR}
     )
     target_compile_options(${fq_target_name} PRIVATE ${compile_options})
+    add_namespace_compile_option(${fq_target_name})
   endif()
 
   if(SHOW_INTERMEDIATE_OBJECTS)
     message(STATUS "Adding object library ${fq_target_name}")
     if(${SHOW_INTERMEDIATE_OBJECTS} STREQUAL "DEPS")
       foreach(dep IN LISTS ADD_OBJECT_DEPENDS)
         message(STATUS "  ${fq_target_name} depends on ${dep}")
       endforeach()
add_library(
       ${internal_target_name}
       # TODO: We don't need an object library for internal consumption.
       # A future change should switch this to a normal static library.
       EXCLUDE_FROM_ALL
       OBJECT
       ${ADD_ENTRYPOINT_OBJ_SRCS}
       ${ADD_ENTRYPOINT_OBJ_HDRS}
     )
+    add_namespace_compile_option(${internal_target_name})
     target_compile_options(${internal_target_name} BEFORE PRIVATE ${common_compile_options})
     target_include_directories(${internal_target_name} PRIVATE ${include_dirs})
     add_dependencies(${internal_target_name} ${full_deps_list})
 
     add_library(
       ${fq_target_name}
       # We want an object library as the objects will eventually get packaged into
       # an archive (like libc.a).
       EXCLUDE_FROM_ALL
       OBJECT
       ${ADD_ENTRYPOINT_OBJ_SRCS}
       ${ADD_ENTRYPOINT_OBJ_HDRS}
     )
+    add_namespace_compile_option(${fq_target_name})
     target_compile_options(${fq_target_name} BEFORE PRIVATE ${common_compile_options} -DLIBC_COPT_PUBLIC_PACKAGING)
     target_include_directories(${fq_target_name} PRIVATE ${include_dirs})
     add_dependencies(${fq_target_name} ${full_deps_list})
   endif()
 
   set_target_properties(
     ${fq_target_name}
     PROPERTIES
add_library(
     EXCLUDE_FROM_ALL
     OBJECT
     ${REDIRECTOR_OBJECT_SRC}
   )
   target_compile_options(
     ${target_name}
     BEFORE PRIVATE -fPIC ${LIBC_COMPILE_OPTIONS_DEFAULT}
   )
+  add_namespace_compile_option(${target_name})
 endfunction(add_redirector_object)

libc/cmake/modules/LLVMLibCRules.cmake

+include(LLVMLibCCompileOptions)
 include(LLVMLibCTargetNameUtils)
 include(LLVMLibCFlagRules)
 include(LLVMLibCObjectRules)
 include(LLVMLibCLibraryRules)
 include(LLVMLibCTestRules)

libc/cmake/modules/LLVMLibCTestRules.cmake

add_executable(
     ${LIBC_UNITTEST_HDRS}
   )
   target_include_directories(
     ${fq_build_target_name}
     PRIVATE
       ${LIBC_SOURCE_DIR}
       ${LIBC_INCLUDE_DIR}
   )
+  add_namespace_compile_option(${fq_build_target_name})
   target_compile_options(
     ${fq_build_target_name}
     PRIVATE -fpie ${LIBC_COMPILE_OPTIONS_DEFAULT}
   )
   if(LLVM_LIBC_FULL_BUILD)
     target_compile_options(
       ${fq_build_target_name}
       PRIVATE -ffreestanding
set_target_properties(${fq_target_name}
       PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 
   add_dependencies(
     ${fq_target_name}
     ${fq_deps_list}
   )
   add_dependencies(libc-fuzzer ${fq_target_name})
 
+  add_namespace_compile_option(${fq_target_name})
   target_compile_options(${fq_target_name}
     PRIVATE
     ${LIBC_FUZZER_COMPILE_OPTIONS})
 
 endfunction(add_libc_fuzzer)
 
 # DEPRECATED: Use add_hermetic_test instead.
 #
function(add_integration_test test_name)
   set_target_properties(${fq_build_target_name}
       PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   target_include_directories(
     ${fq_build_target_name}
     PRIVATE
       ${LIBC_SOURCE_DIR}
       ${LIBC_INCLUDE_DIR}
   )
+  add_namespace_compile_option(${fq_build_target_name})
   target_compile_options(${fq_build_target_name}
       PRIVATE -fpie -ffreestanding -fno-exceptions -fno-rtti ${INTEGRATION_TEST_COMPILE_OPTIONS})
   # The GPU build requires overriding the default CMake triple and architecture.
   if(LIBC_GPU_TARGET_ARCHITECTURE_IS_AMDGPU)
     target_compile_options(${fq_build_target_name} PRIVATE
                            -nogpulib -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE}
                            -flto --target=${LIBC_GPU_TARGET_TRIPLE})
   elseif(LIBC_GPU_TARGET_ARCHITECTURE_IS_NVPTX)
add_library(
     STATIC
     EXCLUDE_FROM_ALL
     ${link_object_files}
   )
   set_target_properties(${fq_target_name}.__libc__
       PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   set_target_properties(${fq_target_name}.__libc__
       PROPERTIES ARCHIVE_OUTPUT_NAME ${fq_target_name}.libc)
+  add_namespace_compile_option(${fq_target_name}.__libc__)
 
   set(fq_build_target_name ${fq_target_name}.__build__)
   add_executable(
     ${fq_build_target_name}
     EXCLUDE_FROM_ALL
     # The NVIDIA 'nvlink' linker does not currently support static libraries.
     $<$<BOOL:${LIBC_GPU_TARGET_ARCHITECTURE_IS_NVPTX}>:${link_object_files}>
     ${HERMETIC_TEST_SRCS}
     ${HERMETIC_TEST_HDRS}
   )
   set_target_properties(${fq_build_target_name}
     PROPERTIES
       RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
       #OUTPUT_NAME ${fq_target_name}
   )
   target_include_directories(
     ${fq_build_target_name}
     PRIVATE
       ${LIBC_SOURCE_DIR}
       ${LIBC_INCLUDE_DIR}
   )
+  add_namespace_compile_option(${fq_build_target_name})
   target_compile_options(${fq_build_target_name}
       PRIVATE ${LIBC_HERMETIC_TEST_COMPILE_OPTIONS} ${HERMETIC_TEST_COMPILE_OPTIONS})
 
   set(link_libraries "")
   foreach(lib IN LISTS HERMETIC_TEST_LINK_LIBRARIES)
     if(TARGET ${lib}.hermetic)
       list(APPEND link_libraries ${lib}.hermetic)
     else()

libc/src/__support/common.h

 //===-- Common internal contructs -------------------------------*- 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_SUPPORT_COMMON_H
 #define LLVM_LIBC_SUPPORT_COMMON_H
 
+#ifndef LIBC_NAMESPACE
+#error "LIBC_NAMESPACE macro is not defined."
+#endif
+
 #include "src/__support/macros/attributes.h"
 #include "src/__support/macros/properties/architectures.h"
 
 #ifndef LLVM_LIBC_FUNCTION_ATTR
 #define LLVM_LIBC_FUNCTION_ATTR
 #endif
 
 // MacOS needs to be excluded because it does not support aliasing.

libc/test/UnitTest/CMakeLists.txt

foreach(lib IN ITEMS ${name}.unit ${name}.hermetic)
     add_library(
       ${lib}
       ${library_type}
       EXCLUDE_FROM_ALL
       ${TEST_LIB_SRCS}
       ${TEST_LIB_HDRS}
     )
     target_include_directories(${lib} PUBLIC ${LIBC_SOURCE_DIR})
+    add_namespace_compile_option(${lib})
     target_compile_options(${lib} PRIVATE -fno-exceptions -fno-rtti)
   endforeach()
   target_include_directories(${name}.hermetic PRIVATE ${LIBC_BUILD_DIR}/include)
+  add_namespace_compile_option(${name}.hermetic)
   target_compile_options(${name}.hermetic
       PRIVATE ${LIBC_HERMETIC_TEST_COMPILE_OPTIONS} -ffreestanding -nostdinc++)
 
   if(TEST_LIB_DEPENDS)
     foreach(dep IN LISTS ${TEST_LIB_DEPENDS})
       if(TARGET ${dep}.unit)
         add_dependencies(${name}.unit ${dep}.unit)
       else()