[flang][driver] Add -fintrinsic-modules-path option

[arnamoy10]

Summary

Add support for the following f18 options:

-fintrinsic-modules-path

Also the following changes:

1. Create a new test case
2. Adding dummy module files in the test directory

Depends on : D98522

[arnamoy10]

Updating the test case to include -fc1 as well

[awarzynski]

Hi @arnamoy10 , thanks for the patch!

Both ieee_arithmetic.mod and iso_fortran_env.mod look like copies of similar files from <build-dir>/tools/flang/include/flang. Could you either re-use those or add something custom?

clang/include/clang/Driver/Options.td

4341

Why not copy this from gfortran?

$ gfortran --help=separate | grep intrinsic
  -fintrinsic-modules-path    Specify where to find the compiled intrinsic modules.

flang/include/flang/Frontend/PreprocessorOptions.h

31–33

As -I and -fintrinsic-modules-path are 2 different options, I would prefer to preserve that separation and model it within the driver with 2 separate vars, e.g.:

std::vector<std::string> searchDirectoriesFromDashI;
std::vector<std::string> searchDirectoriesFromIntrModPath;

48

Thank you :)

[arnamoy10]

Addressing comments, by separating the search directories from fintrinsic-modules-path in a separate variables. Also added dummy modules for the test case, as using %llvmshlibdir did not work out.

Also, not sure what to set as the default directory, as gfortran uses a specific installation location.

[awarzynski]

In D97080#2586289, @arnamoy10 wrote:

Also, not sure what to set as the default directory, as gfortran uses a specific installation location.

We probably should use a location relative to the flang-new binary, i.e. <flang-new-path>/../tools/flang/include/flang/.

Question: What are the semantics for this flag in gfortran? Is the path specified with -fintrinsics-module-path _prepended_ or _appended_ to the default search path?

[awarzynski]

In D97080#2613628, @awarzynski wrote:

Question: What are the semantics for this flag in gfortran? Is the path specified with -fintrinsics-module-path _prepended_ or _appended_ to the default search path?

I believe that it is _prepended_. Take this dummy module:

module ieee_arithmetic
type::ieee_round_type
integer(1),private::mode=0_2
end type
end

Compile it with e.g. flang-new:

flang-new -fc1 -fsyntax-only dummy-module.f90

Next, take this Fortran file:

program test
  use ieee_arithmetic
  implicit none
  real(kind=4) :: x1
  if (.not. ieee_support_nan(x1)) STOP 20
end program test

And try compiling it with gfortran:

mv ieee_arithmetic.mod tools/flang/
gfortran -fintrinsic-modules-path tools/flang/ test.f90
test.f90:2:6:

   use ieee_arithmetic
      1
Fatal Error: File ‘ieee_arithmetic.mod’ opened at (1) is not a GNU Fortran module file
compilation terminated.

So gfortran did look in tools/flang first, i.e. the path specified with -fintrinsic-modules-path.

[arnamoy10]

Thank you so much for this experiment @awarzynski . I will modify the code accordingly.

[arnamoy10]

1. Updated to set the default search directory for the intrinsic module
2. Modified the test case to check the prepend behavior

[arnamoy10]

Clang-formatting

[awarzynski]

Thank you for updating this @arnamoy10 !

flang/lib/Frontend/CompilerInvocation.cpp

28

Not needed

293

This will not work on Windows, will it? I couldn't find any API to deal with this in a OS-agnostic way. Perhaps just remove 9 characters at the end? Or use #ifndef _WIN32?

flang/test/Driver/Inputs/ieee_arithmetic.mod

2

I think that currently the contents of this file are not relevant. So I would limit it to some helpful comment, e.g.:

Added for testing purposes. The contents of this file are currently not relevant.

[arnamoy10]

Modifying the algo of default search path extraction for supporting Windows platforms.

[tskeith]

flang/lib/Frontend/CompilerInvocation.cpp

295

Does this work for an install? I think there the path would be include/flang rather than tools/flang/include/flang.

[arnamoy10]

flang/lib/Frontend/CompilerInvocation.cpp

295

You are probably right, I am giving the path w.r.t a build. Can we make the assumption that there should be always an install? Or do we determine if we flang-new is coming from build or install (by checking if a module file is present through ls) and then set the path accordingly?

[tskeith]

flang/lib/Frontend/CompilerInvocation.cpp

295

I think it should work in a build or an install. The flang/tools/f18/flang script checks for include/flang first and if that doesn't exist it uses tools/flang/include/flang so you could do the same. It would be good if we could fix the build or install so that the location is consistent.

