Skip to content

[flang][CMake] Add missing dependency to generate Fortran module files #91517

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 2 commits into from
May 9, 2024

Conversation

mjklemm
Copy link
Contributor

@mjklemm mjklemm commented May 8, 2024

Fixes bug #90769. Many thanks to @Meinersbur for providing the initial thought and solution to this.

@@ -432,7 +432,7 @@ if(runtimes)
# TODO: This is a workaround until flang becomes a first-class project
# in llvm/CMakeList.txt. Until then, this line ensures that flang-new is
# built before "openmp" is built as a runtime project.
list(APPEND extra_deps "flang-new")
list(APPEND extra_deps "flang-new" "module_files")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment on why the module_files is required here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

Was the issue present only in out-of-tree builds? How did this work correctly in in-tree builds without this change?

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.

@@ -432,7 +432,7 @@ if(runtimes)
# TODO: This is a workaround until flang becomes a first-class project
# in llvm/CMakeList.txt. Until then, this line ensures that flang-new is
# built before "openmp" is built as a runtime project.
list(APPEND extra_deps "flang-new")
list(APPEND extra_deps "flang-new" "module_files")
Copy link
Contributor

Choose a reason for hiding this comment

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

Was the issue present only in out-of-tree builds? How did this work correctly in in-tree builds without this change?

@mjklemm
Copy link
Contributor Author

mjklemm commented May 8, 2024

The issue was present in runtime builds of OpenMP, but was more like a race condition with some succesful builds and some failing. Esp. for stage2 builds it failed more consistently. I guess regular project build were not affected, as all targets had been build in order.

@mjklemm mjklemm merged commit 5adcfd4 into llvm:main May 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants