[PATCH] D21845: [Driver][OpenMP] Add specialized action builder for OpenMP offloading actions.

[Michael Kuron via cfe-commits]

mkuron added a comment.

I think `OffloadAction::DeviceDependences::add(..., ..., /*BoundArch=*/nullptr, 
Action::OFK_OpenMP)` is never sufficient. The invalid `BoundArch` eventually 
ends up in `NVPTX::Assembler::ConstructJob` and triggers an assert; I don't 
think there is any code path with OpenMP offloading where the GPU architecture 
is set correctly. If I compile a simple test file with

  clang -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c example.c 
-march=sm_30

the error message is the following:

  clang: /llvm/tools/clang/lib/Driver/Tools.cpp:11960: virtual void 
clang::driver::tools::NVPTX::Assembler::ConstructJob(clang::driver::Compilation&,
 const clang::driver::JobAction&, const clang::driver::InputInfo&, const 
InputInfoList&, const llvm::opt::ArgList&, const char*) const: Assertion 
`gpu_arch != CudaArch::UNKNOWN && "Device action expected to have an 
architecture."' failed.

On a related but different note, leaving out `-march=sm_30` in the clang call 
above causes an earlier assert to trigger:

  clang: /llvm/tools/clang/lib/Driver/ToolChains.cpp:5049: virtual void 
clang::driver::toolchains::CudaToolChain::addClangTargetOptions(const 
llvm::opt::ArgList&, llvm::opt::ArgStringList&) const: Assertion 
`!GpuArch.empty() && "Must have an explicit GPU arch."' failed.

The more appropriate flag would probably be `--cuda-gpu-arch=sm_30`, but that 
is not recognized.

I thought I'd just report this here as it seemed to me that with the merge of 
all of @sfantao's code yesterday the OpenMP offloading support should mostly 
work. If this is not the case or I should report the issue elsewhere, please 
let me know.


https://reviews.llvm.org/D21845



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D9888: [OPENMP] Driver support for OpenMP offloading

[Michael Kuron via cfe-commits]

mkuron added a comment.

The three smaller patches into which you divided this one appear to be missing 
some things. For example, `AddOpenMPLinkerScript` in //lib/Driver/Tools.cpp// 
from this patch appears to still be necessary to get the desired functionality, 
but it is not present in any of the three.


http://reviews.llvm.org/D9888



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits