Skip to content

Add Cmake flag to ensure we're propagating thread_count#1102

Merged
reazulhoque merged 1 commit intobranch-24.01from
fix/propagate_thread_count_flag_cmake
Dec 8, 2023
Merged

Add Cmake flag to ensure we're propagating thread_count#1102
reazulhoque merged 1 commit intobranch-24.01from
fix/propagate_thread_count_flag_cmake

Conversation

@reazulhoque
Copy link
Copy Markdown
Contributor

Thread count is not being propagated to Cmake. This fixes that.

@copy-pr-bot
Copy link
Copy Markdown
Contributor

copy-pr-bot Bot commented Dec 6, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@magnatelee
Copy link
Copy Markdown
Contributor

@reazulhoque please add the sign key to your GitHub account. the policy is that we don't accept PRs with unverified or unsigned commits.

@magnatelee
Copy link
Copy Markdown
Contributor

also I don't recall we had any issue with the parallel build... can you describe what you're trying to fix with this PR?

@reazulhoque
Copy link
Copy Markdown
Contributor Author

@magnatelee We are propagating to cmake what users have provided with -j flag to install.py.

Without this fix user provided option will not be respected by cmake.

@reazulhoque reazulhoque force-pushed the fix/propagate_thread_count_flag_cmake branch from 5e800f0 to a6fedd6 Compare December 6, 2023 18:58
@manopapad
Copy link
Copy Markdown
Contributor

To be clear, install.py does pass -j directly to ninja/make, which is the last stage of the build, and covers all C++ compilation. We would have to pass this to cmake if we were using cmake --build rather than ninja/make directly. The problem is that compilation of the rust profiler expects to read -j from cmake.

@reazulhoque reazulhoque added the category:bug-fix PR is a bug fix and will be classified as such in release notes label Dec 7, 2023
Copy link
Copy Markdown
Contributor

@manopapad manopapad left a comment

Choose a reason for hiding this comment

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

If you can, please also open a corresponding PR on https://github.com/nv-legate/legate.core.

@reazulhoque reazulhoque merged commit 96ed9cd into branch-24.01 Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:bug-fix PR is a bug fix and will be classified as such in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants