-
Notifications
You must be signed in to change notification settings - Fork 12.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shard the public llvm-config.h in multiple files (NFC) #71273
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! And I can confirm that enabling or disabling targets has a minimal rebuild impact if Ccache is enabled.
Something fishy: this change shouldn't change anything until we update users. |
Switching on & off the target does prompt ninja to rebuild everything, however, if a normal build takes in my machine 10 min, the rebuild of all targets took 2 min with Ccache. |
Right but that should be the case identically with and without this patch I think: do you see anything different? |
I just cleared the cache, cleaned the tree and retried with a clean build and you're right. |
42f4f5a
to
f12f923
Compare
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-backend-directx Author: Mehdi Amini (joker-eph) ChangesThis will allow for more granular include and improve incremental rebuild and caching when switching targets or other options independently. A single change of a target, default triple, or any option is triggering a monolithic rebuild of any user of a single of these options. We should make it as granular as possible. There are also option in llvm-config.h that seems to belong to config.h instead (that is it's not clear why they are part of the public LLVM distribution). For example the aspects relative to the "native" targets have to with the build machine and in a cross-compiler environment shouldn't be in the install package. Another follow-up should be to look into the private config.h and start updating its users Patch is 66.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71273.diff 52 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 7ff3acd48304de7..22da1e9d28af481 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -1063,13 +1063,22 @@ if (NOT TENSORFLOW_AOT_PATH STREQUAL "")
endif()
-# Configure the three LLVM configuration header files.
+# Configure the LLVM configurations header files.
configure_file(
${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/config.h.cmake
${LLVM_INCLUDE_DIR}/llvm/Config/config.h)
configure_file(
${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config.h.cmake
${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config.h)
+configure_file(
+ ${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config-options.h.cmake
+ ${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config-options.h)
+configure_file(
+ ${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config-targets.h.cmake
+ ${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config-targets.h)
+configure_file(
+ ${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config-version.h.cmake
+ ${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config-version.h)
configure_file(
${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/abi-breaking.h.cmake
${LLVM_INCLUDE_DIR}/llvm/Config/abi-breaking.h)
diff --git a/llvm/include/llvm/Config/llvm-config-build-llvm-dylib.h b/llvm/include/llvm/Config/llvm-config-build-llvm-dylib.h
new file mode 100644
index 000000000000000..89db92d7aee8cb0
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-build-llvm-dylib.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-build-llvm-dylib.h -----------*- 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_CONFIG_BUILD_LLVM_DYLIB_H
+#define LLVM_CONFIG_BUILD_LLVM_DYLIB_H
+
+/* Define if building libLLVM shared library */
+#cmakedefine01 LLVM_BUILD_LLVM_DYLIB
+
+#endif // LLVM_CONFIG_BUILD_LLVM_DYLIB_H
diff --git a/llvm/include/llvm/Config/llvm-config-build-shared-libs.h b/llvm/include/llvm/Config/llvm-config-build-shared-libs.h
new file mode 100644
index 000000000000000..8d66e29f2052ac9
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-build-shared-libs.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-build-shared-libs.h ----------*- 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_CONFIG_BUILD_SHARED_LIBS_H
+#define LLVM_CONFIG_BUILD_SHARED_LIBS_H
+
+/* Define if building LLVM with BUILD_SHARED_LIBS */
+#cmakedefine01 LLVM_BUILD_SHARED_LIBS
+
+#endif // LLVM_CONFIG_BUILD_SHARED_LIBS_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-curl.h b/llvm/include/llvm/Config/llvm-config-enable-curl.h
new file mode 100644
index 000000000000000..8a0ac685449a5aa
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-curl.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-curl.h ----------------*- 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_CONFIG_ENABLE_CURL_H
+#define LLVM_CONFIG_ENABLE_CURL_H
+
+/* Define if we have curl and want to use it */
+#cmakedefine01 LLVM_ENABLE_CURL
+
+#endif // LLVM_CONFIG_ENABLE_CURL_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-dia-sdk.h b/llvm/include/llvm/Config/llvm-config-enable-dia-sdk.h
new file mode 100644
index 000000000000000..f793e0c9a8c3f54
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-dia-sdk.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-dia-sdk.h -------------*- 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_CONFIG_ENABLE_DIA_SDK_H
+#define LLVM_CONFIG_ENABLE_DIA_SDK_H
+
+/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
+#cmakedefine01 LLVM_ENABLE_DIA_SDK
+
+#endif // LLVM_CONFIG_ENABLE_DIA_SDK_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-dump.h b/llvm/include/llvm/Config/llvm-config-enable-dump.h
new file mode 100644
index 000000000000000..fc1a3e1b07804c2
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-dump.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-dump.h ----------------*- 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_CONFIG_ENABLE_DUMP_H
+#define LLVM_CONFIG_ENABLE_DUMP_H
+
+/* Define if LLVM_ENABLE_DUMP is enabled */
+#cmakedefine01 LLVM_ENABLE_DUMP
+
+#endif // LLVM_CONFIG_ENABLE_DUMP_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-httplib.h b/llvm/include/llvm/Config/llvm-config-enable-httplib.h
new file mode 100644
index 000000000000000..af9dbbc6604519c
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-httplib.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-httplib.h --------------------*- 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_CONFIG_ENABLE_HTTPLIB_H
+#define LLVM_CONFIG_ENABLE_HTTPLIB_H
+
+/* Define if we have cpp-httplib and want to use it */
+#cmakedefine01 LLVM_ENABLE_HTTPLIB
+
+#endif // LLVM_CONFIG_ENABLE_HTTPLIB_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-plugins.h b/llvm/include/llvm/Config/llvm-config-enable-plugins.h
new file mode 100644
index 000000000000000..730458175687835
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-plugins.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-plugins.h -------------*- 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_CONFIG_ENABLE_PLUGINS_H
+#define LLVM_CONFIG_ENABLE_PLUGINS_H
+
+/* Define if plugins enabled */
+#cmakedefine01 LLVM_ENABLE_PLUGINS
+
+#endif // LLVM_CONFIG_ENABLE_PLUGINS_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-threads.h b/llvm/include/llvm/Config/llvm-config-enable-threads.h
new file mode 100644
index 000000000000000..56926fbcc477756
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-threads.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-threads.h --------------------*- 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_CONFIG_ENABLE_THREADS_H
+#define LLVM_CONFIG_ENABLE_THREADS_H
+
+/* Define if threads enabled */
+#cmakedefine01 LLVM_ENABLE_THREADS
+
+#endif // LLVM_CONFIG_ENABLE_THREADS_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-zlib.h b/llvm/include/llvm/Config/llvm-config-enable-zlib.h
new file mode 100644
index 000000000000000..ab4c4c471a105c1
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-zlib.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-zlib.h ----------------*- 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_CONFIG_ENABLE_ZLIB_H
+#define LLVM_CONFIG_ENABLE_ZLIB_H
+
+/* Define if zlib compression is available */
+#cmakedefine01 LLVM_ENABLE_ZLIB
+
+#endif // LLVM_CONFIG_ENABLE_ZLIB_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-zstd.h b/llvm/include/llvm/Config/llvm-config-enable-zstd.h
new file mode 100644
index 000000000000000..feadf37f064a8d7
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-zstd.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-zstd.h ----------------*- 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_CONFIG_ENABLE_ZSTD_H
+#define LLVM_CONFIG_ENABLE_ZSTD_H
+
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
+#endif // LLVM_CONFIG_ENABLE_ZSTD_H
diff --git a/llvm/include/llvm/Config/llvm-config-force-enable-stats.h b/llvm/include/llvm/Config/llvm-config-force-enable-stats.h
new file mode 100644
index 000000000000000..de37f26ec350aef
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-force-enable-stats.h
@@ -0,0 +1,18 @@
+/*===------- llvm/Config/llvm-config-force-enable-stats.h ---------*- 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_CONFIG_FORCE_ENABLE_STATS_H
+#define LLVM_CONFIG_FORCE_ENABLE_STATS_H
+
+/* Whether LLVM records statistics for use with GetStatistics(),
+ * PrintStatistics() or PrintStatisticsJSON()
+ */
+#cmakedefine01 LLVM_FORCE_ENABLE_STATS
+
+#endif // LLVM_CONFIG_FORCE_ENABLE_STATS_H
diff --git a/llvm/include/llvm/Config/llvm-config-force-use-old-toolchain.h b/llvm/include/llvm/Config/llvm-config-force-use-old-toolchain.h
new file mode 100644
index 000000000000000..c1b5b4894b572b3
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-force-use-old-toolchain.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-force-use-old-toolchain.h ----*- 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_CONFIG_FORCE_USE_OLD_TOOLCHAIN_H
+#define LLVM_CONFIG_FORCE_USE_OLD_TOOLCHAIN_H
+
+/* Define if building LLVM with LLVM_FORCE_USE_OLD_TOOLCHAIN_LIBS */
+#cmakedefine01 LLVM_FORCE_USE_OLD_TOOLCHAIN
+
+#endif // LLVM_CONFIG_FORCE_USE_OLD_TOOLCHAIN_H
diff --git a/llvm/include/llvm/Config/llvm-config-force-with-z3.h b/llvm/include/llvm/Config/llvm-config-force-with-z3.h
new file mode 100644
index 000000000000000..440856c2a973c73
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-force-with-z3.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-force-with-z3.h --------------*- 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_CONFIG_FORCE_WITH_Z3_H
+#define LLVM_CONFIG_FORCE_WITH_Z3_H
+
+/* Define if we have z3 and want to build it */
+#cmakedefine01 LLVM_FORCE_WITH_Z3
+
+#endif // LLVM_CONFIG_FORCE_WITH_Z3_H
diff --git a/llvm/include/llvm/Config/llvm-config-has-atomics.h b/llvm/include/llvm/Config/llvm-config-has-atomics.h
new file mode 100644
index 000000000000000..9b257d726f8ba6d
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-has-atomics.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-has-atomics.h ----------------*- 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_CONFIG_HAS_ATOMICS_H
+#define LLVM_CONFIG_HAS_ATOMICS_H
+
+/* Has gcc/MSVC atomic intrinsics */
+#cmakedefine01 LLVM_HAS_ATOMICS
+
+#endif // LLVM_CONFIG_HAS_ATOMICS_H
diff --git a/llvm/include/llvm/Config/llvm-config-have-sysexits.h b/llvm/include/llvm/Config/llvm-config-have-sysexits.h
new file mode 100644
index 000000000000000..a467bc1159a2edd
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-have-sysexits.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-have-sysexits.h --------------*- 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_CONFIG_HAVE_SYSEXITS_H_H
+#define LLVM_CONFIG_HAVE_SYSEXITS_H_H
+
+/* Define to 1 if you have the <sysexits.h> header file. */
+#cmakedefine01 LLVM_HAVE_SYSEXITS_H
+
+#endif // LLVM_CONFIG_HAVE_SYSEXITS_H_H
diff --git a/llvm/include/llvm/Config/llvm-config-have-tflite.h b/llvm/include/llvm/Config/llvm-config-have-tflite.h
new file mode 100644
index 000000000000000..3a61ccec627bcdf
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-have-tflite.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-have-tflite.h ----------------*- 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_CONFIG_HAVE_TFLITE_H
+#define LLVM_CONFIG_HAVE_TFLITE_H
+
+/* Define if LLVM is using tflite */
+#cmakedefine01 LLVM_HAVE_TFLITE
+
+#endif // LLVM_CONFIG_HAVE_TFLITE_H
diff --git a/llvm/include/llvm/Config/llvm-config-on-unix.cmake.h b/llvm/include/llvm/Config/llvm-config-on-unix.cmake.h
new file mode 100644
index 000000000000000..da6712c07895035
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-on-unix.cmake.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-on-unix.cmake.h --------------*- 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_CONFIG_ON_UNIX_H
+#define LLVM_CONFIG_ON_UNIX_H
+
+/* Define if this is Unixish platform */
+#cmakedefine01 LLVM_ON_UNIX
+
+#endif ...
[truncated]
|
@llvm/pr-subscribers-backend-amdgpu Author: Mehdi Amini (joker-eph) ChangesThis will allow for more granular include and improve incremental rebuild and caching when switching targets or other options independently. A single change of a target, default triple, or any option is triggering a monolithic rebuild of any user of a single of these options. We should make it as granular as possible. There are also option in llvm-config.h that seems to belong to config.h instead (that is it's not clear why they are part of the public LLVM distribution). For example the aspects relative to the "native" targets have to with the build machine and in a cross-compiler environment shouldn't be in the install package. Another follow-up should be to look into the private config.h and start updating its users Patch is 66.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71273.diff 52 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 7ff3acd48304de7..22da1e9d28af481 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -1063,13 +1063,22 @@ if (NOT TENSORFLOW_AOT_PATH STREQUAL "")
endif()
-# Configure the three LLVM configuration header files.
+# Configure the LLVM configurations header files.
configure_file(
${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/config.h.cmake
${LLVM_INCLUDE_DIR}/llvm/Config/config.h)
configure_file(
${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config.h.cmake
${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config.h)
+configure_file(
+ ${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config-options.h.cmake
+ ${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config-options.h)
+configure_file(
+ ${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config-targets.h.cmake
+ ${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config-targets.h)
+configure_file(
+ ${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/llvm-config-version.h.cmake
+ ${LLVM_INCLUDE_DIR}/llvm/Config/llvm-config-version.h)
configure_file(
${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/abi-breaking.h.cmake
${LLVM_INCLUDE_DIR}/llvm/Config/abi-breaking.h)
diff --git a/llvm/include/llvm/Config/llvm-config-build-llvm-dylib.h b/llvm/include/llvm/Config/llvm-config-build-llvm-dylib.h
new file mode 100644
index 000000000000000..89db92d7aee8cb0
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-build-llvm-dylib.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-build-llvm-dylib.h -----------*- 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_CONFIG_BUILD_LLVM_DYLIB_H
+#define LLVM_CONFIG_BUILD_LLVM_DYLIB_H
+
+/* Define if building libLLVM shared library */
+#cmakedefine01 LLVM_BUILD_LLVM_DYLIB
+
+#endif // LLVM_CONFIG_BUILD_LLVM_DYLIB_H
diff --git a/llvm/include/llvm/Config/llvm-config-build-shared-libs.h b/llvm/include/llvm/Config/llvm-config-build-shared-libs.h
new file mode 100644
index 000000000000000..8d66e29f2052ac9
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-build-shared-libs.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-build-shared-libs.h ----------*- 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_CONFIG_BUILD_SHARED_LIBS_H
+#define LLVM_CONFIG_BUILD_SHARED_LIBS_H
+
+/* Define if building LLVM with BUILD_SHARED_LIBS */
+#cmakedefine01 LLVM_BUILD_SHARED_LIBS
+
+#endif // LLVM_CONFIG_BUILD_SHARED_LIBS_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-curl.h b/llvm/include/llvm/Config/llvm-config-enable-curl.h
new file mode 100644
index 000000000000000..8a0ac685449a5aa
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-curl.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-curl.h ----------------*- 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_CONFIG_ENABLE_CURL_H
+#define LLVM_CONFIG_ENABLE_CURL_H
+
+/* Define if we have curl and want to use it */
+#cmakedefine01 LLVM_ENABLE_CURL
+
+#endif // LLVM_CONFIG_ENABLE_CURL_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-dia-sdk.h b/llvm/include/llvm/Config/llvm-config-enable-dia-sdk.h
new file mode 100644
index 000000000000000..f793e0c9a8c3f54
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-dia-sdk.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-dia-sdk.h -------------*- 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_CONFIG_ENABLE_DIA_SDK_H
+#define LLVM_CONFIG_ENABLE_DIA_SDK_H
+
+/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
+#cmakedefine01 LLVM_ENABLE_DIA_SDK
+
+#endif // LLVM_CONFIG_ENABLE_DIA_SDK_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-dump.h b/llvm/include/llvm/Config/llvm-config-enable-dump.h
new file mode 100644
index 000000000000000..fc1a3e1b07804c2
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-dump.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-dump.h ----------------*- 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_CONFIG_ENABLE_DUMP_H
+#define LLVM_CONFIG_ENABLE_DUMP_H
+
+/* Define if LLVM_ENABLE_DUMP is enabled */
+#cmakedefine01 LLVM_ENABLE_DUMP
+
+#endif // LLVM_CONFIG_ENABLE_DUMP_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-httplib.h b/llvm/include/llvm/Config/llvm-config-enable-httplib.h
new file mode 100644
index 000000000000000..af9dbbc6604519c
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-httplib.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-httplib.h --------------------*- 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_CONFIG_ENABLE_HTTPLIB_H
+#define LLVM_CONFIG_ENABLE_HTTPLIB_H
+
+/* Define if we have cpp-httplib and want to use it */
+#cmakedefine01 LLVM_ENABLE_HTTPLIB
+
+#endif // LLVM_CONFIG_ENABLE_HTTPLIB_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-plugins.h b/llvm/include/llvm/Config/llvm-config-enable-plugins.h
new file mode 100644
index 000000000000000..730458175687835
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-plugins.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-plugins.h -------------*- 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_CONFIG_ENABLE_PLUGINS_H
+#define LLVM_CONFIG_ENABLE_PLUGINS_H
+
+/* Define if plugins enabled */
+#cmakedefine01 LLVM_ENABLE_PLUGINS
+
+#endif // LLVM_CONFIG_ENABLE_PLUGINS_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-threads.h b/llvm/include/llvm/Config/llvm-config-enable-threads.h
new file mode 100644
index 000000000000000..56926fbcc477756
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-threads.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-threads.h --------------------*- 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_CONFIG_ENABLE_THREADS_H
+#define LLVM_CONFIG_ENABLE_THREADS_H
+
+/* Define if threads enabled */
+#cmakedefine01 LLVM_ENABLE_THREADS
+
+#endif // LLVM_CONFIG_ENABLE_THREADS_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-zlib.h b/llvm/include/llvm/Config/llvm-config-enable-zlib.h
new file mode 100644
index 000000000000000..ab4c4c471a105c1
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-zlib.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-zlib.h ----------------*- 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_CONFIG_ENABLE_ZLIB_H
+#define LLVM_CONFIG_ENABLE_ZLIB_H
+
+/* Define if zlib compression is available */
+#cmakedefine01 LLVM_ENABLE_ZLIB
+
+#endif // LLVM_CONFIG_ENABLE_ZLIB_H
diff --git a/llvm/include/llvm/Config/llvm-config-enable-zstd.h b/llvm/include/llvm/Config/llvm-config-enable-zstd.h
new file mode 100644
index 000000000000000..feadf37f064a8d7
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-enable-zstd.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-enable-zstd.h ----------------*- 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_CONFIG_ENABLE_ZSTD_H
+#define LLVM_CONFIG_ENABLE_ZSTD_H
+
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
+#endif // LLVM_CONFIG_ENABLE_ZSTD_H
diff --git a/llvm/include/llvm/Config/llvm-config-force-enable-stats.h b/llvm/include/llvm/Config/llvm-config-force-enable-stats.h
new file mode 100644
index 000000000000000..de37f26ec350aef
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-force-enable-stats.h
@@ -0,0 +1,18 @@
+/*===------- llvm/Config/llvm-config-force-enable-stats.h ---------*- 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_CONFIG_FORCE_ENABLE_STATS_H
+#define LLVM_CONFIG_FORCE_ENABLE_STATS_H
+
+/* Whether LLVM records statistics for use with GetStatistics(),
+ * PrintStatistics() or PrintStatisticsJSON()
+ */
+#cmakedefine01 LLVM_FORCE_ENABLE_STATS
+
+#endif // LLVM_CONFIG_FORCE_ENABLE_STATS_H
diff --git a/llvm/include/llvm/Config/llvm-config-force-use-old-toolchain.h b/llvm/include/llvm/Config/llvm-config-force-use-old-toolchain.h
new file mode 100644
index 000000000000000..c1b5b4894b572b3
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-force-use-old-toolchain.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-force-use-old-toolchain.h ----*- 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_CONFIG_FORCE_USE_OLD_TOOLCHAIN_H
+#define LLVM_CONFIG_FORCE_USE_OLD_TOOLCHAIN_H
+
+/* Define if building LLVM with LLVM_FORCE_USE_OLD_TOOLCHAIN_LIBS */
+#cmakedefine01 LLVM_FORCE_USE_OLD_TOOLCHAIN
+
+#endif // LLVM_CONFIG_FORCE_USE_OLD_TOOLCHAIN_H
diff --git a/llvm/include/llvm/Config/llvm-config-force-with-z3.h b/llvm/include/llvm/Config/llvm-config-force-with-z3.h
new file mode 100644
index 000000000000000..440856c2a973c73
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-force-with-z3.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-force-with-z3.h --------------*- 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_CONFIG_FORCE_WITH_Z3_H
+#define LLVM_CONFIG_FORCE_WITH_Z3_H
+
+/* Define if we have z3 and want to build it */
+#cmakedefine01 LLVM_FORCE_WITH_Z3
+
+#endif // LLVM_CONFIG_FORCE_WITH_Z3_H
diff --git a/llvm/include/llvm/Config/llvm-config-has-atomics.h b/llvm/include/llvm/Config/llvm-config-has-atomics.h
new file mode 100644
index 000000000000000..9b257d726f8ba6d
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-has-atomics.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-has-atomics.h ----------------*- 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_CONFIG_HAS_ATOMICS_H
+#define LLVM_CONFIG_HAS_ATOMICS_H
+
+/* Has gcc/MSVC atomic intrinsics */
+#cmakedefine01 LLVM_HAS_ATOMICS
+
+#endif // LLVM_CONFIG_HAS_ATOMICS_H
diff --git a/llvm/include/llvm/Config/llvm-config-have-sysexits.h b/llvm/include/llvm/Config/llvm-config-have-sysexits.h
new file mode 100644
index 000000000000000..a467bc1159a2edd
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-have-sysexits.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-have-sysexits.h --------------*- 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_CONFIG_HAVE_SYSEXITS_H_H
+#define LLVM_CONFIG_HAVE_SYSEXITS_H_H
+
+/* Define to 1 if you have the <sysexits.h> header file. */
+#cmakedefine01 LLVM_HAVE_SYSEXITS_H
+
+#endif // LLVM_CONFIG_HAVE_SYSEXITS_H_H
diff --git a/llvm/include/llvm/Config/llvm-config-have-tflite.h b/llvm/include/llvm/Config/llvm-config-have-tflite.h
new file mode 100644
index 000000000000000..3a61ccec627bcdf
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-have-tflite.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-have-tflite.h ----------------*- 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_CONFIG_HAVE_TFLITE_H
+#define LLVM_CONFIG_HAVE_TFLITE_H
+
+/* Define if LLVM is using tflite */
+#cmakedefine01 LLVM_HAVE_TFLITE
+
+#endif // LLVM_CONFIG_HAVE_TFLITE_H
diff --git a/llvm/include/llvm/Config/llvm-config-on-unix.cmake.h b/llvm/include/llvm/Config/llvm-config-on-unix.cmake.h
new file mode 100644
index 000000000000000..da6712c07895035
--- /dev/null
+++ b/llvm/include/llvm/Config/llvm-config-on-unix.cmake.h
@@ -0,0 +1,16 @@
+/*===------- llvm/Config/llvm-config-on-unix.cmake.h --------------*- 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_CONFIG_ON_UNIX_H
+#define LLVM_CONFIG_ON_UNIX_H
+
+/* Define if this is Unixish platform */
+#cmakedefine01 LLVM_ON_UNIX
+
+#endif ...
[truncated]
|
dc07dbd
to
e04d954
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
e04d954
to
a0c8737
Compare
@nico @fabianmcg : I ended up biting the bullet and do an almost full sharding of the file, PTAL. I also removed the new options I introduced yesterday for testing is a target is enabled from the llvm-config.h: since this file only exists for backward compatiblity now and these options haven't been widely adopted. |
4ec71c6
to
ed05b4c
Compare
a9540dc
to
861cf6a
Compare
This will allow for more granular include and improve incremental rebuild and caching when switching targets or other options independently. A single change of a target, default triple, or any option is triggering a monolithic rebuild of any user of a single of these options. We should make it as granular as possible. There are also option in llvm-config.h that seems to belong to config.h instead (that is it's not clear why they are part of the public LLVM distribution). For example the aspects relative to the "native" targets have to with the build machine and in a cross-compiler environment shouldn't be in the install package. Other candidates includes LLVM_ENABLE_DIA_SDK, HAVE_SYSEXITS_H, LLVM_ON_UNIX.
861cf6a
to
45f4f96
Compare
#define LLVM_CONFIG_ENABLE_CURL_H | ||
|
||
/* Define if we have curl and want to use it */ | ||
#cmakedefine LLVM_ENABLE_CURL ${LLVM_ENABLE_CURL} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I worry that this split is going to cause subtle bugs if we don't use cmakedefine01 everywhere. Otherwise we could end up with #ifndef being evaluated incorrectly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment we keep including everything through the llvm-config.h ; but you're right this is a major hurdle.
I'm already going one by one through all the options and converting them to cmakedefine01.
I am also making the LLVM codebase -Wundef
clean: this is the only way to catch this in a principle way (I think we should enable -Wundef by default actually).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a long flight yesterday, I have locally all the patches for making every option a cmakedefine01
and making the codebase -Wundef clean.
Having one file per setting for settings toggled by autoconf ( |
If we do this, what's the advantage of putting all these headers in Config/? I think this is a noble patch, but doing this piecemeal for the ones people actually toggle is imho good enough. (I would love to be able to turn assertions on and off without a full rebuild, but even if that's in its own .h, assertions are used anywhere, so it's still a full rebuild even then :P) And putting the archs header over in mlir so that we can't end up with code that has ifdef checks for archs also seems nice from a a "keep llvm code cleaner" PoV. |
In Config/ there is
I started to do that and I realized that there were only a couple of options like that (HAVE_SYSEXIT and HAS_ATOMICS I think), I can group them into "llvm-config-platform.h.cmake" if you prefer. Right now
(right it is affecting virtually everything, even then we don't define NDEBUG ourselves anyway, and we don't have a macro for our own assertions I believe) |
I worry about the tradeoff here. How much does this really speed up builds with a caching layer when you change these settings? I feel like in a lot of cases, the caching layer should already be resilient against macros that aren't used because the caches tend to identify collisions based on preprocessed files, so if you don't use a macro you should still get a cache hit. This might bypass cache lookups, but it comes at a cost to maintainability. |
What is the maintainability concern? I don’t quite get what you have in mind on this. |
Adding new files for new options and splitting defines across a bunch of files is messy and will incur maintenance as people have a whole bunch of files floating around. Which isn't to say we shouldn't do this, but have you benchmarked how much this can improve build times? This is a lot of complexity to add to our includes and build settings system if it doesn't have a significant benefit. |
I don't quite get what you mean: what are "floating" files? I'm trying to picture it, but it's very fuzzy, I prompted StableDiffusion to get help ;)
Sorry but I see zero added complexity really right now. The whole thing seems just to me to be a cleaner software structure where you include what you need (include llvm-config-version.h for the VERSION* macros). Not clear why the current "monolithic blog" would be less messy and less complex? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using cmakedefine01 is an improvement. As far as splitting every option into its own header, I've never wanted to dynamically flip between these. These mostly look like the kind of options I'd be surprised would even work to change after setting once in the CMakeCache.
Isn't there an additional cost to reading N smaller files?
When debugging builds, I sometimes look at |
@joker-eph I think the important point here is that you need to justify why this is an improvement. Do you have performance numbers to show that this speeds up builds in some configuration? We can argue about subjectives all day and night, but if you have data to justify making this change it completely shifts the discussion. |
Well I am claiming this is a cleanup: which is subjective, but just as much as a lot of software engineer is (hey you started with a lot of "subjective" claims!). Can make it less subjective by claiming separation of concerns and modularity: which is actually measurable.
Note that this patch alone does not change anything: because I need to update all the users first and stop including llvm-config.h, which is a series of follow-up cleanup patches. But the impacts seemed obvious to me, let me actually give numbers, it is fairly easily collect them:
For example, switching the LLVM_ENABLE_CURL ON/OFF config, on one build config (not including flang/clang) it means 5126 targets to rebuild before, and only 20 after (this is what I observe with |
Your initial comment referred to caching. If you apply This continues to lead me to the conclusion that I'm not convinced this change provides enough benefit to justify complicating our configuration include paradigm. Having a single (or small number) of headers that are autoconf or cmake generated is fairly common, having one header per configuration setting is not something I've ever seen a project do. |
|
No: I wrote "incremental rebuild and caching".
Both have support for "direct mode" where preprocessing can be bypassed, the impact can be going from 7 to 11min (or 60s to 110s), see discussion here: llvm/llvm-zorg#68 (comment)
I don't see the "complicating" aspect really, but also I'm a bit annoyed by your seemingly goal shifting approach to the review and tactic of being vague and evasive on your comments to ultimately get me to waste time on experiment that were obvious. Please don't do that, this isn't productive. |
I prefer not to engage in the debate and hope we can focus on the technical aspects instead. Splitting a config header into multiple files strikes me as somewhat unusual. I don't frequently update As for other configurations like curl,zstd,threads,etc, I rarely make updates, and I am okay with the rebuild time if I do decide to change them. |
To make progress here, can we please just put the macros added in #71164 into their own header (which can live in mlir/, the only consumer of these values) and not do the rest for now? |
Sorry but I don't think what you're proposing is a good path forward. And I strongly object to making this an MLIR specific thing: it's not. |
If MLIR is the only consumer of those values reducing the exposure area of them seems like a good idea. I didn't see the earlier PR go in, but I don't think those values should have been exposed to LLVM in the first place. Exposing target availability preprocessor macros is a significant shift for LLVM and could lead to violating LLVM's library layering. LLVM has a longstanding convention that code dependent on a target being enabled is contained to the target itself. It seems to me that sharding the configuration files as this PR does isn't something the community has a clear agreement on. Further, the people with the most experience maintaining the LLVM build system (which this change impacts) are voicing opposition. |
This will allow for more granular include and improve incremental rebuild and caching when switching targets or other options independently.
A single change of a target, default triple, or any option is triggering a monolithic rebuild of any user of a single of these options. We should make it as granular as possible.
As a follow-up we'll update the user of llvm-config.h incrementally.
Another follow-up should be to look into the private config.h the same way.