[libc] Propagate printf config options from a single config header library. #66979
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Changesprintf_core.parser is not yet updated to use the printf config options; It Full diff: https://github.com/llvm/llvm-project/pull/66979.diff 3 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index c10b81f1af8cba3..8c4b92e4fb4577b 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -383,6 +383,8 @@ function(create_object_library fq_target_name)
if(fq_deps_list)
add_dependencies(${fq_target_name} ${fq_deps_list})
+ # Add deps as link libraries to inherit interface compile and link options.
+ target_link_libraries(${fq_target_name} ${fq_deps_list})
endif()
if(NOT ADD_OBJECT_CXX_STANDARD)
diff --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index f3a75fb965c6e16..9f23362ab16ca2f 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -188,19 +188,6 @@ list(APPEND printf_deps
libc.src.stdio.printf_core.vfprintf_internal
)
-if(LIBC_CONF_PRINTF_DISABLE_FLOAT)
- list(APPEND printf_copts "-DLIBC_COPT_PRINTF_DISABLE_FLOAT")
-endif()
-if(LIBC_CONF_PRINTF_DISABLE_INDEX_MODE)
- list(APPEND printf_copts "-DLIBC_COPT_PRINTF_DISABLE_INDEX_MODE")
-endif()
-if(LIBC_CONF_PRINTF_DISABLE_WRITE_INT)
- list(APPEND printf_copts "-DLIBC_COPT_PRINTF_DISABLE_WRITE_INT")
-endif()
-if(LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE)
- list(APPEND printf_copts "-DLIBC_COPT_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE")
-endif()
-
if(LLVM_LIBC_FULL_BUILD)
list(APPEND printf_deps
libc.src.__support.File.file
@@ -208,7 +195,7 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.__support.File.platform_stdout
)
else()
- list(APPEND printf_copts "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE")
+ list(APPEND use_system_file "COMPILE_OPTIONS" "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE")
endif()
add_entrypoint_object(
@@ -219,8 +206,7 @@ add_entrypoint_object(
printf.h
DEPENDS
${printf_deps}
- COMPILE_OPTIONS
- ${printf_copts}
+ ${use_system_file}
)
add_entrypoint_object(
@@ -232,8 +218,7 @@ add_entrypoint_object(
DEPENDS
libc.src.__support.arg_list
libc.src.stdio.printf_core.vfprintf_internal
- COMPILE_OPTIONS
- ${printf_copts}
+ ${use_system_file}
)
add_entrypoint_object(
@@ -266,8 +251,7 @@ add_entrypoint_object(
vprintf.h
DEPENDS
${printf_deps}
- COMPILE_OPTIONS
- ${printf_copts}
+ ${use_system_file}
)
add_entrypoint_object(
@@ -279,8 +263,7 @@ add_entrypoint_object(
DEPENDS
libc.src.__support.arg_list
libc.src.stdio.printf_core.vfprintf_internal
- COMPILE_OPTIONS
- ${printf_copts}
+ ${use_system_file}
)
add_subdirectory(printf_core)
diff --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index 7087d28ede66e8b..07f1e0aeb7ef322 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -1,3 +1,25 @@
+if(LIBC_CONF_PRINTF_DISABLE_FLOAT)
+ list(APPEND printf_config_copts "-DLIBC_COPT_PRINTF_DISABLE_FLOAT")
+endif()
+if(LIBC_CONF_PRINTF_DISABLE_INDEX_MODE)
+ list(APPEND printf_config_copts "-DLIBC_COPT_PRINTF_DISABLE_INDEX_MODE")
+endif()
+if(LIBC_CONF_PRINTF_DISABLE_WRITE_INT)
+ list(APPEND printf_config_copts "-DLIBC_COPT_PRINTF_DISABLE_WRITE_INT")
+endif()
+if(LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE)
+ list(APPEND printf_config_copts "-DLIBC_COPT_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE")
+endif()
+if(printf_config_copts)
+ list(PREPEND printf_config_copts "COMPILE_OPTIONS")
+endif()
+
+add_header_library(
+ printf_config
+ HDRS
+ printf_config.h
+ ${printf_config_copts}
+)
add_header_library(
core_structs
@@ -34,6 +56,7 @@ add_object_library(
parser.h
DEPENDS
.core_structs
+ .printf_config
libc.src.__support.arg_list
libc.src.__support.ctype_utils
libc.src.__support.str_to_integer
@@ -53,11 +76,11 @@ add_object_library(
HDRS
writer.h
DEPENDS
+ .core_structs
libc.src.__support.CPP.string_view
libc.src.__support.macros.optimization
libc.src.string.memory_utils.inline_memcpy
libc.src.string.memory_utils.inline_memset
- .core_structs
)
add_object_library(
@@ -79,6 +102,7 @@ add_object_library(
DEPENDS
.writer
.core_structs
+ .printf_config
libc.src.__support.CPP.limits
libc.src.__support.CPP.span
libc.src.__support.CPP.string_view
@@ -91,8 +115,6 @@ add_object_library(
libc.src.__support.uint128
libc.src.__support.integer_to_string
libc.src.__support.float_to_string
- COMPILE_OPTIONS
- ${printf_copts}
)
|
libc/src/stdio/CMakeLists.txt
Outdated
| if(LLVM_LIBC_FULL_BUILD) | ||
| list(APPEND printf_deps | ||
| libc.src.__support.File.file | ||
| libc.src.__support.File.platform_file | ||
| libc.src.__support.File.platform_stdout | ||
| ) | ||
| else() | ||
| list(APPEND printf_copts "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE") | ||
| list(APPEND use_system_file "COMPILE_OPTIONS" "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE") |
There was a problem hiding this comment.
is there a benefit to moving the COMPILE_OPTIONS tag into the option? Also, I'm not sure this name is the best option since this option will only work for printf, but scanf will need a similar one.
There was a problem hiding this comment.
Yes - when there are no COMPILE_OPTIONS, then CMake will not treat whatever follows COMPILE_OPTIONS as an intended compile option.
About the name, do you mean LIBC_COPT_PRINTF_USE_SYSTEM_FILE? If yes, then you can either change it when you add similar feature to scanf. If you are talking about use_system_file, you can share it with scanf targets, or add a new one at that point.
sivachandra
force-pushed
the
propagate_copts
branch
from
301d739 to
2a23659
Compare
|
Rebased. @jhuber6 - Can you help with testing this on the GPU? |
|
I'm getting Any clue what that could be? This seems to work for NVPTX, presumably because we don't use static libraries and everything is an object library. |
sivachandra
force-pushed
the
propagate_copts
branch
from
2a23659 to
f6cf13b
Compare
Can you give it a try now? I am hoping the second commit should have fixed it. |
added 2 commits
September 26, 2023 15:15
…brary. printf_core.parser is not yet updated to use the printf config options; It does not use them currently anyway and the corresponding parser_test should be updated to respect the config options.
sivachandra
force-pushed
the
propagate_copts
branch
from
f6cf13b to
51eb706
Compare
legrosbuffle
pushed a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
…brary. (llvm#66979) printf_core.parser is not yet updated to use the printf config options. It does not use them currently anyway and the corresponding parser_test should be updated to respect the config options.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
printf_core.parser is not yet updated to use the printf config options; It
does not use them currently anyway and the corresponding parser_test
should be updated to respect the config options.