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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
