[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

No problem at all, I broke it, happy to fix it! Submitting the fix in the next 
30 minutes~ after a rebase as the build bots seem to have passed happily, it 
took a lot longer than I expected for the build-bot to roundtable to my patch, 
my apologies, teaches me to use it as an adhoc test environment I suppose :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Jean Perier via Phabricator via cfe-commits]

jeanPerier added a comment.



> Hi @jeanPerier,
>
> Thank you! I noticed this today as well and I'm currently looking into it, i 
> have a fix upcoming, just forcing another patch 
> (https://reviews.llvm.org/D144896) to test it via buildbot as I have no easy 
> access to a windows machine to test it at the moment, and yes @jhuber6 and 
> you are correct I believe, it appears to be the .exe suffix on windows! I'll 
> submit the fix as soon as it clears the buildbot.

Thanks a lot for fixing the issue!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

In D145815#4230780 , @jeanPerier 
wrote:

> @agozillon, in the test added here (omp-frontend-forwarding.f90), I am seeing 
> failures in some patches windows pre-merge checks that I think are not 
> related to the patches.
> Could you check if there is a stability/reproducibility issue with the 
> omp-frontend-forwarding.f90 on windows?
>
> The failure message look like:
>
>   # command stderr:
>   
> C:\ws\w8\llvm-project\premerge-checks\flang\test\Driver\omp-frontend-forwarding.f90:21:23:
>  error: CHECK-OPENMP-EMBED: expected string not found in input
>   ! CHECK-OPENMP-EMBED: "{{[^"]*}}clang-offload-packager" {{.*}} 
> "--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
> ^
>   :6:435: note: scanning from here
>"c:\\ws\\w8\\llvm-project\\premerge-checks\\build\\bin\\flang-new" "-fc1" 
> "-triple" "amdgcn-amd-amdhsa" "-emit-llvm-bc" "-fopenmp" "-mrelocation-model" 
> "pic" "-pic-level" "2" "-fopenmp-is-device" "-o" 
> "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-gfx90a-6808f0.bc"
>  "-x" "f95-cpp-input" 
> "C:\\ws\\w8\\llvm-project\\premerge-checks\\flang\\test\\Driver\\omp-frontend-forwarding.f90"
>   
>   
>   
>   
>   
>   ^
>   :7:266: note: possible intended match here
>"c:\\program files\\llvm\\bin\\clang-offload-packager.exe" "-o" 
> "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-9beea6.out"
>  
> "--image=file=C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-gfx90a-6808f0.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
>
> I wonder if this is an issue with the ".exe" command suffix in the windows 
> output.
>
> Example of pre-merge failures:
>
> https://reviews.llvm.org/D146989
> https://buildkite.com/llvm-project/premerge-checks/builds/143821#01872b65-9b0c-457b-8714-c1f0ca00d02b
>
> or
> https://reviews.llvm.org/D147130
> https://buildkite.com/llvm-project/premerge-checks/builds/143873#01872ccb-9251-499a-b9fd-9155e3ffb1f1

Hi @jeanPerier,

Thank you! I noticed this today as well and I'm currently looking into it, i 
have a fix upcoming, just forcing another patch 
(https://reviews.llvm.org/D144896) to test it via buildbot as I have no easy 
access to a windows machine to test it at the moment, and yes @jhuber6 is 
correct, it appears to be the .exe postfix on windows! I'll submit the fix as 
soon as it clears the buildbot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D145815#4230780 , @jeanPerier 
wrote:

> @agozillon, in the test added here (omp-frontend-forwarding.f90), I am seeing 
> failures in some patches windows pre-merge checks that I think are not 
> related to the patches.
> Could you check if there is a stability/reproducibility issue with the 
> omp-frontend-forwarding.f90 on windows?
>
> The failure message look like:
>
>   # command stderr:
>   
> C:\ws\w8\llvm-project\premerge-checks\flang\test\Driver\omp-frontend-forwarding.f90:21:23:
>  error: CHECK-OPENMP-EMBED: expected string not found in input
>   ! CHECK-OPENMP-EMBED: "{{[^"]*}}clang-offload-packager" {{.*}} 
> "--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
> ^
>   :6:435: note: scanning from here
>"c:\\ws\\w8\\llvm-project\\premerge-checks\\build\\bin\\flang-new" "-fc1" 
> "-triple" "amdgcn-amd-amdhsa" "-emit-llvm-bc" "-fopenmp" "-mrelocation-model" 
> "pic" "-pic-level" "2" "-fopenmp-is-device" "-o" 
> "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-gfx90a-6808f0.bc"
>  "-x" "f95-cpp-input" 
> "C:\\ws\\w8\\llvm-project\\premerge-checks\\flang\\test\\Driver\\omp-frontend-forwarding.f90"
>   
>   
>   
>   
>   
>   ^
>   :7:266: note: possible intended match here
>"c:\\program files\\llvm\\bin\\clang-offload-packager.exe" "-o" 
> "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-9beea6.out"
>  
> "--image=file=C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-gfx90a-6808f0.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
>
> I wonder if this is an issue with the ".exe" command suffix in the windows 
> output.
>
> Example of pre-merge failures:
>
> https://reviews.llvm.org/D146989
> https://buildkite.com/llvm-project/premerge-checks/builds/143821#01872b65-9b0c-457b-8714-c1f0ca00d02b
>
> or
> https://reviews.llvm.org/D147130
> https://buildkite.com/llvm-project/premerge-checks/builds/143873#01872ccb-9251-499a-b9fd-9155e3ffb1f1

Most likely the classic `.exe` on Windows problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Jean Perier via Phabricator via cfe-commits]

jeanPerier added a comment.

@agozillon, in the test added here (omp-frontend-forwarding.f90), I am seeing 
failures in some patches windows pre-merge checks that I think are not related 
to the patches.
Could you check if there is a stability/reproducibility issue with the 
omp-frontend-forwarding.f90 on windows?

The failure message look like:

  # command stderr:
  
C:\ws\w8\llvm-project\premerge-checks\flang\test\Driver\omp-frontend-forwarding.f90:21:23:
 error: CHECK-OPENMP-EMBED: expected string not found in input
  ! CHECK-OPENMP-EMBED: "{{[^"]*}}clang-offload-packager" {{.*}} 
"--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
^
  :6:435: note: scanning from here
   "c:\\ws\\w8\\llvm-project\\premerge-checks\\build\\bin\\flang-new" "-fc1" 
"-triple" "amdgcn-amd-amdhsa" "-emit-llvm-bc" "-fopenmp" "-mrelocation-model" 
"pic" "-pic-level" "2" "-fopenmp-is-device" "-o" 
"C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-gfx90a-6808f0.bc"
 "-x" "f95-cpp-input" 
"C:\\ws\\w8\\llvm-project\\premerge-checks\\flang\\test\\Driver\\omp-frontend-forwarding.f90"





^
  :7:266: note: possible intended match here
   "c:\\program files\\llvm\\bin\\clang-offload-packager.exe" "-o" 
"C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-9beea6.out"
 
