tejohnson marked an inline comment as done. tejohnson added inline comments.
================ Comment at: clang/test/Driver/hip-options.hip:63 // RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \ -// RUN: --cuda-gpu-arch=gfx906 -foffload-lto=thin %s 2>&1 \ -// RUN: | FileCheck -check-prefix=THINLTO %s +// RUN: --cuda-gpu-arch=gfx906 -foffload-lto=thin -fwhole-program-vtables %s 2>&1 \ +// RUN: | FileCheck -check-prefix=HIPTHINLTO %s ---------------- tra wrote: > tejohnson wrote: > > tra wrote: > > > > caused a regression when compiling cuda code with -flto=thin > > > > -fwhole-program-vtables. > > > > > > We should add a CUDA test for that. This test only covers HIP compilation. > > AFAICT there are no existing Cuda lto tests in clang/test/Driver that I > > could add -fwhole-program-vtables to. > > > > However, for the purposes of this bug fix, I think adding the below testing > > here should be sufficient - it triggers exactly the same way as when I saw > > this in an internal cuda build. > > AFAICT there are no existing Cuda lto tests in clang/test/Driver that I > > could add -fwhole-program-vtables to. > > `cuda-options.cu` would be the right place. > > > However, for the purposes of this bug fix, I think adding the below testing > > here should be sufficient - it triggers exactly the same way as when I saw > > this in an internal cuda build. > > The key difference is that HIP does support LTO on the GPU side, but CUDA's > does not, which suggests that their handling of lto-related flags is likely > different and worth testing that we do see the expected behavior. E.g. HIP > compilation with `-foffload-lto=thin` does propagate LTO flags to device > compilation, but CUDA compilation should not (but should still compile with > LTO enabled on the host. > > While this patch does fix the issue for both CUDA and HIP, it's good to have > a test which demonstrates how we expect HIP/CUDA to behave. Right now we only > have that for HIP. > Added the cuda test, and confirmed it fails with the error without my patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103579/new/ https://reviews.llvm.org/D103579 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
