[PATCH] D81427: [hip] Fix device-only relocatable code compilation.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

LGTM in general, but I'll let Sam stamp it.



================
Comment at: clang/lib/Driver/Driver.cpp:4594-4599
+    if (!AtTopLevel && JA.getType() == types::TY_LLVM_BC &&
+        (C.getArgs().hasArg(options::OPT_emit_llvm) ||
+         (JA.getOffloadingDeviceKind() == Action::OFK_HIP &&
+          isa<CompileJobAction>(JA) &&
+          C.getArgs().hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc,
+                              false))))
----------------
This is rather hard to understand.
Would it be simpler to specify when we shouldn't add `.tmp`?
At the very least I'd extract the newly added clause into a temporary variable and would add some comments explaining why -fgpu-rdc gets special treatment.


================
Comment at: clang/test/Driver/hip-device-only.hip:6
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
----------------
We appear to test `-fgpu-rdc` only and the test case seems to be fairly specific to the way that option is handled by HIP.
Perhaps the test file should reflect that. `hip-rdc-device-only` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81427