[Flang][OpenMP][Driver][MLIR] Port fopenmp-host-ir-file-path flag and add MLIR module attribute to proliferate to OpenMP IR lowering

[agozillon]

Summary

This patch ports the fopenmp-host-ir-file-path flag from Clang to Flang-new, this flag
is added by the driver to the device pass when doing two phase compilation (device + host).

This flag is then applied to the module when compiling during the OpenMP device phase.
This file can then be utilised during lowering of the OpenMP dialect to LLVM-IR, which
allows the device and host to maintain 1:1 mapping of OpenMP metadata for variables
during lowering via the OpenMPIRBuilders loadOffloadInfoMetadata facilities
(which is used for declare target and I believe target regions as well).

[agozillon]

Adding missing newline

[agozillon]

Just a small ping to keep this patch on peoples radar and to try and get some reviewer attention (I am aware you're all quite busy, so thank you very much for any time you can spare) for more review feedback to push this patch further along the pipeline!

[awarzynski]

clang/include/clang/Driver/Options.td

6562–6564

With both options using identical Flags, could you use let Flags = ?

clang/lib/Driver/ToolChains/Flang.cpp

198

200

[nit] You could refer to a specific method in Clang (I did that in the past in a few places)

flang/include/flang/Frontend/LangOptions.h

18

[nit] Empty line between

flang/lib/Frontend/CompilerInvocation.cpp

734

I think that this is expecting a bit too much from CompilerInvocation. Instead, whatever is going to use this file should complain if the file does not exist (CompilerInvocation is merely a messenger here, and file I/O can be quite expensive, hence my reservations).

Is it possible to create a test for "invalid file path" wherever this becomes relevant?

[agozillon]

• Resolve some of the reviewer comments

[agozillon]

Addressed 4/5 of your comments in the latest patch @awarzynski, I've replied to the final one to give a little more information to add to the final decision you make!

flang/lib/Frontend/CompilerInvocation.cpp

734

I believe the follow up patch I have here which uses the file, emits an error if it can't find it: https://reviews.llvm.org/D148370
However, in this case it's an assert rather than more useful Diagnostics unfortunately.

Although, this segment of code does currently just mimic what Clang does in it's own
CompilerInvocation: https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInvocation.cpp#L3883

So it depends if we wish to try to maintain the status quo for the shared argument across the compiler frontends!

So whichever you prefer, I am happy to remove the check at this level and let the lowering handle the problem if it arises :-) but I do like sharing the commonality with Clang where possible.

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

734

Consistency with Clang is a good idea 👍🏻 Though I would appreciate a test that demonstrates that this diagnostic is indeed issues when a non-existing files is specified :)

[agozillon]

• Resolve some of the reviewer comments
• [Flang][OpenMP][Driver][MLIR] Add diagnostic error test

[agozillon]

flang/lib/Frontend/CompilerInvocation.cpp

734

Thank you for pointing out a test was required! I've added one now :-)

[awarzynski]

LGTM, thanks for addressing my comments!

[agozillon]

In D148038#4292262, @awarzynski wrote:

LGTM, thanks for addressing my comments!

Thank you for your time and the great review comments as always!

[awarzynski]

In D148038#4292549, @agozillon wrote:

In D148038#4292262, @awarzynski wrote:

LGTM, thanks for addressing my comments!

Thank you for your time and the great review comments as always!

Thank you 🙏🏻

clang/include/clang/Driver/Options.td

def fno_cuda_host_device_constexpr : Flag<["-"], "fno-cuda-host-device-constexpr">,
   MarshallingInfoNegativeFlag<LangOpts<"CUDAHostDeviceConstexpr">>;
 
 } // let Flags = [CC1Option, NoDriverOption]
 
 //===----------------------------------------------------------------------===//
 // OpenMP Options
 //===----------------------------------------------------------------------===//
 
+let Flags = [CC1Option, FC1Option, NoDriverOption] in {
+
 def fopenmp_is_device : Flag<["-"], "fopenmp-is-device">,
-  HelpText<"Generate code only for an OpenMP target device.">,
-  Flags<[CC1Option, FC1Option, NoDriverOption]>;
+  HelpText<"Generate code only for an OpenMP target device.">;
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
-  HelpText<"Path to the IR file produced by the frontend for the host.">,
-  Flags<[CC1Option, NoDriverOption]>;
+  HelpText<"Path to the IR file produced by the frontend for the host.">;
+
+} // let Flags = [CC1Option, FC1Option, NoDriverOption]
 
 //===----------------------------------------------------------------------===//
 // SYCL Options
 //===----------------------------------------------------------------------===//
 
 def fsycl_is_device : Flag<["-"], "fsycl-is-device">,
   HelpText<"Generate code for SYCL device.">,
   Flags<[CC1Option, NoDriverOption]>,

clang/lib/Driver/ToolChains/Flang.cpp

void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs,
                               const JobAction &JA, const ArgList &Args,
                               ArgStringList &CmdArgs) const {
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
   bool IsHostOffloadingAction = JA.isHostOffloading(Action::OFK_OpenMP) ||
                                 JA.isHostOffloading(C.getActiveOffloadKinds());
 
   // Skips the primary input file, which is the input file that the compilation
   // proccess will be executed upon (e.g. the host bitcode file) and
-  // adds the other secondary input (e.g. device bitcode files for embedding)
-  // to the embed offload object. This is condensed logic from the Clang driver
-  // for embedding offload objects during HostOffloading.
-  if (IsHostOffloadingAction) {
-    for (size_t i = 1; i < Inputs.size(); ++i) {
-      if (Inputs[i].getType() != types::TY_Nothing)
-        CmdArgs.push_back(
-            Args.MakeArgString("-fembed-offload-object=" +
-                               getToolChain().getInputFilename(Inputs[i])));
+  // adds other secondary input (e.g. device bitcode files for embedding to the
+  // -fembed-offload-object argument or the host IR file for proccessing
+  // during device compilation to the fopenmp-host-ir-file-path argument via
+  // OpenMPDeviceInput). This is condensed logic from the ConstructJob
+  // function inside of the Clang driver for pushing on further input arguments
+  // needed for offloading during various phases of compilation.
+    if (Inputs[i].getType() == types::TY_Nothing) {
+      // contains nothing, so it's skippable
+    } else if (IsHostOffloadingAction) {
+    } else if (IsOpenMPDevice) {
+      if (Inputs[i].getFilename()) {
+        CmdArgs.push_back("-fopenmp-host-ir-file-path");
+        CmdArgs.push_back(Args.MakeArgString(Inputs[i].getFilename()));
+      } else {
+        llvm_unreachable("missing openmp host-ir file for device offloading");
+      }
+    } else {
+      llvm_unreachable(
+          "unexpectedly given multiple inputs or given unknown input");
     }
   }
 
   if (IsOpenMPDevice) {
     // -fopenmp-is-device is passed along to tell the frontend that it is
     // generating code for a device, so that only the relevant code is
     // emitted.
     CmdArgs.push_back("-fopenmp-is-device");

flang/include/flang/Frontend/LangOptions.h

 //  configuration for LLVM's middle-end and back-end. It controls LLVM's code
 //  generation into assembly or machine code.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_FLANG_FRONTEND_LANGOPTIONS_H
 #define LLVM_FLANG_FRONTEND_LANGOPTIONS_H
 
+#include <string>
+
 namespace Fortran::frontend {
 
 /// Bitfields of LangOptions, split out from LangOptions to ensure
 /// that this large collection of bitfields is a trivial class type.
 class LangOptionsBase {
 
 public:
   enum FPModeKind {

 public:
   // Define accessors/mutators for code generation options of enumeration type.
 #define LANGOPT(Name, Bits, Default)
 #define ENUM_LANGOPT(Name, Type, Bits, Default)                                \
   Type get##Name() const { return static_cast<Type>(Name); }                   \
   void set##Name(Type Value) { Name = static_cast<unsigned>(Value); }
 #include "flang/Frontend/LangOptions.def"
 
+  /// Name of the IR file that contains the result of the OpenMP target
+  /// host code generation.
+  std::string OMPHostIRFile;
+
   LangOptions();
 };
 
 } // end namespace Fortran::frontend
 
 #endif

flang/include/flang/Tools/CrossToolHelpers.h

 
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/BuiltinOps.h"
 
 struct OffloadModuleOpts {
   OffloadModuleOpts() {}
   OffloadModuleOpts(uint32_t OpenMPTargetDebug, bool OpenMPTeamSubscription,
       bool OpenMPThreadSubscription, bool OpenMPNoThreadState,
-      bool OpenMPNoNestedParallelism, bool OpenMPIsDevice)
+      bool OpenMPNoNestedParallelism, bool OpenMPIsDevice,
+      std::string OMPHostIRFile = {})
       : OpenMPTargetDebug(OpenMPTargetDebug),
         OpenMPTeamSubscription(OpenMPTeamSubscription),
         OpenMPThreadSubscription(OpenMPThreadSubscription),
         OpenMPNoThreadState(OpenMPNoThreadState),
         OpenMPNoNestedParallelism(OpenMPNoNestedParallelism),
-        OpenMPIsDevice(OpenMPIsDevice) {}
+        OpenMPIsDevice(OpenMPIsDevice), OMPHostIRFile(OMPHostIRFile) {}
 
   OffloadModuleOpts(Fortran::frontend::LangOptions &Opts)
       : OpenMPTargetDebug(Opts.OpenMPTargetDebug),
         OpenMPTeamSubscription(Opts.OpenMPTeamSubscription),
         OpenMPThreadSubscription(Opts.OpenMPThreadSubscription),
         OpenMPNoThreadState(Opts.OpenMPNoThreadState),
         OpenMPNoNestedParallelism(Opts.OpenMPNoNestedParallelism),
-        OpenMPIsDevice(Opts.OpenMPIsDevice) {}
+        OpenMPIsDevice(Opts.OpenMPIsDevice), OMPHostIRFile(Opts.OMPHostIRFile) {
+  }
 
   uint32_t OpenMPTargetDebug = 0;
   bool OpenMPTeamSubscription = false;
   bool OpenMPThreadSubscription = false;
   bool OpenMPNoThreadState = false;
   bool OpenMPNoNestedParallelism = false;
   bool OpenMPIsDevice = false;
+  std::string OMPHostIRFile = {};
 };
 
 //  Shares assinging of the OpenMP OffloadModuleInterface and its assorted
 //  attributes accross Flang tools (bbc/flang)
 void setOffloadModuleInterfaceAttributes(
     mlir::ModuleOp &module, OffloadModuleOpts Opts) {
   // Should be registered by the OpenMPDialect
   if (auto offloadMod = llvm::dyn_cast<mlir::omp::OffloadModuleInterface>(
           module.getOperation())) {
     offloadMod.setIsDevice(Opts.OpenMPIsDevice);
     if (Opts.OpenMPIsDevice) {
       offloadMod.setFlags(Opts.OpenMPTargetDebug, Opts.OpenMPTeamSubscription,
           Opts.OpenMPThreadSubscription, Opts.OpenMPNoThreadState,
           Opts.OpenMPNoNestedParallelism);
+
+      if (!Opts.OMPHostIRFile.empty())
+        offloadMod.setHostIRFilePath(Opts.OMPHostIRFile);
     }
   }
 }
 
 //  Shares assinging of the OpenMP OffloadModuleInterface and its TargetCPU
 //  attribute accross Flang tools (bbc/flang)
 void setOffloadModuleInterfaceTargetAttribute(mlir::ModuleOp &module,
     llvm::StringRef targetCPU, llvm::StringRef targetFeatures) {
   // Should be registered by the OpenMPDialect
   if (auto offloadMod = llvm::dyn_cast<mlir::omp::OffloadModuleInterface>(
           module.getOperation())) {
     offloadMod.setTarget(targetCPU, targetFeatures);
   }
 }
 
 #endif // FORTRAN_TOOLS_CROSS_TOOL_HELPERS_H

flang/lib/Frontend/CompilerInvocation.cpp

static bool parseDialectArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
   }
   if (args.hasArg(clang::driver::options::OPT_fopenmp)) {
     res.getFrontendOpts().features.Enable(
         Fortran::common::LanguageFeature::OpenMP);
 
     if (args.hasArg(clang::driver::options::OPT_fopenmp_is_device)) {
       res.getLangOpts().OpenMPIsDevice = 1;
 
+      // Get OpenMP host file path if any and report if a non existent file is
+      // found
+      if (auto *arg = args.getLastArg(
+              clang::driver::options::OPT_fopenmp_host_ir_file_path)) {
+        res.getLangOpts().OMPHostIRFile = arg->getValue();
+        if (!llvm::sys::fs::exists(res.getLangOpts().OMPHostIRFile))
+          diags.Report(clang::diag::err_drv_omp_host_ir_file_not_found)
+              << res.getLangOpts().OMPHostIRFile;
+      }
+
       if (args.hasFlag(
               clang::driver::options::OPT_fopenmp_assume_teams_oversubscription,
               clang::driver::options::
                   OPT_fno_openmp_assume_teams_oversubscription,
               /*Default=*/false))
         res.getLangOpts().OpenMPTeamSubscription = true;
 
       if (args.hasArg(

flang/test/Driver/driver-help.f90

 ! HELP-FC1-NEXT: -fno-automatic         Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE
 ! HELP-FC1-NEXT: -fno-debug-pass-manager Disables debug printing for the new pass manager
 ! HELP-FC1-NEXT: -fno-reformat          Dump the cooked character stream in -E mode
 ! HELP-FC1-NEXT: -fno-signed-zeros      Allow optimizations that ignore the sign of floating point zeros
 ! HELP-FC1-NEXT: -fno-stack-arrays      Allocate array temporaries on the heap (default)
 ! HELP-FC1-NEXT: -fno-version-loops-for-stride
 ! HELP-FC1-NEXT:                        Do not create unit-strided loops (default)
 ! HELP-FC1-NEXT: -fopenacc              Enable OpenACC
+! HELP-FC1-NEXT: -fopenmp-host-ir-file-path <value> 
+! HELP-FC1-NEXT:                        Path to the IR file produced by the frontend for the host.
 ! HELP-FC1-NEXT: -fopenmp-is-device     Generate code only for an OpenMP target device.
 ! HELP-FC1-NEXT: -fopenmp-target-debug  Enable debugging in the OpenMP offloading device RTL
 ! HELP-FC1-NEXT: -fopenmp               Parse OpenMP pragmas and generate parallel code.
 ! HELP-FC1-NEXT: -fpass-plugin=<dsopath> Load pass plugin from a dynamic shared object file (only with new pass manager).
 ! HELP-FC1-NEXT: -freciprocal-math      Allow division operations to be reassociated
 ! HELP-FC1-NEXT: -fstack-arrays         Attempt to allocate array temporaries on the stack, no matter their size
 ! HELP-FC1-NEXT: -fsyntax-only          Run the preprocessor, parser and semantic analysis stages
 ! HELP-FC1-NEXT: -funderscoring         Appends one trailing underscore to external names

flang/test/Driver/omp-driver-offload.f90

 ! OFFLOAD-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa"
 ! OFFLOAD-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "nvptx64-nvidia-cuda"
 ! OFFLOAD-DEVICE-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
 
 ! Test regular -fopenmp with offload for basic fopenmp-is-device flag addition and correct fopenmp 
 ! RUN: %flang -### -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP-IS-DEVICE %s
 ! CHECK-OPENMP-IS-DEVICE: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
 
-! Testing fembed-offload-object and fopenmp-is-device
+! Testing appropriate flags are gnerated and appropriately assigned by the driver when offloading
 ! RUN: %flang -S -### %s -o %t 2>&1 \
 ! RUN: -fopenmp --offload-arch=gfx90a \
 ! RUN: --target=aarch64-unknown-linux-gnu \
-! RUN:   | FileCheck %s --check-prefixes=CHECK-OPENMP-EMBED
-! CHECK-OPENMP-EMBED: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
-! CHECK-OPENMP-EMBED-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
-! CHECK-OPENMP-EMBED: "{{[^"]*}}clang-offload-packager{{.*}} "--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
-! CHECK-OPENMP-EMBED-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
+! RUN:   | FileCheck %s --check-prefix=OPENMP-OFFLOAD-ARGS
+! OPENMP-OFFLOAD-ARGS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
+! OPENMP-OFFLOAD-ARGS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-host-ir-file-path" "{{.*}}.bc" "-fopenmp-is-device" {{.*}}.f90"
+! OPENMP-OFFLOAD-ARGS: "{{[^"]*}}clang-offload-packager" {{.*}} "--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
+! OPENMP-OFFLOAD-ARGS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
 
 ! Test -fopenmp with offload for RTL Flag Options
 ! RUN: %flang -### %s -o %t 2>&1 \
 ! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \
 ! RUN: -fopenmp-assume-threads-oversubscription \
 ! RUN: | FileCheck %s --check-prefixes=CHECK-THREADS-OVS
 ! CHECK-THREADS-OVS: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" "-fopenmp-assume-threads-oversubscription" {{.*}}.f90"
 

 ! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \
 ! RUN: -fopenmp-target-debug -fopenmp-assume-threads-oversubscription \
 ! RUN: -fopenmp-assume-teams-oversubscription -fopenmp-assume-no-nested-parallelism \
 ! RUN: -fopenmp-assume-no-thread-state \
 ! RUN: | FileCheck %s --check-prefixes=CHECK-RTL-ALL
 ! CHECK-RTL-ALL: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" "-fopenmp-target-debug" "-fopenmp-assume-teams-oversubscription"
 ! CHECK-RTL-ALL: "-fopenmp-assume-threads-oversubscription" "-fopenmp-assume-no-thread-state" "-fopenmp-assume-no-nested-parallelism"
 ! CHECK-RTL-ALL: {{.*}}.f90"
