Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recognize CUDA header file type (.cuh) #12134

Closed
wants to merge 2 commits into from
Closed

Conversation

liuliu
Copy link
Contributor

@liuliu liuliu commented Sep 18, 2020

This PR mirrors the fix in
8804128

.cuh is widely used as suffix for CUDA. It is made more prominent since
"cub" (https://nvlabs.github.io/cub/) recently distributed with CUDA
ToolKit 11, hence, now .cuh headers are in every CUDA ToolKit
distribution.

I would like to continue with this PR and fix any breakages that may encounter.

This PR mirrors the fix in
bazelbuild@8804128
therefore shouldn't be considered as a standalone PR, but rather a
follow up for the said commit as a bugfix.

.cuh is widely used as suffix for CUDA. It is made more prominent since
"cub" (https://nvlabs.github.io/cub/) recently distributed with CUDA
ToolKit 11, hence, now .cuh headers are in every CUDA ToolKit
distribution.

From that lens, this PR simply fixed the said commit so that newer CUDA
ToolKit can be natively supported in Bazel.

I also believe if this PR made its way in, we can close this issue: bazelbuild#6578
@liuliu
Copy link
Contributor Author

liuliu commented Sep 18, 2020

Just noticed @chsigg has to rollback this change for some internal breakages. Would love to know more to close this one out.

@lberki lberki requested review from oquenchil and removed request for lberki September 21, 2020 07:08
@oquenchil oquenchil self-assigned this Oct 5, 2020
@oquenchil oquenchil added the team-Rules-CPP Issues for C++ rules label Oct 5, 2020
Copy link
Contributor

@oquenchil oquenchil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@liuliu
Copy link
Contributor Author

liuliu commented Jan 12, 2021

@oquenchil do you want to merge it? Fixed the merge conflict earlier today.

@bazel-io bazel-io closed this in c750c52 Jan 14, 2021
@c-mita
Copy link
Member

c-mita commented Jan 18, 2021

This had to be rolledback because it caused internal build failures. We don't yet know why.

@chsigg
Copy link

chsigg commented Jan 18, 2021

We don't yet know why.

It's because Google internal builds try to build CUDA headers into header modules (a Google internal variant of C++ modules) and fail spectacularly. We can land this in bazel, but not in blaze.

See the comment here: #6578 (comment)

@oquenchil
Copy link
Contributor

Ideally, we keep the codebase between Bazel and the internal version as close as possible. Can we control whether we build a type of header into modules through the toolchain? IMO that's the only thing that should be different instead of introducing yet another flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants