[AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

[pdhaliwal]

Summary

When offloading kind is OFK_OpenMP, the host toolchain (Generic_GCC)
returns DerivedArgList which copies input arguments along with few
other arguments. HIPToolchain again copies input arguments to the
same DerivedArgList.

[yaxunl]

Can we have a lit test? Thanks.

[pdhaliwal]

Added lit test case

[arsenm]

clang/lib/Driver/ToolChains/HIP.cpp

389

Needs a comment? I don't understand why openmp is any different here

[pdhaliwal]

clang/lib/Driver/ToolChains/HIP.cpp

389

HostTC.TranslateArgs (HostTC = Generic_GCC, Line#383) returns DAL which contains Args when offloadkind is OFK_OpenMP (for all other cases, it returns nullptr). Thus, Line#{390,391} is just duplicating the arguments which are propagating to the opt, llc, etc. commands.
Ref: https://clang.llvm.org/doxygen/Gnu_8cpp_source.html#l02966

[sameerds]

clang/lib/Driver/ToolChains/HIP.cpp

389

I think @arsenm was asking for a comment in the code itself.

Also, I am not sufficiently familiar with the design here, but why is the HIP driver checking for OpenMP offloading? Is the offloaded region treated as HIP code? It seems a bit strange that we are mixing two different things in the same driver.

[pdhaliwal]

clang/lib/Driver/ToolChains/HIP.cpp

389

@sameerds you are correct, openmp should not be mixing things in HIP driver. So, I am dropping this patch before it creates more confusion.

clang/lib/Driver/ToolChains/HIP.cpp

HIPToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args,
                              Action::OffloadKind DeviceOffloadKind) const {
   DerivedArgList *DAL =
       HostTC.TranslateArgs(Args, BoundArch, DeviceOffloadKind);
   if (!DAL)
     DAL = new DerivedArgList(Args.getBaseArgs());
 
   const OptTable &Opts = getDriver().getOpts();
 
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
     for (Arg *A : Args) {
       DAL->append(A);
     }
+  }
 
   if (!BoundArch.empty()) {
     DAL->eraseArg(options::OPT_march_EQ);
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), BoundArch);
   }
 
   return DAL;
 }

clang/test/Driver/hip-openmp-arguments.c

+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -nogpulib \
+// RUN:   -mllvm -amdgpu-dump-hsa-metadata \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-pc-linux-gnu" "-emit-obj"
+// CHECK-SAME: {{.*}} "-fopenmp"
+// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-dump-hsa-metadata" "-fopenmp-is-device" {{.*}}