[Flang] Fix omp_lib.h location and search path

[Meinersbur]

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 be 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.
2. Move the omp_lib.h install dir to ${RUNTIMES_INSTALL_RESOURCE_MOD_PATH}.
3. Remove the implicit search path include/flang/OpenMP from the frontend. There is nothing in there anyway.
4. 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.
5. 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.

[llvmorg-github-actions]

@llvm/pr-subscribers-flang-driver

Author: Michael Kruse (Meinersbur)

Changes

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 details 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.
2. Move the omp_lib.h install dir to ${RUNTIMES_INSTALL_RESOURCE_MOD_PATH}.
3. Remove the implicit search path include/flang/OpenMP from the frontend. There is nothing in there anyway.
4. 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.
5. 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.

---

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

4 Files Affected:

• (modified) flang/lib/Frontend/CompilerInvocation.cpp (-14)
• (modified) flang/test/Driver/include-omp-header.f90 (+32-25)
• (modified) flang/test/lit.cfg.py (-8)
• (modified) openmp/module/CMakeLists.txt (+3-3)

diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 9853fc600ff6a..0cacccc56fce8 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -929,15 +929,6 @@ static std::string getIntrinsicDir(const char *argv) {
   return std::string(driverPath);
 }
 
-// Generate the path to look for OpenMP headers
-static std::string getOpenMPHeadersDir(const char *argv) {
-  llvm::SmallString<128> includePath;
-  includePath.assign(llvm::sys::fs::getMainExecutable(argv, nullptr));
-  llvm::sys::path::remove_filename(includePath);
-  includePath.append("/../include/flang/OpenMP/");
-  return std::string(includePath);
-}
-
 /// Parses all preprocessor input arguments and populates the preprocessor
 /// options accordingly.
 ///
@@ -1795,11 +1786,6 @@ void CompilerInvocation::setDefaultFortranOpts() {
   std::vector<std::string> searchDirectories{"."s};
   fortranOptions.searchDirectories = searchDirectories;
 
-  // Add the location of omp_lib.h to the search directories. Currently this is
-  // identical to the modules' directory.
-  fortranOptions.searchDirectories.emplace_back(
-      getOpenMPHeadersDir(getArgv0()));
-
   fortranOptions.isFixedForm = false;
 }
 
diff --git a/flang/test/Driver/include-omp-header.f90 b/flang/test/Driver/include-omp-header.f90
index 7e54910a4b589..e3ffd376f9058 100644
--- a/flang/test/Driver/include-omp-header.f90
+++ b/flang/test/Driver/include-omp-header.f90
@@ -1,27 +1,34 @@
 ! REQUIRES: openmp_runtime
 
