saiislam 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 \ ---------------- 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. 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
