-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[flang] Add FLANG_PARALLEL_COMPILE_JOBS option #127364
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
Conversation
This is a re-apply of 083c683 with a fix for the flang runtime build. This works the same way as LLVM_PARALLEL_COMPILE_JOBS except that it is specific to the flang source rather than for the whole project. Configuring with -DFLANG_PARALLEL_COMPILE_JOBS=1 would mean that there would only ever be one flang source being compiled at a time. Some of the flang sources require large amounts of memory to compile, so this option can be used to avoid OOM erros when compiling those files while still allowing the rest of the project to compile using the maximum number of jobs. Update flang/CMakeLists.txt Co-authored-by: Nikita Popov <[email protected]>
@llvm/pr-subscribers-flang-runtime Author: Tom Stellard (tstellar) ChangesThis is a re-apply of 083c683 with a fix for the flang runtime build. This works the same way as LLVM_PARALLEL_COMPILE_JOBS except that it is specific to the flang source rather than for the whole project. Configuring with -DFLANG_PARALLEL_COMPILE_JOBS=1 would mean that there would only ever be one flang source being compiled at a time. Some of the flang sources require large amounts of memory to compile, so this option can be used to avoid OOM erros when compiling those files while still allowing the rest of the project to compile using the maximum number of jobs. Update flang/CMakeLists.txt Full diff: https://github.com/llvm/llvm-project/pull/127364.diff 3 Files Affected:
diff --git a/flang/CMakeLists.txt b/flang/CMakeLists.txt
index 0f98d12343c43..421a997bf048e 100644
--- a/flang/CMakeLists.txt
+++ b/flang/CMakeLists.txt
@@ -473,6 +473,18 @@ if (APPLE)
endif()
endif()
+# Set up job pools for flang. Some of the flang sources take a lot of memory to
+# compile, so allow users to limit the number of parallel flang jobs. This is
+# useful for building flang alongside several other projects since you can use
+# the maximum number of build jobs for the other projects while limiting the
+# number of flang compile jobs.
+#
+# We want this set to infinity by default
+set(FLANG_PARALLEL_COMPILE_JOBS 0 CACHE STRING
+ "The maximum number of concurrent compilation jobs (Ninja only)")
+
+set_property(GLOBAL APPEND PROPERTY JOB_POOLS flang_compile_job_pool=${FLANG_PARALLEL_COMPILE_JOBS})
+
include(AddFlang)
if (FLANG_INCLUDE_TESTS)
diff --git a/flang/cmake/modules/AddFlang.cmake b/flang/cmake/modules/AddFlang.cmake
index badbd4e7b964b..b9fa6bb8c249f 100644
--- a/flang/cmake/modules/AddFlang.cmake
+++ b/flang/cmake/modules/AddFlang.cmake
@@ -94,6 +94,7 @@ function(add_flang_library name)
set_property(GLOBAL APPEND PROPERTY FLANG_LIBS ${name})
endif()
set_property(GLOBAL APPEND PROPERTY FLANG_EXPORTS ${name})
+ set_property(TARGET ${name} PROPERTY JOB_POOL_COMPILE flang_compile_job_pool)
else()
# Add empty "phony" target
add_custom_target(${name})
diff --git a/flang/runtime/CMakeLists.txt b/flang/runtime/CMakeLists.txt
index 1f4ee69598918..09846ad5bcd44 100644
--- a/flang/runtime/CMakeLists.txt
+++ b/flang/runtime/CMakeLists.txt
@@ -57,6 +57,11 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
REAL(16) is mapped to __float128, or libm for targets where REAL(16) \
is mapped to long double, etc."
)
+
+ # For the stand alone runtime builds, we need to define this job pool,
+ # because add_flang_library will use it. If we don't define it here, the
+ # build will fail.
+ set_property(GLOBAL APPEND PROPERTY JOB_POOLS flang_compile_job_pool=0)
endif()
set(linked_libraries "")
|
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.
Thanks for reviving this patch. One question about the default setting that was raised previously.
flang/CMakeLists.txt
Outdated
# | ||
# We want this set to infinity by default | ||
set(FLANG_PARALLEL_COMPILE_JOBS 0 CACHE STRING | ||
"The maximum number of concurrent compilation jobs (Ninja only)") |
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.
There was a comment by @Meinersbur that this default is not a good setting.
Separately, why doesn't this apply to Make?
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.
To elaborate on #112789 (comment): I think this approach is a bigger footgun than the problem it wants to solve. If someone sets Using multiple limited jobs pools is problematic by itself. Say we limit the number of flang jobs to 4 and other jobs to 8. That still means that up to 12 jobs can run concurrently, stealing each other's memory. Unfortunately its seems that it is not poosible for a job to belong to multiple pools, being limited by the most restricted of them. What I have been doing:
Also consider pushing for better support in ninja:
|
@Meinersbur What if we changed the default to be something like jobs = TotalMemory / (4 * LLVM_RAM_PER_COMPILE_JOB) ? I understand that any solution we have is not going to be ideal, but I think we need to do something, because having the build consistently fail due to OOM errors on some systems is not a good experience for users. Especially new users who may not know what is going on. |
Considering global user settings is the least we can do. In addition to LLVM_RAM_PER_COMPILE_JOB there is also LLVM_PARALLEL_COMPILE_JOBS. If user-friendlyness is the goal, we should pre-fill LLVM_RAM_PER_COMPILE_JOB for known compilers. This concrete PR adds just another trap for new users. However, with the current support for pools, there is a trade-off to make:
Neither of them really looks better than the other. We could instead guide new users to a standalone Flang build which can have a different LLVM_RAM_PER_COMPILE_JOB than the LLVM build. |
@Meinersbur Ok, I've updated the patch so that the flang job pools are only created if you use the FLANG_PARALLEL_COMPILE_JOBS option. So this means that implicitly, the default is LLVM_PARALLEL_COMPILE_JOBS since by default all the flang jobs are in the default compilation pool. And so this patch is now a no-op for existing build configurations. |
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.
Not doing anything unless FLANG_PARALLEL_COMPILE_JOBS
is specified is ok, but I don't think it is an universional solution 1.
LGTM
Footnotes
-
e.g. with FLANG_PARALLEL_COMPILE_JOBS=1 as the summary suggested, and me being a Flang developer editing mostly Flang files, will limit me to single thread compilation. I could change the setting depending on what I am compiling, but that is what
-j
exists for. ↩
Co-authored-by: Michael Kruse <[email protected]>
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.
LG
This is a re-apply of 083c683 with a fix for the flang runtime build.
This works the same way as LLVM_PARALLEL_COMPILE_JOBS except that it is specific to the flang source rather than for the whole project.
Configuring with -DFLANG_PARALLEL_COMPILE_JOBS=1 would mean that there would only ever be one flang source being compiled at a time.
Some of the flang sources require large amounts of memory to compile, so this option can be used to avoid OOM erros when compiling those files while still allowing the rest of the project to compile using the maximum number of jobs.
Update flang/CMakeLists.txt