Skip to content

[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

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

tstellar
Copy link
Collaborator

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

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]>
@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category labels Feb 16, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2025

@llvm/pr-subscribers-flang-runtime

Author: Tom Stellard (tstellar)

Changes

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


Full diff: https://github.com/llvm/llvm-project/pull/127364.diff

3 Files Affected:

  • (modified) flang/CMakeLists.txt (+12)
  • (modified) flang/cmake/modules/AddFlang.cmake (+1)
  • (modified) flang/runtime/CMakeLists.txt (+5)
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 "")

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a 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.

#
# 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)")
Copy link
Contributor

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.

#112789 (comment)

Separately, why doesn't this apply to Make?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented here:

Ninja only: List of available pools.

Ninja has job pools, GNU Make does not.

@Meinersbur
Copy link
Member

Meinersbur commented Feb 16, 2025

To elaborate on #112789 (comment):

I think this approach is a bigger footgun than the problem it wants to solve. If someone sets LLVM_RAM_PER_COMPILE_JOB=2000 (2GB), which seems to be the documented solution for running out of memory during compilation, of all the jobs, this PR will make that setting to be ignored for flang compile jobs.

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:

  1. Run ninja with Flang's dependencies: ninja clang FIRTransforms HLFIRTransforms MLIRDebug MLIROptLib opt llc mlir-translate mlir-opt -j12
  2. Run ninja for flang with limited parallelism: ninja bbc flang -j3
  3. Build everything else with normal parallelism: ninja -j12

Also consider pushing for better support in ninja:

  1. Prevent ninja starting new job when physical memory become low ninja-build/ninja#1571
  2. Ninja and RAM/Memory usage ninja-build/ninja#2187
  3. Better interactivity in low-memory situations on Linux desktop ninja-build/ninja#1706
  4. Allow a rule to consume jobs from multiple pools ninja-build/ninja#1471
  5. Support sub-pools that cap parallelism but also consume a job from their parent ninja-build/ninja#1351

@tstellar
Copy link
Collaborator Author

@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.

@Meinersbur
Copy link
Member

Meinersbur commented Feb 17, 2025

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:

  1. Two different pools for flang and other (compile) jobs
    a. LLVM_PARALLEL_COMPILE_JOBS * LLVM_RAM_PER_COMPILE_JOB + FLANG_PARALLEL_COMPILE_JOBS * FLANG_RAM_PER_COMPILE_JOB <= SYSTEM_RAM: Wasted parallelism while no flang jobs are running
    b. Otherwise: Risk of swapping/OOM
  2. Sharing the same pool for all jobs (e.g. FLANG_RAM_PER_COMPILE_JOB = 4 * LLVM_RAM_PER_COMPILE_JOB)
    a. LLVM_PARALLEL_COMPILE_JOBS * FLANG_RAM_PER_COMPILE_JOB <= SYSTEM_RAM: Wasted parallelism when few jobs actually use FLANG_RAM_PER_COMPILE_JOB
    b. Otherwise: Risk of swapping/OOM

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.

@tstellar
Copy link
Collaborator Author

@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.

Copy link
Member

@Meinersbur Meinersbur left a 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

  1. 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]>
Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:runtime flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants