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 \
----------------
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() ]]



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

Reply via email to