[tskeith]

flang/lib/Frontend/CompilerInvocation.cpp

295

I've created D98522 to make the relative path be include/flang in both build and install. If no one objects, that should make it so you don't need conditional code here, just use "/../include/flang".

[arnamoy10]

flang/lib/Frontend/CompilerInvocation.cpp

295

Thanks for doing this

[arnamoy10]

Update the path based on the patch D98522

[bryanpkc]

flang/lib/Frontend/CompilerInvocation.cpp

294

Can you use llvm::sys::path::remove_filename here?

[awarzynski]

I tried reproducing the CI failures reported at the top (in particular intrinsic_module_path.f90), but haven't had much luck. @arnamoy10 - how about you?

Could you clang-format this patch when uploading the next diff? Thank you!

flang/lib/Frontend/CompilerInvocation.cpp

323

This TODO has been addressed, right?

327

This method is meant for parsing input parameters, but the default directory for intrinsic modules is not a parameter. I think that setFortranOpts would be a better place for adding the default directory for fortranOptions.searchDirectories.

[arnamoy10]

Clang-formatting and addressing reviewers' comments:

Moved the default directory append to setFortranOpts()

[arnamoy10]

Adding the dummy modules

[arnamoy10]

Adding the test file

[arnamoy10]

Fixing bug in test case

[arnamoy10]

flang/lib/Frontend/CompilerInvocation.cpp

294

Thank you. This seems like a better option, but I could not make it work. For the sake of time I am keeping this hard coded manipulation. It can be improved in a later revision.

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

294

It would be really nice to use llvm::sys::path::remove_filename instead. Perhaps something like this:

llvm::SmallString<128> driverPath;
driverPath.assign(llvm::sys::fs::getMainExecutable(nullptr, nullptr));
llvm::sys::path::remove_filename(driverPath);

?

[arnamoy10]

Using llvm::sys::path::remove_filename as per suggestion.

flang/lib/Frontend/CompilerInvocation.cpp

294

This works, thanks for the code!

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

297

Given this conversion operator, wouldn't return std::string(driverPath); be more efficient?

[arnamoy10]

Addressing reviewers' comments

flang/lib/Frontend/CompilerInvocation.cpp

297

Done, thanks

[awarzynski]

LGTM, thank you for working on this!

In the summary you refer to -fdebug-module-writer. Could you please update before merging this?

flang/lib/Frontend/CompilerInvocation.cpp

28

Please revert before merging.

322

[nit] To me, currently hardcoded suggests that it's a temporary solution. But that's not the case, is it? Also, the default location is relative to the location of the driver, so it's not hardcoded.

[arnamoy10]

Addressing minor issues

clang/include/clang/Driver/Options.td

 defm dump_fortran_optimized : BooleanFFlag<"dump-fortran-optimized">, Group<gfortran_Group>;
 defm dump_fortran_original : BooleanFFlag<"dump-fortran-original">, Group<gfortran_Group>;
 defm dump_parse_tree : BooleanFFlag<"dump-parse-tree">, Group<gfortran_Group>;
 defm external_blas : BooleanFFlag<"external-blas">, Group<gfortran_Group>;
 defm f2c : BooleanFFlag<"f2c">, Group<gfortran_Group>;
 defm frontend_optimize : BooleanFFlag<"frontend-optimize">, Group<gfortran_Group>;
 defm init_local_zero : BooleanFFlag<"init-local-zero">, Group<gfortran_Group>;
 defm integer_4_integer_8 : BooleanFFlag<"integer-4-integer-8">, Group<gfortran_Group>;
-defm intrinsic_modules_path : BooleanFFlag<"intrinsic-modules-path">, Group<gfortran_Group>;
 defm max_identifier_length : BooleanFFlag<"max-identifier-length">, Group<gfortran_Group>;
 defm module_private : BooleanFFlag<"module-private">, Group<gfortran_Group>;
 defm pack_derived : BooleanFFlag<"pack-derived">, Group<gfortran_Group>;
 defm protect_parens : BooleanFFlag<"protect-parens">, Group<gfortran_Group>;
 defm range_check : BooleanFFlag<"range-check">, Group<gfortran_Group>;
 defm real_4_real_10 : BooleanFFlag<"real-4-real-10">, Group<gfortran_Group>;
 defm real_4_real_16 : BooleanFFlag<"real-4-real-16">, Group<gfortran_Group>;
 defm real_4_real_8 : BooleanFFlag<"real-4-real-8">, Group<gfortran_Group>;

 def flogical_abbreviations : Flag<["-"], "flogical-abbreviations">, Group<f_Group>,
   HelpText<"Enable logical abbreviations">;
 def fno_logical_abbreviations : Flag<["-"], "fno-logical-abbreviations">, Group<f_Group>;
 def fimplicit_none : Flag<["-"], "fimplicit-none">, Group<f_Group>,
   HelpText<"No implicit typing allowed unless overridden by IMPLICIT statements">;
 def fno_implicit_none : Flag<["-"], "fno-implicit-none">, Group<f_Group>;
 def falternative_parameter_statement : Flag<["-"], "falternative-parameter-statement">, Group<f_Group>,
   HelpText<"Enable the old style PARAMETER statement">;
