[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