-! Verify that the omp_lib.h header is found and included correctly. This header file should be available at a path:
-!   * relative to the driver, that's
-!   * known the driver.
-! This is taken care of at the CMake and the driver levels. Note that when searching for header files, the directory of the current
-! source file takes precedence over other search paths. Hence adding omp_lib.h in the current directory will make Flang use that
-! header file instead of the one shipped with Flang.
-
-! This should just work
-! RUN: not rm omp_lib.h
-! RUN: %flang -cpp -fsyntax-only %openmp_flags %s  2>&1
-
-! Create an empty omp_lib.h header that _does not_ define omp_default_mem_alloc - this should lead to semantic errors
-! RUN: touch omp_lib.h
-! RUN: not %flang -cpp -fsyntax-only %openmp_flags %s  2>&1 | FileCheck %s
-! RUN: rm omp_lib.h
-
-! CHECK: error: Must have INTEGER type, but is REAL(4)
-
-include "omp_lib.h"
-
-integer :: x, y
-
-!$omp allocate(x, y) allocator(omp_default_mem_alloc)
-
-end
+! Check omp_lib.h works with driver
+! RUN: %flang -fsyntax-only -cpp %s -v 2>&1 | FileCheck %s --check-prefix=DRIVER
+! RUN: %flang -fsyntax-only -cpp %s -v -DHASHINCLUDE 2>&1 | FileCheck %s --check-prefix=DRIVER
+! DRIVER: -fc1
+! DRIVER-SAME: -fintrinsic-modules-path
+
+! Check frontend only works (no output expected)
+! RUN: %flang_fc1 -fsyntax-only -cpp %s
+! RUN: %flang_fc1 -fsyntax-only -cpp -DHASHINCLUDE %s
+
+! Check non-#include
+! RUN: %flang_fc1 -cpp %s -E -fno-reformat 2>&1 | FileCheck %s --check-prefix=INCLUDE
+! INCLUDE: include "omp_lib.h"
+
+! Check omp_lib.h contents
+! RUN: %flang_fc1 -cpp %s -E -fno-reformat -DHASHINCLUDE 2>&1 | FileCheck %s --check-prefix=PREPROCESSED
+! PREPROCESSED: integer(kind=omp_integer_kind)openmp_version
+! PREPROCESSED: parameter(openmp_version={{[0-9]+}})
+
+
+program main
+#ifdef HASHINCLUDE
+  #include "omp_lib.h"
+#else
+  include "omp_lib.h"
+#endif
+
+  integer :: x, y
+  !$omp allocate(x, y) allocator(omp_default_mem_alloc)
+
+  print *, 'PASS: openmp_version parameter ', openmp_version
+end program main
diff --git a/flang/test/lit.cfg.py b/flang/test/lit.cfg.py
index 92278a220cec0..3c6a33e010f59 100644
--- a/flang/test/lit.cfg.py
+++ b/flang/test/lit.cfg.py
@@ -187,14 +187,6 @@ def get_resource_module_intrinsic_dir(modfile):
 # Determine if OpenMP runtime was built (enable OpenMP tests via REQUIRES in test file)
 if config.flang_test_enable_openmp or openmp_mod_path:
     config.available_features.add("openmp_runtime")
-
-    # Search path for omp_lib.h with LLVM_ENABLE_RUNTIMES=openmp
-    # FIXME: In a bootstrapping build, openmp should write this file into a
-    #        shared directory
-    flang_extra_search_args += [
-        "-I",
-        f"{config.flang_obj_root}/../../runtimes/runtimes-bins/openmp/runtime/src",
-    ]
 else:
     lit_config.warning(
         f"OpenMP modules found not in driver default paths: OpenMP tests disabled; Use FLANG_TEST_ENABLE_OPENMP=ON to force-enable"
diff --git a/openmp/module/CMakeLists.txt b/openmp/module/CMakeLists.txt
index c0a873f9be552..0e06d84d3b2d6 100644
--- a/openmp/module/CMakeLists.txt
+++ b/openmp/module/CMakeLists.txt
@@ -9,7 +9,7 @@
 # Build the module files if a Fortran compiler is available.
 
 configure_file(omp_lib.F90.var "${CMAKE_CURRENT_BINARY_DIR}/omp_lib.F90" @ONLY)
-configure_file(omp_lib.h.var "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src/omp_lib.h" @ONLY)
+configure_file(omp_lib.h.var "${RUNTIMES_OUTPUT_RESOURCE_MOD_DIR}/omp_lib.h" @ONLY)
 
 # One compilation step creates both, omp_lib.mod and omp_lib_kinds.mod. Only
 # these files are used, the object file itself can be discarded.
@@ -30,8 +30,8 @@ flang_module_target(libomp-mod PUBLIC)
 add_dependencies(libomp-mod ${RUNTIMES_FORTRAN_BUILD_DEPS})
 
 install(FILES
-  "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src/omp_lib.h"
-  DESTINATION ${LIBOMP_HEADERS_INSTALL_PATH}
+  "${RUNTIMES_OUTPUT_RESOURCE_MOD_DIR}/omp_lib.h"
+  DESTINATION "${RUNTIMES_INSTALL_RESOURCE_MOD_PATH}"
     COMPONENT openmp
 )
 

[cenewcombe]

Thanks for the comprehensive fix and improved lit test, LGTM!

[tarunprabhu]

LGTM. Thanks.

[pawosm-arm]

@Meinersbur this is the F77 code which includes omp_lib.h that I've mentioned on the call today: https://people.math.sc.edu/Burkardt/f77_src/openmp/dot_product.f

[pawosm-arm]

@Meinersbur I've tried the most recent CI build, I can confirm that this PR fixes the problem of building dot_product.f.