[Flang] Move builtin .mod generation into runtimes#679
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
0f93940 to
a41ffe8
Compare
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
a41ffe8 to
4a8fef3
Compare
|
@ronlieb Merged-in llvm@7675fc7. This includes all the patches since the last merge. Had to resolve two merge conflicts unrelated to the PR:
The test 'flang/test/Integration/amdgpu/debug-target-var.f90' is failing. I think it is unrelated to llvm#169638 These were automatically resolved by kdiff3:
Had to add For aomp to build the amgdcn-amd-amdhsa modules, ROCm/aomp#1737 needs to be applied. |
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
80634ae to
021eaa2
Compare
Meinersbur
requested review from
b-sumner,
chinmaydd,
david-salinas and
lamb-j
as code owners
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
82abb14 to
4b2ee45
Compare
Meinersbur
requested review from
fabianmcg,
ftynse,
stellaraccident and
vangthao95
as code owners
|
This PR is intended @ronlieb to resolve a merge conflict of amd-staging with upstream, it is not intended for an additional review, but GitHub seems to have automatically added reviewers. Sorry for the noise. |
Meinersbur
requested review from
ronlieb
and removed request for
b-sumner,
chinmaydd,
david-salinas,
fabianmcg,
lamb-j and
vangthao95
…eapply llvm#137828) (llvm#171515)"" This reverts commit ce78ca1.
Before this PR, omp_lib.h is emitted to `${PREFIX}/include` or
`${PREFIX}/lib/clang/<version>/include` (install prefix) and
`${PREFIX}/runtime/src/omp_lib.h` (builddir prefix). It is never found
there because the driver only adds `${PREFIX}/include/flang/OpenMP` to
the include path.
Fix the `omp_lib.h` include by using the same mechanism as the
omp_lib.mod; that is, move it to
`${PREFIX}/lib/clang/<version>/finclude/flang/<target-triple>`. The
search path is already added by the driver via
`-fintrinsics-modules-path` by the driver. Although omp_lib.h currently
does not contain anything target-specific, it could do so in the future
and I don't think it is worth the effort to add a mechanism without the
target triple. It should also me consistent with omp_lib.mod.
The changes in detail consist of:
1. Move the omp_lib.h output in the builddir to
`${RUNTIMES_OUTPUT_RESOURCE_MOD_DIR}`. This already takes care of
whether it is a bootstrapping build or not.
3. Move the omp_lib.h install dir to
`${RUNTIMES_INSTALL_RESOURCE_MOD_PATH}`.
4. Remove the implicit search path `include/flang/OpenMP` from the
frontend. There is nothing in there anyway.
5. Hardcoding the include search path for testing to the
`LLVM_RUNTIME_TARGETS=default` build becomes unnecessary. This was way
the `include-omp-header.f90` test was still passing, although it would
not work outside the regression tests. Essentially, it tested
lit.site.cfg, not the driver.
6. Replace the old `include-omp-header.f90` test. It created a temporary
`omp_lib.h` that interferes with any other test that use `include
omp_lib.h` (there currently isn't but I originally intended to add the
replacement as an additional omp_lib.h test which resulted in a flaky
test while the original test was there). Due to how the %flang(_fc1)
substitution works, lookup order may also vary.
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
964b66d to
77b8814
Compare
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
3873b24 to
64b690d
Compare
…/rocm_flang_builtin-mods
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
5c80b71 to
826eb63
Compare
Meinersbur
force-pushed
the
users/meinersbur/rocm_flang_builtin-mods
branch
from
826eb63 to
ebbfdf3
Compare
3 participants
llvm#169638 pre-merged into amd-staging