[Flang][Driver][OpenMP] Enable options for selecting offloading phases in flang

[skatrak]

Summary

This patch unlocks the "--offload-device-only", "--offload-host-only" and
"--offload-host-device" options available in Clang for use by the Flang driver.
These can be used to modify the behavior of the driver to select which
compilation invocations are triggered during OpenMP offloading.

[awarzynski]

Could you add tests that demonstrate what these options actually do?

[skatrak]

In D147941#4255458, @awarzynski wrote:

Could you add tests that demonstrate what these options actually do?

Thank you for the quick review! These options just modify which flang-new -fc1 invocations are produced by the driver when compiling for device offloading. I have added tests that check that only the expected invocations are present, but if these tests are not what you'd expect I'd gladly make some more if you can explain a bit further what you had in mind.

[awarzynski]

In D147941#4255461, @skatrak wrote:

In D147941#4255458, @awarzynski wrote:

Could you add tests that demonstrate what these options actually do?

Thank you for the quick review! These options just modify which flang-new -fc1 invocations are produced by the driver when compiling for device offloading. I have added tests that check that only the expected invocations are present, but if these tests are not what you'd expect I'd gladly make some more if you can explain a bit further what you had in mind.

Cheers!

These can be used to modify the behavior of the driver to select which compilation passes are triggered during OpenMP offloading.

In the context of LLVM, I would normally associate "pass" with something else. I'm not that familiar with offloading, so perhaps that's the language that people use? Personally, in the context of the driver I'd normally use the term "phase" rather than "pass".

This patch makes sense, though I'd like the following to be addressed before landing this:

1. Where's the logic that implements what --offload-host-device? It looks like this is already implemented elsewhere and this patch simply "unlocks" (rather than "implements") this option for Flang.
2. The name of "omp-frontend-forwarding.f90" is very misleading. "Forwarding" would mean flang-new --offload-host-only %s --> flang-new -fc1 --offload-host-only %s, but that's not what's happening here, is it?

For 1. you could just update the summary, and for 2. I suggest renaming "omp-frontend-forwarding.f90" as e.g. "omp-compiler-flag-expansion.f90". WDYT?

clang/include/clang/Driver/Options.td

2738

Why is CoreOption needed here? Wouldn't FlangOption be sufficient?

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

14

[nit] You can reduce noise by using OFFLOAD-HOST-DEVICE instead of CHECK-OFFLOAD-HOST-DEVICE. I think that most people understand that these are CHECK prefixes anyway.

19

[nit] No harm in being a bit more verbose to communicate the intent a bit clearer :)

30

Shouldn't there be 2 driver invocation for the host, as in the the CHECK-OFFLOAD-HOST-DEVICE case?

[skatrak]

Address reviewer's comments

[skatrak]

In D147941#4255622, @awarzynski wrote:

In the context of LLVM, I would normally associate "pass" with something else. I'm not that familiar with offloading, so perhaps that's the language that people use? Personally, in the context of the driver I'd normally use the term "phase" rather than "pass".

This patch makes sense, though I'd like the following to be addressed before landing this:

1. Where's the logic that implements what --offload-host-device? It looks like this is already implemented elsewhere and this patch simply "unlocks" (rather than "implements") this option for Flang.
2. The name of "omp-frontend-forwarding.f90" is very misleading. "Forwarding" would mean flang-new --offload-host-only %s --> flang-new -fc1 --offload-host-only %s, but that's not what's happening here, is it?

For 1. you could just update the summary, and for 2. I suggest renaming "omp-frontend-forwarding.f90" as e.g. "omp-compiler-flag-expansion.f90". WDYT?

I updated the patch title and summary, and renamed the test file to something that I think represents better the contents of the test. Thank you again for the suggestions, let me know if there are any other improvements to make before landing this patch.

clang/include/clang/Driver/Options.td

2738

You're right, thanks for the comment.

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

30

In this case only the first invocation would remain, but I added an additional OFFLOAD-HOST-NOT to make this behavior explicit

[awarzynski]

LGTM, thanks for addressing my comments. I'd wait a day before landing this - just in case other reviewers have comments.

clang/include/clang/Driver/Options.td

defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",
   LangOpts<"OpenMPOptimisticCollapse">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option]>, NegFlag<SetFalse>, BothFlags<[NoArgumentUnused, HelpHidden]>>;
 def static_openmp: Flag<["-"], "static-openmp">,
   HelpText<"Use the static host OpenMP runtime while linking.">;
 def offload_new_driver : Flag<["--"], "offload-new-driver">, Flags<[CC1Option]>, Group<f_Group>,
   MarshallingInfoFlag<LangOpts<"OffloadingNewDriver">>, HelpText<"Use the new driver for offloading compilation.">;
 def no_offload_new_driver : Flag<["--"], "no-offload-new-driver">, Flags<[CC1Option]>, Group<f_Group>,
   HelpText<"Don't Use the new driver for offloading compilation.">;
-def offload_device_only : Flag<["--"], "offload-device-only">,
+def offload_device_only : Flag<["--"], "offload-device-only">, Flags<[FlangOption]>,
   HelpText<"Only compile for the offloading device.">;
-def offload_host_only : Flag<["--"], "offload-host-only">,
+def offload_host_only : Flag<["--"], "offload-host-only">, Flags<[FlangOption]>,
   HelpText<"Only compile for the offloading host.">;
-def offload_host_device : Flag<["--"], "offload-host-device">,
+def offload_host_device : Flag<["--"], "offload-host-device">, Flags<[FlangOption]>,
   HelpText<"Only compile for the offloading host.">;
 def cuda_device_only : Flag<["--"], "cuda-device-only">, Alias<offload_device_only>,
   HelpText<"Compile CUDA code for device only">;
 def cuda_host_only : Flag<["--"], "cuda-host-only">, Alias<offload_host_only>,
   HelpText<"Compile CUDA code for host only. Has no effect on non-CUDA compilations.">;
 def cuda_compile_host_device : Flag<["--"], "cuda-compile-host-device">, Alias<offload_host_device>,
   HelpText<"Compile CUDA code for both host and device (default). Has no "
            "effect on non-CUDA compilations.">;

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

 ! CHECK-NEXT: -g                     Generate source-level debug information
 ! CHECK-NEXT: -help     Display available options
 ! CHECK-NEXT: -I <dir>               Add directory to the end of the list of include search paths
 ! CHECK-NEXT: -mllvm=<arg>           Alias for -mllvm
 ! CHECK-NEXT: -mllvm <value>         Additional arguments to forward to LLVM's option processing
 ! CHECK-NEXT: -mmlir <value>         Additional arguments to forward to MLIR's option processing
 ! CHECK-NEXT: -module-dir <dir>      Put MODULE files in <dir>
 ! CHECK-NEXT: -nocpp                 Disable predefined and command line preprocessor macros
+! CHECK-NEXT: --offload-device-only   Only compile for the offloading device.
+! CHECK-NEXT: --offload-host-device   Only compile for the offloading host.
+! CHECK-NEXT: --offload-host-only     Only compile for the offloading host.
 ! CHECK-NEXT: -o <file> Write output to <file>
 ! CHECK-NEXT: -pedantic              Warn on language extensions
 ! CHECK-NEXT: -print-effective-triple Print the effective target triple
 ! CHECK-NEXT: -print-target-triple    Print the normalized target triple
 ! CHECK-NEXT: -P                     Disable linemarker output in -E mode
 ! CHECK-NEXT: -save-temps=<value>    Save intermediate compilation results.
 ! CHECK-NEXT: -save-temps            Save intermediate compilation results
 ! CHECK-NEXT: -std=<value>           Language standard to compile for

flang/test/Driver/driver-help.f90

 ! HELP-NEXT: -g                     Generate source-level debug information
 ! HELP-NEXT: -help                  Display available options
 ! HELP-NEXT: -I <dir>               Add directory to the end of the list of include search paths
 ! HELP-NEXT: -mllvm=<arg>           Alias for -mllvm
 ! HELP-NEXT: -mllvm <value>         Additional arguments to forward to LLVM's option processing
 ! HELP-NEXT: -mmlir <value>         Additional arguments to forward to MLIR's option processing
 ! HELP-NEXT: -module-dir <dir>      Put MODULE files in <dir>
 ! HELP-NEXT: -nocpp                 Disable predefined and command line preprocessor macros
+! HELP-NEXT: --offload-device-only   Only compile for the offloading device.
+! HELP-NEXT: --offload-host-device   Only compile for the offloading host.
+! HELP-NEXT: --offload-host-only     Only compile for the offloading host.
 ! HELP-NEXT: -o <file>              Write output to <file>
 ! HELP-NEXT: -pedantic              Warn on language extensions
 ! HELP-NEXT: -print-effective-triple Print the effective target triple
 ! HELP-NEXT: -print-target-triple    Print the normalized target triple
 ! HELP-NEXT: -P                     Disable linemarker output in -E mode
 ! HELP-NEXT: -save-temps=<value>    Save intermediate compilation results.
 ! HELP-NEXT: -save-temps            Save intermediate compilation results
 ! HELP-NEXT: -std=<value>           Language standard to compile for

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

 ! 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, and invocation filtering options
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-HOST-AND-DEVICE
+
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 --offload-host-device \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-HOST-AND-DEVICE
+
+! OFFLOAD-HOST-AND-DEVICE: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
+! OFFLOAD-HOST-AND-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa"
+! OFFLOAD-HOST-AND-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "nvptx64-nvidia-cuda"
+! OFFLOAD-HOST-AND-DEVICE: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
+
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 --offload-host-only \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-HOST
+
+! OFFLOAD-HOST: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
+! OFFLOAD-HOST-NOT: "-triple" "amdgcn-amd-amdhsa"
+! OFFLOAD-HOST-NOT: "-triple" "nvptx64-nvidia-cuda"
+! OFFLOAD-HOST-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
+
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 --offload-device-only \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-DEVICE
+
+! OFFLOAD-DEVICE: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
+! OFFLOAD-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa"
+! OFFLOAD-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "nvptx64-nvidia-cuda"
+! OFFLOAD-DEVICE-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
+
 ! Test regular -fopenmp with offload for basic fopenmp-is-device flag addition and correct fopenmp 
 ! RUN: %flang -### -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa %s 2>&1 | FileCheck --check-prefixes=CHECK-OPENMP-IS-DEVICE %s
 ! CHECK-OPENMP-IS-DEVICE: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90"
 
 ! Testing fembed-offload-object and fopenmp-is-device
 ! RUN: %flang -S -### %s -o %t 2>&1 \
 ! RUN: -fopenmp --offload-arch=gfx90a \
 ! RUN: --target=aarch64-unknown-linux-gnu \