Page MenuHomePhabricator

Accept -x cu to indicate language is CUDA, transfer CUDA language flag to header-file arguments
ClosedPublic

Authored by ADRAADRA on Apr 3 2020, 7:16 PM.

Details

Summary
  • accept -x cu to indicate language is CUDA
  • transfer CUDA language flag to header-file arguments

Diff Detail

Event Timeline

ADRAADRA created this revision.Apr 3 2020, 7:16 PM
ptaylor added a subscriber: ptaylor.Apr 4 2020, 3:33 PM

Please add some more details to the bug description. This change is to make clangd work when compilation database sees CUDA sources compiled with nvcc. NVCC uses different options that should be properly translated. This patch only deals with recognizing the sources as CUDA, but does not handle the compiler options. While not perfect, it's still a useful improvement.

It would be great to extract CUDA path from nvcc's command line and add it as --cuda-path=.... I believe clang-based tools are not doing as much as clang itself when it comes to guessing CUDA location. Without it it will pick up CUDA from the default location (if at all) and that may not be the CUDA used during the build.

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
119

I don't think you need CUDA_FATBIN for clangd. If your input is .fatbin, the source code has been long gone.

sammccall accepted this revision.Apr 6 2020, 12:34 PM

NVCC uses different options that should be properly translated

Interested to see how this will work. Is clang itself going to support these args (act compatibly with nvcc, or is the idea that just tools will be?

clang/lib/Driver/Types.cpp
298

I think this could be clearer: "Accept "cu" as an alias for "cuda" for NVCC compatibility"

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
119

This establishes equivalence classes within which it makes sense to transfer command-line flags. So if you might have compile_commands.json entries building .fatbin inputs, and those flags are a sensible template for building *.cu.cc files, then you want fatbin here, otherwise not.

This revision is now accepted and ready to land.Apr 6 2020, 12:34 PM
tra added a comment.Apr 6 2020, 1:36 PM

NVCC uses different options that should be properly translated

Interested to see how this will work. Is clang itself going to support these args (act compatibly with nvcc, or is the idea that just tools will be?

I don't think it makes a lot of sense to create nvcc-compatible driver. Even if we did, we'd still have to handle things in the tooling library, too, in a way similar to what we currently do for cl or clang-cl. Argument translation could use some refactoring, too, to make it more generic and easier to adapt.

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
119

Both nvcc and clang can build .ptx (nvcc's -ptx, clang's --cuda-device-only -S) or .cubin (-cubin and --cuda-device-only -c). I believe neither nvcc not clang expose fatbin as a compilation result. It's purely for internal driver use in both cases.

ADRAADRA updated this revision to Diff 256111.Apr 8 2020, 2:14 PM
  • Update Types.cpp comment
  • remove types::TY_CUDA_FATBIN from switch
ADRAADRA marked 4 inline comments as done.Apr 8 2020, 2:14 PM
tra accepted this revision.Apr 8 2020, 3:02 PM
tra added a comment.Apr 8 2020, 3:06 PM

Thank you for the patch. I assume you don't have commit access to LLVM. I can land the patch for you.
How should I attribute it? Will ADRA <alandriaawesome@gmail.com> (used in phabricator emails) do or do you prefer some other form?

In D77451#1970461, @tra wrote:

Thank you for the patch. I assume you don't have commit access to LLVM. I can land the patch for you.
How should I attribute it? Will ADRA <alandriaawesome@gmail.com> (used in phabricator emails) do or do you prefer some other form?

Can you use ADRA <plugin2adra@gmail.com>? That's my public email. Thanks!

This revision was automatically updated to reflect the committed changes.
ctetreau added inline comments.
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
119

Syntax error here, master is broken

ctetreau added inline comments.Apr 9 2020, 1:47 PM
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
119
tra added a comment.Apr 9 2020, 2:02 PM

Thank you! A apologize for not checking the patch before landing it.