+def fintrinsic_modules_path : Separate<["-"], "fintrinsic-modules-path">,  Group<f_Group>, MetaVarName<"<dir>">,
+  HelpText<"Specify where to find the compiled intrinsic modules">,
+  DocBrief<[{This option specifies the location of pre-compiled intrinsic modules, 
+  if they are not in the default location expected by the compiler.}]>;
 }
 
 def J : JoinedOrSeparate<["-"], "J">,
   Flags<[RenderJoined, FlangOption, FC1Option, FlangOnlyOption]>,
   Group<gfortran_Group>,
   Alias<module_dir>;
 
 //===----------------------------------------------------------------------===//

clang/lib/Driver/ToolChains/Flang.cpp

 
 void Flang::AddPreprocessingOptions(const ArgList &Args,
                                     ArgStringList &CmdArgs) const {
   Args.AddAllArgs(CmdArgs, {options::OPT_D, options::OPT_U, options::OPT_I});
 }
 
 void Flang::AddOtherOptions(const ArgList &Args, ArgStringList &CmdArgs) const {
   Args.AddAllArgs(CmdArgs,
-                  {options::OPT_module_dir, options::OPT_fdebug_module_writer});
+                  {options::OPT_module_dir, options::OPT_fdebug_module_writer,
+                   options::OPT_fintrinsic_modules_path});
 }
 
 void Flang::ConstructJob(Compilation &C, const JobAction &JA,
                          const InputInfo &Output, const InputInfoList &Inputs,
                          const ArgList &Args, const char *LinkingOutput) const {
   const auto &TC = getToolChain();
   // TODO: Once code-generation is available, this will need to be commented
   // out.

flang/include/flang/Frontend/PreprocessorOptions.h

 /// in preprocessor initialization to the parser options.
 class PreprocessorOptions {
 public:
   std::vector<std::pair<std::string, /*isUndef*/ bool>> macros;
   // Search directories specified by the user with -I
   // TODO: When adding support for more options related to search paths,
   // consider collecting them in a separate aggregate. For now we keep it here
   // as there is no point creating a class for just one field.
   std::vector<std::string> searchDirectoriesFromDashI;
+  // Search directories specified by the user with -fintrinsic-modules-path
+  std::vector<std::string> searchDirectoriesFromIntrModPath;
 
 public:
   PreprocessorOptions() {}
 
   void addMacroDef(llvm::StringRef name) {
     macros.emplace_back(std::string(name), false);
   }
 
   void addMacroUndef(llvm::StringRef name) {
     macros.emplace_back(std::string(name), true);
   }
 };
 
 } // namespace Fortran::frontend
 
-#endif // LLVM_FLANG_PREPROCESSOROPTIONS_H
+#endif // LLVM_FLANG_PREPROCESSOROPTIONS_H
 

flang/lib/Frontend/CompilerInvocation.cpp

 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
 
 using namespace Fortran::frontend;
 
 //===----------------------------------------------------------------------===//
 // Initialization.
 //===----------------------------------------------------------------------===//
 CompilerInvocationBase::CompilerInvocationBase()
     : diagnosticOpts_(new clang::DiagnosticOptions()),
if (const llvm::opt::Arg *arg =
     }
   }
 
   setUpFrontendBasedOnAction(opts);
 
   return dashX;
 }
 
+// Generate the path to look for intrinsic modules
+static std::string getIntrinsicDir() {
+  // TODO: Find a system independent API
+  llvm::SmallString<128> driverPath;
+  driverPath.assign(llvm::sys::fs::getMainExecutable(nullptr, nullptr));
+  llvm::sys::path::remove_filename(driverPath);
+  driverPath.append("/../include/flang/");
+  return std::string(driverPath);
+}
+
 /// Parses all preprocessor input arguments and populates the preprocessor
 /// options accordingly.
 ///
 /// \param [in] opts The preprocessor options instance
 /// \param [out] args The list of input arguments
 static void parsePreprocessorArgs(
     Fortran::frontend::PreprocessorOptions &opts, llvm::opt::ArgList &args) {
   // Add macros from the command line.
   for (const auto *currentArg : args.filtered(
            clang::driver::options::OPT_D, clang::driver::options::OPT_U)) {
     if (currentArg->getOption().matches(clang::driver::options::OPT_D)) {
       opts.addMacroDef(currentArg->getValue());
     } else {
       opts.addMacroUndef(currentArg->getValue());
     }
   }
 
   // Add the ordered list of -I's.
   for (const auto *currentArg : args.filtered(clang::driver::options::OPT_I))
     opts.searchDirectoriesFromDashI.emplace_back(currentArg->getValue());
+
+  // Prepend the ordered list of -intrinsic-modules-path
+  // to the default location to search.
+  for (const auto *currentArg :
+      args.filtered(clang::driver::options::OPT_fintrinsic_modules_path))
+    opts.searchDirectoriesFromIntrModPath.emplace_back(currentArg->getValue());
 }
 
 /// Parses all semantic related arguments and populates the variables
 /// options accordingly.
 static void parseSemaArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
     clang::DiagnosticsEngine &diags) {
 
   // -J/module-dir option
   auto moduleDirList =
       args.getAllArgValues(clang::driver::options::OPT_module_dir);
void CompilerInvocation::setFortranOpts() {
   }
   fortranOptions.fixedFormColumns = frontendOptions.fixedFormColumns_;
 
   fortranOptions.features = frontendOptions.features_;
   fortranOptions.encoding = frontendOptions.encoding_;
 
   collectMacroDefinitions(preprocessorOptions, fortranOptions);
 
+  // Adding search directories specified by -I
   fortranOptions.searchDirectories.insert(
       fortranOptions.searchDirectories.end(),
       preprocessorOptions.searchDirectoriesFromDashI.begin(),
       preprocessorOptions.searchDirectoriesFromDashI.end());
 
+  // Add the ordered list of -intrinsic-modules-path
+  fortranOptions.searchDirectories.insert(
+      fortranOptions.searchDirectories.end(),
+      preprocessorOptions.searchDirectoriesFromIntrModPath.begin(),
+      preprocessorOptions.searchDirectoriesFromIntrModPath.end());
+
+  //  Add the default intrinsic module directory at the end
+  fortranOptions.searchDirectories.emplace_back(getIntrinsicDir());
+
   // Add the directory supplied through -J/-module-dir to the list of search
   // directories
   if (moduleDirJ.compare(".") != 0)
     fortranOptions.searchDirectories.emplace_back(moduleDirJ);
 
   if (frontendOptions.instrumentedParse_)
     fortranOptions.instrumentedParse = true;
 }

flang/test/Driver/Inputs/ieee_arithmetic.mod

+! DUMMY module
+! Added for testing purposes. The contents of this file are currently not relevant.
+module ieee_arithmetic
+type::ieee_round_type
+integer(1),private::mode=0_1
+end type
+end

flang/test/Driver/Inputs/iso_fortran_env.mod

+! DUMMY module
+! Added for testing purposes. The contents of this file are currently not relevant.
+module iso_fortran_env
+use __fortran_builtins,only:event_type=>__builtin_event_type
+use __fortran_builtins,only:lock_type=>__builtin_lock_type
+use __fortran_builtins,only:team_type=>__builtin_team_type
+end

flang/test/Driver/driver-help-hidden.f90

 ! CHECK-NEXT: -fdefault-integer-8    Set the default integer kind to an 8 byte wide type
 ! CHECK-NEXT: -fdefault-real-8       Set the default real kind to an 8 byte wide type
 ! CHECK-NEXT: -ffixed-form           Process source files in fixed form
 ! CHECK-NEXT: -ffixed-line-length=<value>
 ! CHECK-NEXT: Use <value> as character line width in fixed mode
 ! CHECK-NEXT: -ffree-form            Process source files in free form
 ! CHECK-NEXT: -fimplicit-none        No implicit typing allowed unless overridden by IMPLICIT statements
 ! CHECK-NEXT: -finput-charset=<value> Specify the default character set for source files
+! CHECK-NEXT: -fintrinsic-modules-path <dir>
+! CHECK-NEXT:                        Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flarge-sizes          Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! CHECK-NEXT: -fopenacc              Enable OpenACC
 ! CHECK-NEXT: -fopenmp               Parse OpenMP pragmas and generate parallel code.
 ! CHECK-NEXT: -fxor-operator         Enable .XOR. as a synonym of .NEQV.
 ! CHECK-NEXT: -help     Display available options
 ! CHECK-NEXT: -I <dir>               Add directory to the end of the list of include search paths