+
+! Test diagnostic error when host IR file is non-existent 
+! RUN: not %flang_fc1 %s -o %t 2>&1 -fopenmp -fopenmp-is-device \
+! RUN: -fopenmp-host-ir-file-path non-existant-file.bc \
+! RUN: | FileCheck %s --check-prefix=HOST-IR-MISSING
+! HOST-IR-MISSING: error: provided host compiler IR file 'non-existant-file.bc' is required to generate code for OpenMP target regions but cannot be found

flang/test/Lower/OpenMP/omp-host-ir-flag.f90

+!RUN: %flang_fc1 -emit-llvm-bc -fopenmp -o %t.bc %s 2>&1
+!RUN: %flang_fc1 -emit-mlir -fopenmp -fopenmp-is-device -fopenmp-host-ir-file-path %t.bc -o - %s 2>&1 | FileCheck %s
+
+!CHECK: module attributes {{{.*}}, omp.host_ir_filepath = "{{.*}}.bc", omp.is_device = #omp.isdevice<is_device = true>{{.*}}}
+subroutine omp_subroutine()
+end subroutine omp_subroutine

mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td

InterfaceMethod<
       (ins "bool":$isDevice), [{}], [{
         $_op->setAttr(
           mlir::StringAttr::get($_op->getContext(), llvm::Twine{"omp.is_device"}),
             mlir::omp::IsDeviceAttr::get($_op->getContext(), isDevice));
       }]>,
       InterfaceMethod<
       /*description=*/[{
         Get the IsDeviceAttr attribute on the current module if it exists and return
-        its value, if it doesn't exit it returns false by default.
+        its value, if it doesn't exist it returns false by default.
       }],
       /*retTy=*/"bool",
       /*methodName=*/"getIsDevice",
       (ins), [{}], [{
         if (Attribute isDevice = $_op->getAttr("omp.is_device"))
           if (isDevice.isa<mlir::omp::IsDeviceAttr>())
             return isDevice.dyn_cast<IsDeviceAttr>().getIsDevice();
         return false;
InterfaceMethod<
            "llvm::StringRef":$targetFeatures), [{}], [{
         if (targetCPU.empty())
           return;
         $_op->setAttr(("omp." + mlir::omp::TargetAttr::getMnemonic()).str(),
                   mlir::omp::TargetAttr::get($_op->getContext(),
                                              targetCPU.str(),
                                              targetFeatures.str()));
       }]>,
+      InterfaceMethod<
+      /*description=*/[{
+        Set a StringAttr on the current module containing the host IR file path. This 
+        file path is used in two-phase compilation during the device phase to generate
+        device side LLVM IR when lowering MLIR. 
+      }],
+      /*retTy=*/"void",
+      /*methodName=*/"setHostIRFilePath", 
+      (ins "std::string":$hostIRFilePath), [{}], [{
+        $_op->setAttr(
+          mlir::StringAttr::get($_op->getContext(), llvm::Twine{"omp.host_ir_filepath"}),
+            mlir::StringAttr::get($_op->getContext(), hostIRFilePath));
+       }]>,
+      InterfaceMethod<
+      /*description=*/[{
+        Find the host-ir file path StringAttr from the current module if it exists and 
+        return its contained value, if it doesn't exist it returns an empty string. This 
+        file path is used in two-phase compilation during the device phase to generate
+        device side LLVM IR when lowering MLIR. 
+      }],
+      /*retTy=*/"llvm::StringRef",
+      /*methodName=*/"getHostIRFilePath", 
+      (ins), [{}], [{
+        if (Attribute filepath = $_op->getAttr("omp.host_ir_filepath"))
+          if (filepath.isa<mlir::StringAttr>())
+            return filepath.dyn_cast<mlir::StringAttr>().getValue();
+        return {};
+      }]>
   ];
 }
 
 #endif // OpenMP_OPS_INTERFACES