"--image=file=C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\lit-tmp-1koflt_t\\omp-frontend-forwarding-gfx90a-6808f0.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"

I wonder if this is an issue with the ".exe" command suffix in the windows 
output.

Example of pre-merge failures:

https://reviews.llvm.org/D146989
https://buildkite.com/llvm-project/premerge-checks/builds/143821#01872b65-9b0c-457b-8714-c1f0ca00d02b

or
https://reviews.llvm.org/D147130
https://buildkite.com/llvm-project/premerge-checks/builds/143873#01872ccb-9251-499a-b9fd-9155e3ffb1f1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0cd31a7d3087: [Flang][Driver] Add support for 
fopenmp-is-device and fembed-offload-object to… (authored by agozillon).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

Files:
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/test/Driver/omp-frontend-forwarding.f90


Index: flang/test/Driver/omp-frontend-forwarding.f90
===
--- /dev/null
+++ flang/test/Driver/omp-frontend-forwarding.f90
@@ -0,0 +1,22 @@
+! Test that flang-new OpenMP and OpenMP offload related 
+! commands forward or expand to the appropriate commands 
+! for flang-new -fc1 as expected.
+
+! Test regular -fopenmp with no offload
+! RUN: %flang -### -fopenmp %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP 
%s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} 
"-fopenmp-is-device" {{.*}}.f90"
+
+! 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
+! 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"
Index: clang/lib/Driver/ToolChains/Flang.h
===
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -56,6 +56,17 @@
   void addTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
 
+  /// Extract offload options from the driver arguments and add them to
+  /// the command arguments.
+  /// \param [in] C The current compilation for the driver invocation
+  /// \param [in] Inputs The input infomration on the current file inputs
+  /// \param [in] JA The job action
+  /// \param [in] Args The list of input driver arguments
+  /// \param [out] CmdArgs The list of output command arguments
+  void addOffloadOptions(Compilation , const InputInfoList ,
+ const JobAction , const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
   /// Extract other compilation options from the driver arguments and add them
   /// to the command arguments.
   ///
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -114,6 +114,35 @@
   // TODO: Add target specific flags, ABI, mtune option etc.
 }
 
+void Flang::addOffloadOptions(Compilation , const InputInfoList ,
+  const JobAction , const ArgList ,
+  ArgStringList ) 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])));
+}
+  }
+
+  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");
+  }
+}
+
 static 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

May I please request a final acceptance from both @jhuber6 and @awarzynski 
before I commit this upstream! If you have no further comments to add or 
requests of course.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

Recent commit added some more detail to the comment on the 
fembed-offload-object argument as requested by @awarzynski


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 505957.
agozillon edited the summary of this revision.
agozillon added a comment.

- [Flang][Driver] Expand further on the embed-offload-object comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

Files:
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/test/Driver/omp-frontend-forwarding.f90


Index: flang/test/Driver/omp-frontend-forwarding.f90
===
--- /dev/null
+++ flang/test/Driver/omp-frontend-forwarding.f90
@@ -0,0 +1,22 @@
+! Test that flang-new OpenMP and OpenMP offload related 
+! commands forward or expand to the appropriate commands 
+! for flang-new -fc1 as expected.
+
+! Test regular -fopenmp with no offload
+! RUN: %flang -### -fopenmp %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP 
%s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} 
"-fopenmp-is-device" {{.*}}.f90"
+
+! 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
+! 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"
Index: clang/lib/Driver/ToolChains/Flang.h
===
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -56,6 +56,17 @@
   void addTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
 
+  /// Extract offload options from the driver arguments and add them to
+  /// the command arguments.
+  /// \param [in] C The current compilation for the driver invocation
+  /// \param [in] Inputs The input infomration on the current file inputs
+  /// \param [in] JA The job action
+  /// \param [in] Args The list of input driver arguments
+  /// \param [out] CmdArgs The list of output command arguments
+  void addOffloadOptions(Compilation , const InputInfoList ,
+ const JobAction , const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
   /// Extract other compilation options from the driver arguments and add them
   /// to the command arguments.
   ///
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -114,6 +114,35 @@
   // TODO: Add target specific flags, ABI, mtune option etc.
 }
 
+void Flang::addOffloadOptions(Compilation , const InputInfoList ,
+  const JobAction , const ArgList ,
+  ArgStringList ) 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])));
+}
+  }
+
+  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");
+  }
+}
+
 static void 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

awarzynski wrote:
> jhuber6 wrote:
> > agozillon wrote:
> > > jhuber6 wrote:
> > > > agozillon wrote:
> > > > > awarzynski wrote:
> > > > > > agozillon wrote:
> > > > > > > awarzynski wrote:
> > > > > > > > What's the magic "1"? And given that the input count matters 
> > > > > > > > here - is there a test with multiple inputs?
> > > > > > > It aims to mimic the behavior of Clang: 
> > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
> > > > > > >  where the main input is skipped (the input currently being 
> > > > > > > compiled or embedded into etc.), when adding to 
> > > > > > > //-fembed-offload-object//. 
> > > > > > > 
> > > > > > > It does look different to Clang's as Clang has more cases and the 
> > > > > > > logic is spread across the constructJob invocation, but the first 
> > > > > > > if case is what the if statement inside of the loop and setting 
> > > > > > > the loop index variable to 1 do. The HostOffloadingInputs array 
> > > > > > > is what is being generated here, except we're skipping and 
> > > > > > > directly applying it as arguments.
> > > > > > > 
> > > > > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > > > > readability though, I had hoped the comment might have kept it 
> > > > > > > clear
> > > > > > Thanks for the link - that code in Clang doesn't really clarify 
> > > > > > what makes `Inputs[0]` special 樂 . 
> > > > > > 
> > > > > > Let me rephrase my question - what's so special about the first 
> > > > > > input? (referred to in Clang as "main input") Is that something 
> > > > > > specific to OpenMP? For example, in this case:
> > > > > > ```
> > > > > > flang-new  -fopenmp  file.f90
> > > > > > ```
> > > > > > I assume that `inputs[0]` is "file.f90", so nothing will happen?
> > > > > > 
> > > > > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > > > > readability though, I had hoped the comment might have kept it 
> > > > > > > clear
> > > > > > 
> > > > > > Nah, I think that your implementation is fine. It's my ignorance 
> > > > > > with respect to OpenMP that's the problem here ;-)
> > > > > It's not specific to OpenMP I believe, as far as I am aware Clang's 
> > > > > supported offload models (SYCL and CUDA as well as OpenMP) use it! In 
> > > > > Flang's case we only really care about OpenMP as I believe it's the 
> > > > > only offload programming model supported.
> > > > > 
> > > > > In the case of the command: 
> > > > > 
> > > > > ```
> > > > > flang-new -fopenmp file.f90
> > > > > ``` 
> > > > > The code should never be executed as no part of the command will 
> > > > > enable an offloading action (for device or host)! But yes inputs[0] 
> > > > > would indeed refer to file.f90.
> > > > > 
> > > > > However, this code becomes relevant when you utilise an option that 
> > > > > enables the clangDriver to perform some form of offloading action. 
> > > > > For example a command like: 
> > > > > 
> > > > > ```
> > > > > flang-new -fopenmp --offload-arch=gfx90a file.f90 
> > > > > ```
> > > > > Will trigger two phase compilation, one for the host device (your 
> > > > > resident CPU in this command) and one for the device (gfx90a in this 
> > > > > command), the regular host pass will invoke like your provided 
> > > > > command and the device pass will then invoke with -fopenmp-is-device 
> > > > > in addition alongside the device triple. This generates two bitcode 
> > > > > files from the one file, one containing the host code from the file, 
> > > > > the other the device code (extracted from OpenMP target regions or 
> > > > > declare target etc.). 
> > > > > 
> > > > > However, now we have two files, with both parts of our program, we 
> > > > > need to conjoin them together, the clangDriver generates an 
> > > > > embeddable fat-binary/binary using the clang-offload-packager and 
> > > > > then invokes flang-new again, and this is where the above code 
> > > > > becomes relevant, as this binary (or multiple binaries, if we target 
> > > > > multiple devices in the same program) is what is passed to 
> > > > > -fembed-offload-object! And inputs[0] in this case would refer to the 
> > > > > output from the original host pass, which is what we want to embed 
> > > > > the device binary into, so we wish to skip this original host output 
> > > > > and only pass the subsequent inputs (which should be device binaries 
> > > > > when the clangDriver initiates a host offloading action) we want to 
> > > > > embed as -fembed-offload-object arguments. 
> > > > > 
> > > > > The offloading driver is quite complex and my knowledge of it is not 
> > > > > perfect as I am not our resident expert on it 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:1
+! REQUIRES: amdgpu-registered-target
+

