[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