jhuber6 added inline comments.
================ Comment at: clang/test/Driver/amdgpu-openmp-toolchain-new.c:6 // RUN: | FileCheck %s +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \ +// RUN: --offload-arch=gfx906 --libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \ ---------------- saiislam wrote: > jhuber6 wrote: > > saiislam wrote: > > > saiislam wrote: > > > > jhuber6 wrote: > > > > > saiislam wrote: > > > > > > Wouldn't it be better if the user is not required to specify the > > > > > > triple in this shorthand version? We can infer the triple from the > > > > > > GPUArch. We have this support in our downstream branch. > > > > > > > > > > > > ``` > > > > > > clang --target=x86_64-unknown-linux-gnu -fopenmp > > > > > > --offload-arch=gfx906 helloworld.c -o helloworld > > > > > > ``` > > > > > We could, HIP and CUDA both use some kind of > > > > > `getAMDOffloadTargetTriple`. I guess in this case we would consider > > > > > OpenMP offloading active if the user specified `-fopenmp` and > > > > > `--offload-arch`? I could do this in a separate patch. > > > > Yes, exactly. OpenMP offloading should be active when `-fopenmp` and > > > > `--offload-arch` both are present. > > > > > > > > Thank you! > > > Following code might be useful for your patch (it assumes that > > > OffloadArch is associated with each device tool chain so that multiple > > > archs of same triple can be compiled together): > > > > > > > > > # [[ > > > https://github.com/RadeonOpenCompute/llvm-project/blob/359002f885ea860609f0c841d69f4970ccbb37af/clang/lib/Driver/Driver.cpp#L735 > > > | GetTargetInfoFromOffloadArch() ]] > > > # [[ > > > https://github.com/RadeonOpenCompute/llvm-project/blob/359002f885ea860609f0c841d69f4970ccbb37af/clang/lib/Driver/Driver.cpp#L779 > > > | Driver::GetTargetInfoFromMarch() ]] > > > # [[ > > > https://github.com/RadeonOpenCompute/llvm-project/blob/359002f885ea860609f0c841d69f4970ccbb37af/clang/lib/Driver/Driver.cpp#L819 > > > | Driver::GetTargetInfoFromOffloadArchOpts() ]] > > > # [[ > > > https://github.com/RadeonOpenCompute/llvm-project/blob/359002f885ea860609f0c841d69f4970ccbb37af/clang/lib/Driver/Driver.cpp#L851 > > > | modified definition of Driver::CreateOffloadingDeviceToolChains() ]] > > > > > I'll look into it, I was thinking of a good way to specify architectures > > per triple. So we could theoretically have `--offload-arch=sm_70` and > > `--offload_arch=gfx908` work in unison and it might just be easy to group > > the triples from the architecture. > Along with this, we would also like to support --offload-arch=gfx906 and > --offload-arch=gfx908 in the same command. This patch already supports that, we'll compile for all the architectures and they'll all end up linked in the linker wrapper. What's missing is the changes to select an appropriate image in the `libomptarget` runtime. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124721/new/ https://reviews.llvm.org/D124721 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
