-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Flang][OpenMP] Move builtin .mod generation into runtimes #137828
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
base: main
Are you sure you want to change the base?
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.
What's the main limitation here? If this is just a file dependency it should be identical to how all the OpenMP tests depend on omp.h
being in the resource directory. IMHO this is trivial if we do a runtimes build, since we can just require that openmp;flang-rt
are in the same toolchain, which then gives us well defined access to openmp
's CMake targets so long as it's listed before flang-rt
.
def fno_builtin_modules : Flag<["-"], "fno-builtin-modules">, | ||
Visibility<[FC1Option]>, | ||
HelpText<"Do not implicitly use builtin modules (for internal use 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.
Shouldn't this be implied by the user specifying it below?
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.
-fintrinsic-modules-path
is defined by gfortran, but flang's interpretation is incompatible (Even though they copy-pasted gfortran's help description). -fbuiltin-modules-path=
is the temporary replacement. At the end, I hope I will not need either of them, by fixing -fintrinsic-modules-path
.
path_list ModulePaths; | ||
path_list IntrinsicModulePaths; |
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.
Format.
clang/lib/Driver/ToolChain.cpp
Outdated
if (D.IsFlangMode()) { | ||
getIntrinsicModulePaths().append(getIntrinsicModulePaths()); | ||
} |
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.
if (D.IsFlangMode()) { | |
getIntrinsicModulePaths().append(getIntrinsicModulePaths()); | |
} | |
if (D.IsFlangMode()) | |
getIntrinsicModulePaths().append(getIntrinsicModulePaths()); |
While I appreciate the review, it is not yet in the state that warants one. It is still in an experimentation stage, so I did not yet care about formatting. There are also a lot of changes in here that will eventually not be needed. Goals are:
Sounds relatively simple, but there have been many small issues, starting with CMake's misspelling of CMAKE_Fortran_BUILDING_INSTRINSIC_MODULES. |
Just want to make sure: Should it be |
That is correct, I forgot the version number that is part of the resource directory. |
Experiments for per-target builtin modules