jhuber6 wrote:
> agozillon wrote:
> > awarzynski wrote:
> > > Given that you use `-###`, I think that this can be skipped (please 
> > > double check).
> > It does appear that it can be, at the very least I can swap in an NVIIDIA 
> > arch when I haven't configured the project to target it and it has no 
> > issues! Thank you. 
> I'm not completely familiar with Flangs status on this, do we have tests in 
> tree that perform the entire build and check `-ccc-print-bindings/phases` 
> like we do in Clang?
> I'm not completely familiar with Flangs status on this

I don't recall any other efforts to support offloading in Flang's compiler 
driver - very nice to see this being worked on!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 accepted this revision.
jhuber6 added a comment.

LGTM, it's much simpler for now since Flang doesn't support CUDA, HIP, OpenCL, 
OpenMP, etc.




Comment at: flang/test/Driver/omp-frontend-forwarding.f90:1
+! REQUIRES: amdgpu-registered-target
+

agozillon wrote:
> awarzynski wrote:
> > Given that you use `-###`, I think that this can be skipped (please double 
> > check).
> It does appear that it can be, at the very least I can swap in an NVIIDIA 
> arch when I haven't configured the project to target it and it has no issues! 
> Thank you. 
I'm not completely familiar with Flangs status on this, do we have tests in 
tree that perform the entire build and check `-ccc-print-bindings/phases` like 
we do in Clang?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski accepted this revision.
awarzynski added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for working on this! Would be great for somebody with a bit more 
experience with offloading to OK this as well :) @tschuett or perhaps @jhuber6 ?




Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

jhuber6 wrote:
> agozillon wrote:
> > jhuber6 wrote:
> > > agozillon wrote:
> > > > awarzynski wrote:
> > > > > agozillon wrote:
> > > > > > awarzynski wrote:
> > > > > > > What's the magic "1"? And given that the input count matters here 
> > > > > > > - is there a test with multiple inputs?
> > > > > > It aims to mimic the behavior of Clang: 
> > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
> > > > > >  where the main input is skipped (the input currently being 
> > > > > > compiled or embedded into etc.), when adding to 
> > > > > > //-fembed-offload-object//. 
> > > > > > 
> > > > > > It does look different to Clang's as Clang has more cases and the 
> > > > > > logic is spread across the constructJob invocation, but the first 
> > > > > > if case is what the if statement inside of the loop and setting the 
> > > > > > loop index variable to 1 do. The HostOffloadingInputs array is what 
> > > > > > is being generated here, except we're skipping and directly 
> > > > > > applying it as arguments.
> > > > > > 
> > > > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > > > readability though, I had hoped the comment might have kept it clear
> > > > > Thanks for the link - that code in Clang doesn't really clarify what 
> > > > > makes `Inputs[0]` special 樂 . 
> > > > > 
> > > > > Let me rephrase my question - what's so special about the first 
> > > > > input? (referred to in Clang as "main input") Is that something 
> > > > > specific to OpenMP? For example, in this case:
> > > > > ```
> > > > > flang-new  -fopenmp  file.f90
> > > > > ```
> > > > > I assume that `inputs[0]` is "file.f90", so nothing will happen?
> > > > > 
> > > > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > > > readability though, I had hoped the comment might have kept it clear
> > > > > 
> > > > > Nah, I think that your implementation is fine. It's my ignorance with 
> > > > > respect to OpenMP that's the problem here ;-)
> > > > It's not specific to OpenMP I believe, as far as I am aware Clang's 
> > > > supported offload models (SYCL and CUDA as well as OpenMP) use it! In 
> > > > Flang's case we only really care about OpenMP as I believe it's the 
> > > > only offload programming model supported.
> > > > 
> > > > In the case of the command: 
> > > > 
> > > > ```
> > > > flang-new -fopenmp file.f90
> > > > ``` 
> > > > The code should never be executed as no part of the command will enable 
> > > > an offloading action (for device or host)! But yes inputs[0] would 
> > > > indeed refer to file.f90.
> > > > 
> > > > However, this code becomes relevant when you utilise an option that 
> > > > enables the clangDriver to perform some form of offloading action. For 
> > > > example a command like: 
> > > > 
> > > > ```
> > > > flang-new -fopenmp --offload-arch=gfx90a file.f90 
> > > > ```
> > > > Will trigger two phase compilation, one for the host device (your 
> > > > resident CPU in this command) and one for the device (gfx90a in this 
> > > > command), the regular host pass will invoke like your provided command 
> > > > and the device pass will then invoke with -fopenmp-is-device in 
> > > > addition alongside the device triple. This generates two bitcode files 
> > > > from the one file, one containing the host code from the file, the 
> > > > other the device code (extracted from OpenMP target regions or declare 
> > > > target etc.). 
> > > > 
> > > > However, now we have two files, with both parts of our program, we need 
> > > > to conjoin them together, the clangDriver generates an embeddable 
> > > > fat-binary/binary using the clang-offload-packager and then invokes 
> > > > flang-new again, and this is where the above code becomes relevant, as 
> > > > this binary (or multiple binaries, if we target multiple devices in the 
> > > > same program) is what is passed to -fembed-offload-object! And 
> > > > inputs[0] in this case would refer to the output from the original host 
> > > > pass, which is what we want to embed the device binary into, so we wish 
> > > > to skip this original host output and only pass the subsequent inputs 
> > > > (which should be device binaries when the clangDriver initiates a host 
> > > > offloading action) we want to embed as -fembed-offload-object 
> > > > arguments. 
> > > > 
> > > > The offloading driver is quite complex and my knowledge of it is not 
> > > > perfect as I am not our resident 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

agozillon wrote:
> jhuber6 wrote:
> > agozillon wrote:
> > > awarzynski wrote:
> > > > agozillon wrote:
> > > > > awarzynski wrote:
> > > > > > What's the magic "1"? And given that the input count matters here - 
> > > > > > is there a test with multiple inputs?
> > > > > It aims to mimic the behavior of Clang: 
> > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
> > > > >  where the main input is skipped (the input currently being compiled 
> > > > > or embedded into etc.), when adding to //-fembed-offload-object//. 
> > > > > 
> > > > > It does look different to Clang's as Clang has more cases and the 
> > > > > logic is spread across the constructJob invocation, but the first if 
> > > > > case is what the if statement inside of the loop and setting the loop 
> > > > > index variable to 1 do. The HostOffloadingInputs array is what is 
> > > > > being generated here, except we're skipping and directly applying it 
> > > > > as arguments.
> > > > > 
> > > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > > readability though, I had hoped the comment might have kept it clear
> > > > Thanks for the link - that code in Clang doesn't really clarify what 
> > > > makes `Inputs[0]` special 樂 . 
> > > > 
> > > > Let me rephrase my question - what's so special about the first input? 
> > > > (referred to in Clang as "main input") Is that something specific to 
> > > > OpenMP? For example, in this case:
> > > > ```
> > > > flang-new  -fopenmp  file.f90
> > > > ```
> > > > I assume that `inputs[0]` is "file.f90", so nothing will happen?
> > > > 
> > > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > > readability though, I had hoped the comment might have kept it clear
> > > > 
> > > > Nah, I think that your implementation is fine. It's my ignorance with 
> > > > respect to OpenMP that's the problem here ;-)
> > > It's not specific to OpenMP I believe, as far as I am aware Clang's 
> > > supported offload models (SYCL and CUDA as well as OpenMP) use it! In 
> > > Flang's case we only really care about OpenMP as I believe it's the only 
> > > offload programming model supported.
> > > 
> > > In the case of the command: 
> > > 
> > > ```
> > > flang-new -fopenmp file.f90
> > > ``` 
> > > The code should never be executed as no part of the command will enable 
> > > an offloading action (for device or host)! But yes inputs[0] would indeed 
> > > refer to file.f90.
> > > 
> > > However, this code becomes relevant when you utilise an option that 
> > > enables the clangDriver to perform some form of offloading action. For 
> > > example a command like: 
> > > 
> > > ```
> > > flang-new -fopenmp --offload-arch=gfx90a file.f90 
> > > ```
> > > Will trigger two phase compilation, one for the host device (your 
> > > resident CPU in this command) and one for the device (gfx90a in this 
> > > command), the regular host pass will invoke like your provided command 
> > > and the device pass will then invoke with -fopenmp-is-device in addition 
> > > alongside the device triple. This generates two bitcode files from the 
> > > one file, one containing the host code from the file, the other the 
> > > device code (extracted from OpenMP target regions or declare target 
> > > etc.). 
> > > 
> > > However, now we have two files, with both parts of our program, we need 
> > > to conjoin them together, the clangDriver generates an embeddable 
> > > fat-binary/binary using the clang-offload-packager and then invokes 
> > > flang-new again, and this is where the above code becomes relevant, as 
> > > this binary (or multiple binaries, if we target multiple devices in the 
> > > same program) is what is passed to -fembed-offload-object! And inputs[0] 
> > > in this case would refer to the output from the original host pass, which 
> > > is what we want to embed the device binary into, so we wish to skip this 
> > > original host output and only pass the subsequent inputs (which should be 
> > > device binaries when the clangDriver initiates a host offloading action) 
> > > we want to embed as -fembed-offload-object arguments. 
> > > 
> > > The offloading driver is quite complex and my knowledge of it is not 
> > > perfect as I am not our resident expert on it unfortunately (so if anyone 
> > > sees anything incorrect, please do chime in and correct me)! 
> > > 
> > > But hopefully this answers your question and gives you an idea of when 
> > > and how this -fembed-offload-object comes into play, essentially a way to 
> > > get the device binaries we wish to insert into the host binary, so it can 
> > > load the binaries at runtime and execute them. Currently upstream Flang 
> > > 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

jhuber6 wrote:
> agozillon wrote:
> > awarzynski wrote:
> > > agozillon wrote:
> > > > awarzynski wrote:
> > > > > What's the magic "1"? And given that the input count matters here - 
> > > > > is there a test with multiple inputs?
> > > > It aims to mimic the behavior of Clang: 
> > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
> > > >  where the main input is skipped (the input currently being compiled or 
> > > > embedded into etc.), when adding to //-fembed-offload-object//. 
> > > > 
> > > > It does look different to Clang's as Clang has more cases and the logic 
> > > > is spread across the constructJob invocation, but the first if case is 
> > > > what the if statement inside of the loop and setting the loop index 
> > > > variable to 1 do. The HostOffloadingInputs array is what is being 
> > > > generated here, except we're skipping and directly applying it as 
> > > > arguments.
> > > > 
> > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > readability though, I had hoped the comment might have kept it clear
> > > Thanks for the link - that code in Clang doesn't really clarify what 
> > > makes `Inputs[0]` special 樂 . 
> > > 
> > > Let me rephrase my question - what's so special about the first input? 
> > > (referred to in Clang as "main input") Is that something specific to 
> > > OpenMP? For example, in this case:
> > > ```
> > > flang-new  -fopenmp  file.f90
> > > ```
> > > I assume that `inputs[0]` is "file.f90", so nothing will happen?
> > > 
> > > > I tried to condense it a little in this case! Perhaps it loses 
> > > > readability though, I had hoped the comment might have kept it clear
> > > 
> > > Nah, I think that your implementation is fine. It's my ignorance with 
> > > respect to OpenMP that's the problem here ;-)
> > It's not specific to OpenMP I believe, as far as I am aware Clang's 
> > supported offload models (SYCL and CUDA as well as OpenMP) use it! In 
> > Flang's case we only really care about OpenMP as I believe it's the only 
> > offload programming model supported.
> > 
> > In the case of the command: 
> > 
> > ```
> > flang-new -fopenmp file.f90
> > ``` 
> > The code should never be executed as no part of the command will enable an 
> > offloading action (for device or host)! But yes inputs[0] would indeed 
> > refer to file.f90.
> > 
> > However, this code becomes relevant when you utilise an option that enables 
> > the clangDriver to perform some form of offloading action. For example a 
> > command like: 
> > 
> > ```
> > flang-new -fopenmp --offload-arch=gfx90a file.f90 
> > ```
> > Will trigger two phase compilation, one for the host device (your resident 
> > CPU in this command) and one for the device (gfx90a in this command), the 
> > regular host pass will invoke like your provided command and the device 
> > pass will then invoke with -fopenmp-is-device in addition alongside the 
> > device triple. This generates two bitcode files from the one file, one 
> > containing the host code from the file, the other the device code 
> > (extracted from OpenMP target regions or declare target etc.). 
> > 
> > However, now we have two files, with both parts of our program, we need to 
> > conjoin them together, the clangDriver generates an embeddable 
> > fat-binary/binary using the clang-offload-packager and then invokes 
> > flang-new again, and this is where the above code becomes relevant, as this 
> > binary (or multiple binaries, if we target multiple devices in the same 
> > program) is what is passed to -fembed-offload-object! And inputs[0] in this 
> > case would refer to the output from the original host pass, which is what 
> > we want to embed the device binary into, so we wish to skip this original 
> > host output and only pass the subsequent inputs (which should be device 
> > binaries when the clangDriver initiates a host offloading action) we want 
> > to embed as -fembed-offload-object arguments. 
> > 
> > The offloading driver is quite complex and my knowledge of it is not 
> > perfect as I am not our resident expert on it unfortunately (so if anyone 
> > sees anything incorrect, please do chime in and correct me)! 
> > 
> > But hopefully this answers your question and gives you an idea of when and 
> > how this -fembed-offload-object comes into play, essentially a way to get 
> > the device binaries we wish to insert into the host binary, so it can load 
> > the binaries at runtime and execute them. Currently upstream Flang doesn't 
> > utilise this option of course, but we intend to use this as part of our 
> > OpenMP offloading efforts for AMD devices (whilst leaving the door open for 
> > other vendors devices as well). 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

agozillon wrote:
> awarzynski wrote:
> > agozillon wrote:
> > > awarzynski wrote:
> > > > What's the magic "1"? And given that the input count matters here - is 
> > > > there a test with multiple inputs?
> > > It aims to mimic the behavior of Clang: 
> > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
> > >  where the main input is skipped (the input currently being compiled or 
> > > embedded into etc.), when adding to //-fembed-offload-object//. 
> > > 
> > > It does look different to Clang's as Clang has more cases and the logic 
> > > is spread across the constructJob invocation, but the first if case is 
> > > what the if statement inside of the loop and setting the loop index 
> > > variable to 1 do. The HostOffloadingInputs array is what is being 
> > > generated here, except we're skipping and directly applying it as 
> > > arguments.
> > > 
> > > I tried to condense it a little in this case! Perhaps it loses 
> > > readability though, I had hoped the comment might have kept it clear
> > Thanks for the link - that code in Clang doesn't really clarify what makes 
> > `Inputs[0]` special 樂 . 
> > 
> > Let me rephrase my question - what's so special about the first input? 
> > (referred to in Clang as "main input") Is that something specific to 
> > OpenMP? For example, in this case:
> > ```
> > flang-new  -fopenmp  file.f90
> > ```
> > I assume that `inputs[0]` is "file.f90", so nothing will happen?
> > 
> > > I tried to condense it a little in this case! Perhaps it loses 
> > > readability though, I had hoped the comment might have kept it clear
> > 
> > Nah, I think that your implementation is fine. It's my ignorance with 
> > respect to OpenMP that's the problem here ;-)
> It's not specific to OpenMP I believe, as far as I am aware Clang's supported 
> offload models (SYCL and CUDA as well as OpenMP) use it! In Flang's case we 
> only really care about OpenMP as I believe it's the only offload programming 
> model supported.
> 
> In the case of the command: 
> 
> ```
> flang-new -fopenmp file.f90
> ``` 
> The code should never be executed as no part of the command will enable an 
> offloading action (for device or host)! But yes inputs[0] would indeed refer 
> to file.f90.
> 
> However, this code becomes relevant when you utilise an option that enables 
> the clangDriver to perform some form of offloading action. For example a 
> command like: 
> 
> ```
> flang-new -fopenmp --offload-arch=gfx90a file.f90 
> ```
> Will trigger two phase compilation, one for the host device (your resident 
> CPU in this command) and one for the device (gfx90a in this command), the 
> regular host pass will invoke like your provided command and the device pass 
> will then invoke with -fopenmp-is-device in addition alongside the device 
> triple. This generates two bitcode files from the one file, one containing 
> the host code from the file, the other the device code (extracted from OpenMP 
> target regions or declare target etc.). 
> 
> However, now we have two files, with both parts of our program, we need to 
> conjoin them together, the clangDriver generates an embeddable 
> fat-binary/binary using the clang-offload-packager and then invokes flang-new 
> again, and this is where the above code becomes relevant, as this binary (or 
> multiple binaries, if we target multiple devices in the same program) is what 
> is passed to -fembed-offload-object! And inputs[0] in this case would refer 
> to the output from the original host pass, which is what we want to embed the 
> device binary into, so we wish to skip this original host output and only 
> pass the subsequent inputs (which should be device binaries when the 
> clangDriver initiates a host offloading action) we want to embed as 
> -fembed-offload-object arguments. 
> 
> The offloading driver is quite complex and my knowledge of it is not perfect 
> as I am not our resident expert on it unfortunately (so if anyone sees 
> anything incorrect, please do chime in and correct me)! 
> 
> But hopefully this answers your question and gives you an idea of when and 
> how this -fembed-offload-object comes into play, essentially a way to get the 
> device binaries we wish to insert into the host binary, so it can load the 
> binaries at runtime and execute them. Currently upstream Flang doesn't 
> utilise this option of course, but we intend to use this as part of our 
> OpenMP offloading efforts for AMD devices (whilst leaving the door open for 
> other vendors devices as well). We are trying to re-use/mimic as much of the 
> existing machinery that the clangDriver implements as we can! 
>  
The compiler requires at least one input file to run, otherwise it exits 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

Deleted the clang test that was forwarding to Flang and merged with the 
omp-frontend-forwarding.f90 test where relevant. Second push was because I 
forgot to add a missing newline, which I seem to do frequently...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 505844.
agozillon added a comment.

- Readd missing newline


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

Files:
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/test/Driver/omp-frontend-forwarding.f90


Index: flang/test/Driver/omp-frontend-forwarding.f90
===
--- /dev/null
+++ flang/test/Driver/omp-frontend-forwarding.f90
@@ -0,0 +1,22 @@
+! Test that flang-new OpenMP and OpenMP offload related 
+! commands forward or expand to the appropriate commands 
+! for flang-new -fc1 as expected.
+
+! Test regular -fopenmp with no offload
+! RUN: %flang -### -fopenmp %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP 
%s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} 
"-fopenmp-is-device" {{.*}}.f90"
+
+! 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
+! 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"
Index: clang/lib/Driver/ToolChains/Flang.h
===
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -56,6 +56,17 @@
   void addTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
 
+  /// Extract offload options from the driver arguments and add them to
+  /// the command arguments.
+  /// \param [in] C The current compilation for the driver invocation
+  /// \param [in] Inputs The input infomration on the current file inputs
+  /// \param [in] JA The job action
+  /// \param [in] Args The list of input driver arguments
+  /// \param [out] CmdArgs The list of output command arguments
+  void addOffloadOptions(Compilation , const InputInfoList ,
+ const JobAction , const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
   /// Extract other compilation options from the driver arguments and add them
   /// to the command arguments.
   ///
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -114,6 +114,33 @@
   // TODO: Add target specific flags, ABI, mtune option etc.
 }
 
+void Flang::addOffloadOptions(Compilation , const InputInfoList ,
+  const JobAction , const ArgList ,
+  ArgStringList ) const {
+  bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
+  bool IsHostOffloadingAction = JA.isHostOffloading(Action::OFK_OpenMP) ||
+JA.isHostOffloading(C.getActiveOffloadKinds());
+
+  // Skips primary input file, but adds other input to the 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])));
+}
+  }
+
+  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");
+  }
+}
+
 static void addFloatingPointOptions(const Driver , const ArgList ,
 ArgStringList ) {
   StringRef FPContract;
@@ -327,6 +354,9 @@
   // Add other compile options
   addOtherOptions(Args, CmdArgs);
 
+  // Offloading related options
+  addOffloadOptions(C, Inputs, 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

In D145815#4199848 , @tschuett wrote:

> Flang will also support OpenACC for offload. It is very similar to OpenMP.

You are correct, thank you for reminding me! No idea how I forgot it, my 
apologies.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Flang will also support OpenACC for offload. It is very similar to OpenMP.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 505842.
agozillon added a comment.

- [Flang][Driver] Try to fix failing omp-frontend-forwarding.f90 test
- [Flang][Driver] Simplify omp-frontend-forwarding.f90 test
- [Flang][Driver][Test] Delete flang-omp test from Clang and merge tests into 
omp-frontend-forwarding.f90


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

Files:
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/test/Driver/omp-frontend-forwarding.f90


Index: flang/test/Driver/omp-frontend-forwarding.f90
===
--- /dev/null
+++ flang/test/Driver/omp-frontend-forwarding.f90
@@ -0,0 +1,22 @@
+! Test that flang-new OpenMP and OpenMP offload related 
+! commands forward or expand to the appropriate commands 
+! for flang-new -fc1 as expected.
+
+! Test regular -fopenmp with no offload
+! RUN: %flang -### -fopenmp %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP 
%s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} 
"-fopenmp-is-device" {{.*}}.f90"
+
+! 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
+! 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"
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/Flang.h
===
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -56,6 +56,17 @@
   void addTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
 
+  /// Extract offload options from the driver arguments and add them to
+  /// the command arguments.
+  /// \param [in] C The current compilation for the driver invocation
+  /// \param [in] Inputs The input infomration on the current file inputs
+  /// \param [in] JA The job action
+  /// \param [in] Args The list of input driver arguments
+  /// \param [out] CmdArgs The list of output command arguments
+  void addOffloadOptions(Compilation , const InputInfoList ,
+ const JobAction , const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
   /// Extract other compilation options from the driver arguments and add them
   /// to the command arguments.
   ///
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -114,6 +114,33 @@
   // TODO: Add target specific flags, ABI, mtune option etc.
 }
 
+void Flang::addOffloadOptions(Compilation , const InputInfoList ,
+  const JobAction , const ArgList ,
+  ArgStringList ) const {
+  bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
+  bool IsHostOffloadingAction = JA.isHostOffloading(Action::OFK_OpenMP) ||
+JA.isHostOffloading(C.getActiveOffloadKinds());
+
+  // Skips primary input file, but adds other input to the 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])));
+}
+  }
+
+  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");
+  }
+}
+
 static void addFloatingPointOptions(const Driver , const 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

awarzynski wrote:
> agozillon wrote:
> > awarzynski wrote:
> > > What's the magic "1"? And given that the input count matters here - is 
> > > there a test with multiple inputs?
> > It aims to mimic the behavior of Clang: 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
> >  where the main input is skipped (the input currently being compiled or 
> > embedded into etc.), when adding to //-fembed-offload-object//. 
> > 
> > It does look different to Clang's as Clang has more cases and the logic is 
> > spread across the constructJob invocation, but the first if case is what 
> > the if statement inside of the loop and setting the loop index variable to 
> > 1 do. The HostOffloadingInputs array is what is being generated here, 
> > except we're skipping and directly applying it as arguments.
> > 
> > I tried to condense it a little in this case! Perhaps it loses readability 
> > though, I had hoped the comment might have kept it clear
> Thanks for the link - that code in Clang doesn't really clarify what makes 
> `Inputs[0]` special 樂 . 
> 
> Let me rephrase my question - what's so special about the first input? 
> (referred to in Clang as "main input") Is that something specific to OpenMP? 
> For example, in this case:
> ```
> flang-new  -fopenmp  file.f90
> ```
> I assume that `inputs[0]` is "file.f90", so nothing will happen?
> 
> > I tried to condense it a little in this case! Perhaps it loses readability 
> > though, I had hoped the comment might have kept it clear
> 
> Nah, I think that your implementation is fine. It's my ignorance with respect 
> to OpenMP that's the problem here ;-)
It's not specific to OpenMP I believe, as far as I am aware Clang's supported 
offload models (SYCL and CUDA as well as OpenMP) use it! In Flang's case we 
only really care about OpenMP as I believe it's the only offload programming 
model supported.

In the case of the command: 

```
flang-new -fopenmp file.f90
``` 
The code should never be executed as no part of the command will enable an 
offloading action (for device or host)! But yes inputs[0] would indeed refer to 
file.f90.

However, this code becomes relevant when you utilise an option that enables the 
clangDriver to perform some form of offloading action. For example a command 
like: 

```
flang-new -fopenmp --offload-arch=gfx90a file.f90 
```
Will trigger two phase compilation, one for the host device (your resident CPU 
in this command) and one for the device (gfx90a in this command), the regular 
host pass will invoke like your provided command and the device pass will then 
invoke with -fopenmp-is-device in addition alongside the device triple. This 
generates two bitcode files from the one file, one containing the host code 
from the file, the other the device code (extracted from OpenMP target regions 
or declare target etc.). 

However, now we have two files, with both parts of our program, we need to 
conjoin them together, the clangDriver generates an embeddable 
fat-binary/binary using the clang-offload-packager and then invokes flang-new 
again, and this is where the above code becomes relevant, as this binary (or 
multiple binaries, if we target multiple devices in the same program) is what 
is passed to -fembed-offload-object! And inputs[0] in this case would refer to 
the output from the original host pass, which is what we want to embed the 
device binary into, so we wish to skip this original host output and only pass 
the subsequent inputs (which should be device binaries when the clangDriver 
initiates a host offloading action) we want to embed as -fembed-offload-object 
arguments. 

The offloading driver is quite complex and my knowledge of it is not perfect as 
I am not our resident expert on it unfortunately (so if anyone sees anything 
incorrect, please do chime in and correct me)! 

But hopefully this answers your question and gives you an idea of when and how 
this -fembed-offload-object comes into play, essentially a way to get the 
device binaries we wish to insert into the host binary, so it can load the 
binaries at runtime and execute them. Currently upstream Flang doesn't utilise 
this option of course, but we intend to use this as part of our OpenMP 
offloading efforts for AMD devices (whilst leaving the door open for other 
vendors devices as well). We are trying to re-use/mimic as much of the existing 
machinery that the clangDriver implements as we can! 
 



Comment at: clang/test/Driver/flang/flang-omp.f90:6
+! Test regular -fopenmp with no offload
+! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
--check-prefixes=CHECK-OPENMP %s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

agozillon wrote:
> awarzynski wrote:
> > What's the magic "1"? And given that the input count matters here - is 
> > there a test with multiple inputs?
> It aims to mimic the behavior of Clang: 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
>  where the main input is skipped (the input currently being compiled or 
> embedded into etc.), when adding to //-fembed-offload-object//. 
> 
> It does look different to Clang's as Clang has more cases and the logic is 
> spread across the constructJob invocation, but the first if case is what the 
> if statement inside of the loop and setting the loop index variable to 1 do. 
> The HostOffloadingInputs array is what is being generated here, except we're 
> skipping and directly applying it as arguments.
> 
> I tried to condense it a little in this case! Perhaps it loses readability 
> though, I had hoped the comment might have kept it clear
Thanks for the link - that code in Clang doesn't really clarify what makes 
`Inputs[0]` special 樂 . 

Let me rephrase my question - what's so special about the first input? 
(referred to in Clang as "main input") Is that something specific to OpenMP? 
For example, in this case:
```
flang-new  -fopenmp  file.f90
```
I assume that `inputs[0]` is "file.f90", so nothing will happen?

> I tried to condense it a little in this case! Perhaps it loses readability 
> though, I had hoped the comment might have kept it clear

Nah, I think that your implementation is fine. It's my ignorance with respect 
to OpenMP that's the problem here ;-)



Comment at: clang/test/Driver/flang/flang-omp.f90:6
+! Test regular -fopenmp with no offload
+! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
--check-prefixes=CHECK-OPENMP %s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"

agozillon wrote:
> awarzynski wrote:
> > Can you remind me why isn't it possible to test this with `flang-new`? 
> I double checked, it seems possible to test these with flang-new directly, 
> the main reason I've tested it like this is as it's based on the other tests 
> in the same directory which use clang! 
> 
> However, I'm more than happy to move these tests to the 
> omp-frontend-forwarding.f90 test, remove them or keep these and add flang-new 
> variations into omp-frontend-forwarding.f90. 
I know that it's a bit confusing - Flang.cpp lives in Clang rather than Flang. 
But that's because `Flang.cpp` is part of `clangDriver` - the driver library. 
That library is shared between Clang and Flang and in principle should be taken 
out of Clang into a dedicated subproject - that's the plan :)

This is effectively a Flang patch and I would prefer this test to be added in 
Flang (with `clang` being replaced with `flang-new`). In general, I wouldn't 
add any test in Clang unless testing something Clang specific. This test, 
however, tests Flang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

awarzynski wrote:
> What's the magic "1"? And given that the input count matters here - is there 
> a test with multiple inputs?
It aims to mimic the behavior of Clang: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
 where the main input is skipped (the input currently being compiled or 
embedded into etc.), when adding to //-fembed-offload-object//. 

It does look different to Clang's as Clang has more cases and the logic is 
spread across the constructJob invocation, but the first if case is what the if 
statement inside of the loop and setting the loop index variable to 1 do. The 
HostOffloadingInputs array is what is being generated here, except we're 
skipping and directly applying it as arguments.

I tried to condense it a little in this case! Perhaps it loses readability 
though, I had hoped the comment might have kept it clear



Comment at: clang/test/Driver/flang/flang-omp.f90:6
+! Test regular -fopenmp with no offload
+! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
--check-prefixes=CHECK-OPENMP %s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"

awarzynski wrote:
> Can you remind me why isn't it possible to test this with `flang-new`? 
I double checked, it seems possible to test these with flang-new directly, the 
main reason I've tested it like this is as it's based on the other tests in the 
same directory which use clang! 

However, I'm more than happy to move these tests to the 
omp-frontend-forwarding.f90 test, remove them or keep these and add flang-new 
variations into omp-frontend-forwarding.f90. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.
Herald added a subscriber: jplehr.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+  if (IsHostOffloadingAction) {
+for (size_t i = 1; i < Inputs.size(); ++i) {
+  if (Inputs[i].getType() != types::TY_Nothing)

What's the magic "1"? And given that the input count matters here - is there a 
test with multiple inputs?



Comment at: clang/test/Driver/flang/flang-omp.f90:6
+! Test regular -fopenmp with no offload
+! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
--check-prefixes=CHECK-OPENMP %s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"

Can you remind me why isn't it possible to test this with `flang-new`? 



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:13
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"

agozillon wrote:
> awarzynski wrote:
> > I feel that you can safely remove this line and replace the following with 
> > `CHECK-NEXT`, no? Just wondering whether there's a way to reduce the noise 
> > in these tests (without sacrificing the rigor).
> I can indeed, thank you very much @awarzynski! I have a knack for 
> overcomplicating these it seems
>  I have a knack for overcomplicating these it seems

You should see one of my patches ;-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

Recent update was to simplify the omp-frontend-forwarding.f90 test




Comment at: flang/test/Driver/omp-frontend-forwarding.f90:1
+! REQUIRES: amdgpu-registered-target
+

awarzynski wrote:
> Given that you use `-###`, I think that this can be skipped (please double 
> check).
It does appear that it can be, at the very least I can swap in an NVIIDIA arch 
when I haven't configured the project to target it and it has no issues! Thank 
you. 



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:13
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"

awarzynski wrote:
> I feel that you can safely remove this line and replace the following with 
> `CHECK-NEXT`, no? Just wondering whether there's a way to reduce the noise in 
> these tests (without sacrificing the rigor).
I can indeed, thank you very much @awarzynski! I have a knack for 
overcomplicating these it seems


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 504829.
agozillon added a comment.

- [Flang][Driver] Simplify omp-frontend-forwarding.f90 test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

Files:
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/test/Driver/flang/flang-omp.f90
  flang/test/Driver/omp-frontend-forwarding.f90

Index: flang/test/Driver/omp-frontend-forwarding.f90
===
--- /dev/null
+++ flang/test/Driver/omp-frontend-forwarding.f90
@@ -0,0 +1,13 @@
+! Test that flang-new OpenMP and OpenMP offload related 
+! commands forward or expand to the appropriate commands 
+! for flang-new -fc1 as expected.
+
+! Testing fembed-offload-object and fopenmp-is-device
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}clang-offload-packager" {{.*}} "--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
+! CHECK-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
Index: clang/test/Driver/flang/flang-omp.f90
===
--- /dev/null
+++ clang/test/Driver/flang/flang-omp.f90
@@ -0,0 +1,20 @@
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised
+! and passed down correctly
+
+! Test regular -fopenmp with no offload
+! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP %s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+
+! Test regular -fopenmp with offload for basic fopenmp-is-device flag addition and correct fopenmp 
+! RUN: %clang --driver-mode=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"
+
+! Test -fembed-object, -fopenmp-is-device and -fopenmp carry through when offload-arch is provided 
+! RUN: %clang --driver-mode=flang -### -S --target=aarch64-unknown-linux-gnu -fopenmp --offload-arch=gfx90a %s 2>&1 | FileCheck --check-prefixes=CHECK-EMBED-OBJ-WITH-OARCH %s
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.bc"
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
Index: clang/lib/Driver/ToolChains/Flang.h
===
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -56,6 +56,17 @@
   void addTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
 
+  /// Extract offload options from the driver arguments and add them to
+  /// the command arguments.
+  /// \param [in] C The current compilation for the driver invocation
+  /// \param [in] Inputs The input infomration on the current file inputs
+  /// \param [in] JA The job action
+  /// \param [in] Args The list of input driver arguments
+  /// \param [out] CmdArgs The list of output command arguments
+  void addOffloadOptions(Compilation , const InputInfoList ,
+ const JobAction , const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
   /// Extract other compilation options from the driver arguments and add them
   /// to the command arguments.
   ///
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -114,6 +114,33 @@
   // TODO: Add target specific flags, ABI, mtune option etc.
 }
 
+void Flang::addOffloadOptions(Compilation , const 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:1
+! REQUIRES: amdgpu-registered-target
+

Given that you use `-###`, I think that this can be skipped (please double 
check).



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:13
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"

I feel that you can safely remove this line and replace the following with 
`CHECK-NEXT`, no? Just wondering whether there's a way to reduce the noise in 
these tests (without sacrificing the rigor).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:15
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}clang-offload-packager" {{.*}} 
"--image=file={{.*}}gfx90a.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.bc"

skatrak wrote:
> This unit test seems to be failing due to the pattern you've used here. 
> Probably a simple fix to get working, this is the test output: 
> https://reviews.llvm.org/harbormaster/unit/view/6153283/
Thank you! Hopefully it's working now. Not sure how it got passed me running 
check-all the first time... 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 504592.
agozillon added a comment.

- [Flang][Driver] Try to fix failing omp-frontend-forwarding.f90 test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

Files:
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/test/Driver/flang/flang-omp.f90
  flang/test/Driver/omp-frontend-forwarding.f90

Index: flang/test/Driver/omp-frontend-forwarding.f90
===
--- /dev/null
+++ flang/test/Driver/omp-frontend-forwarding.f90
@@ -0,0 +1,17 @@
+! REQUIRES: amdgpu-registered-target
+
+! Test that flang-new OpenMP and OpenMP offload related 
+! commands forward or expand to the appropriate commands 
+! for flang-new -fc1 as expected.
+
+! Testing fembed-offload-object and fopenmp-is-device
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}clang-offload-packager" {{.*}} "--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.bc"
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
Index: clang/test/Driver/flang/flang-omp.f90
===
--- /dev/null
+++ clang/test/Driver/flang/flang-omp.f90
@@ -0,0 +1,20 @@
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised
+! and passed down correctly
+
+! Test regular -fopenmp with no offload
+! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP %s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+
+! Test regular -fopenmp with offload for basic fopenmp-is-device flag addition and correct fopenmp 
+! RUN: %clang --driver-mode=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"
+
+! Test -fembed-object, -fopenmp-is-device and -fopenmp carry through when offload-arch is provided 
+! RUN: %clang --driver-mode=flang -### -S --target=aarch64-unknown-linux-gnu -fopenmp --offload-arch=gfx90a %s 2>&1 | FileCheck --check-prefixes=CHECK-EMBED-OBJ-WITH-OARCH %s
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.bc"
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
Index: clang/lib/Driver/ToolChains/Flang.h
===
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -56,6 +56,17 @@
   void addTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
 
+  /// Extract offload options from the driver arguments and add them to
+  /// the command arguments.
+  /// \param [in] C The current compilation for the driver invocation
+  /// \param [in] Inputs The input infomration on the current file inputs
+  /// \param [in] JA The job action
+  /// \param [in] Args The list of input driver arguments
+  /// \param [out] CmdArgs The list of output command arguments
+  void addOffloadOptions(Compilation , const InputInfoList ,
+ const JobAction , const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
   /// Extract other compilation options from the driver arguments and add them
   /// to the command arguments.
   ///
Index: 

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Sergio Afonso via Phabricator via cfe-commits]

skatrak added inline comments.



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:15
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}clang-offload-packager" {{.*}} 
"--image=file={{.*}}gfx90a.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" 
{{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.bc"

This unit test seems to be failing due to the pattern you've used here. 
Probably a simple fix to get working, this is the test output: 
https://reviews.llvm.org/harbormaster/unit/view/6153283/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145815/new/

https://reviews.llvm.org/D145815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon created this revision.
Herald added subscribers: sunshaoce, guansong, yaxunl.
Herald added a reviewer: sscalpone.
Herald added a reviewer: awarzynski.
Herald added projects: Flang, All.
agozillon requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, jdoerfert, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

This allows-fembed-offload-object's and -fopenmp-is-device 
compiler invocation arguments to be passed to the Flang frontend 
during split compilation when offloading in OpenMP.

An example use case is when passing an offload-arch alongside 
-fopenmp to embed device objects compiled for the offload-arch 
within the host architecture.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145815

Files:
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/test/Driver/flang/flang-omp.f90
  flang/test/Driver/omp-frontend-forwarding.f90

Index: flang/test/Driver/omp-frontend-forwarding.f90
===
--- /dev/null
+++ flang/test/Driver/omp-frontend-forwarding.f90
@@ -0,0 +1,17 @@
+! REQUIRES: amdgpu-registered-target
+
+! Test that flang-new OpenMP and OpenMP offload related 
+! commands forward or expand to the appropriate commands 
+! for flang-new -fc1 as expected.
+
+! Testing fembed-offload-object and fopenmp-is-device
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK: "{{[^"]*}}clang-offload-packager" {{.*}} "--image=file={{.*}}gfx90a.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
+! CHECK-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.bc"
+! CHECK: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
Index: clang/test/Driver/flang/flang-omp.f90
===
--- /dev/null
+++ clang/test/Driver/flang/flang-omp.f90
@@ -0,0 +1,20 @@
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised
+! and passed down correctly
+
+! Test regular -fopenmp with no offload
+! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP %s
+! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+
+! Test regular -fopenmp with offload for basic fopenmp-is-device flag addition and correct fopenmp 
+! RUN: %clang --driver-mode=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"
+
+! Test -fembed-object, -fopenmp-is-device and -fopenmp carry through when offload-arch is provided 
+! RUN: %clang --driver-mode=flang -### -S --target=aarch64-unknown-linux-gnu -fopenmp --offload-arch=gfx90a %s 2>&1 | FileCheck --check-prefixes=CHECK-EMBED-OBJ-WITH-OARCH %s
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
+! CHECK-EMBED-OBJ-WITH-OARCH-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.bc"
+! CHECK-EMBED-OBJ-WITH-OARCH: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
Index: clang/lib/Driver/ToolChains/Flang.h
===
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -56,6 +56,17 @@
   void addTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
 
+  /// Extract offload options from the driver arguments and add them to
+  /// the command arguments.
+  /// \param [in] C The current compilation for the driver invocation
+  /// \param [in] Inputs The input infomration on the current file inputs
+