flang/test/Driver/driver-help.f90

 ! HELP-NEXT: -fdefault-integer-8    Set the default integer kind to an 8 byte wide type
 ! HELP-NEXT: -fdefault-real-8       Set the default real kind to an 8 byte wide type
 ! HELP-NEXT: -ffixed-form           Process source files in fixed form
 ! HELP-NEXT: -ffixed-line-length=<value>
 ! HELP-NEXT: Use <value> as character line width in fixed mode
 ! HELP-NEXT: -ffree-form            Process source files in free form
 ! HELP-NEXT: -fimplicit-none        No implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-NEXT: -finput-charset=<value> Specify the default character set for source files
+! HELP-NEXT: -fintrinsic-modules-path <dir>
+! HELP-NEXT:                        Specify where to find the compiled intrinsic modules
 ! HELP-NEXT: -flarge-sizes          Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! HELP-NEXT: -fopenacc              Enable OpenACC
 ! HELP-NEXT: -fopenmp               Parse OpenMP pragmas and generate parallel code.
 ! HELP-NEXT: -fxor-operator         Enable .XOR. as a synonym of .NEQV.
 ! HELP-NEXT: -help                  Display available options
 ! HELP-NEXT: -I <dir>               Add directory to the end of the list of include search paths

 ! HELP-FC1-NEXT: -fdefault-real-8    Set the default real kind to an 8 byte wide type
 ! HELP-FC1-NEXT: -ffixed-form           Process source files in fixed form
 ! HELP-FC1-NEXT: -ffixed-line-length=<value>
 ! HELP-FC1-NEXT: Use <value> as character line width in fixed mode
 ! HELP-FC1-NEXT: -ffree-form            Process source files in free form
 ! HELP-FC1-NEXT: -fget-symbols-sources   Dump symbols and their source code locations
 ! HELP-FC1-NEXT: -fimplicit-none        No implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-FC1-NEXT: -finput-charset=<value> Specify the default character set for source files
+! HELP-FC1-NEXT: -fintrinsic-modules-path <dir>
+! HELP-FC1-NEXT:                        Specify where to find the compiled intrinsic modules
 ! HELP-FC1-NEXT: -flarge-sizes          Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-FC1-NEXT: -fopenacc              Enable OpenACC
 ! HELP-FC1-NEXT: -fopenmp               Parse OpenMP pragmas and generate parallel code.
 ! HELP-FC1-NEXT: -fxor-operator         Enable .XOR. as a synonym of .NEQV.
 ! HELP-FC1-NEXT: -help                  Display available options
 ! HELP-FC1-NEXT: -I <dir>               Add directory to the end of the list of include search paths
 ! HELP-FC1-NEXT: -module-dir <dir>      Put MODULE files in <dir>

flang/test/Driver/intrinsic_module_path.f90

+! Ensure argument -fintrinsic-modules-path works as expected.
+! WITHOUT the option, the default location for the module is checked and no error generated.
+! With the option GIVEN, the module with the same name is PREPENDED, and considered over the
+! default one, causing a CHECKSUM error.
+
+! REQUIRES: new-flang-driver
+
+
+!--------------------------
+! FLANG DRIVER (flang-new)
+!--------------------------
+! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --check-prefix=WITHOUT
+! RUN: not %flang-new -fsyntax-only -fintrinsic-modules-path %S/Inputs/ %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+
+!-----------------------------------------
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-----------------------------------------
+! RUN: %flang-new -fc1 %s  2>&1 | FileCheck %s --allow-empty --check-prefix=WITHOUT
+! RUN: not %flang-new -fc1 -fintrinsic-modules-path %S/Inputs/ %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+
+!-----------------------------------------
+! EXPECTED OUTPUT WITHOUT
+!-----------------------------------------
+! WITHOUT-NOT: 'ieee_arithmetic.mod' was not found
+! WITHOUT-NOT: 'iso_fortran_env.mod' was not found
+
+!-----------------------------------------
+! EXPECTED OUTPUT WITH
+!-----------------------------------------
+! GIVEN: error: Cannot read module file for module 'ieee_arithmetic': File has invalid checksum
+! GIVEN: error: Cannot read module file for module 'iso_fortran_env': File has invalid checksum
+
+
+program test_intrinsic_module_path
+   use ieee_arithmetic, only: ieee_round_type
+   use iso_fortran_env, only: team_type, event_type, lock_type
+end program