Skip to content

Add clang link to PATH. #9053

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ysiraichi
Copy link
Collaborator

This PR install the clang and clang++ alternative links using update-alternatives. This is so we don't need to specify a the Clang version inside bazelrc in #8665. Therefore, instead of setting CC=/usr/lib/llvm-17/bin/clang, we can just set CC=clang.

Copy link
Collaborator

@yaoshiang yaoshiang left a comment

Choose a reason for hiding this comment

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

Can you document this in an existing or new guide in the contributing section so that users can discover support for clang? And also speak to the value of doing so? (faster compile / better debugging symbols / etc)?

@ysiraichi
Copy link
Collaborator Author

I wouldn't say that we support Clang, yet. Support for it is coming in #8665, alongside hermetic CUDA. The reason we are transitioning to Clang is because that's the only way to make hermetic CUDA work. There may be other benefits to which I'm not aware, though.

In summary, this PR is only making it possible to use clang from the generated docker images, without having to specify the version, e.g. clang-17.

That said, I agree it would be nice to say somewhere that we have to use Clang because of hermetic CUDA. I can do that after we land the hermetic CUDA PR.

@yaoshiang
Copy link
Collaborator

Sounds good. Would you mind filing an issue and making sure we do in fact document these flags in a future PR? I personally would have liked to use clang instead of GCc…

@bhavya01
Copy link
Collaborator

Will this change also build cuda plugin using clang. I recently ran into an issue that was caused by our gcc use openxla/xla#25801

@ysiraichi
Copy link
Collaborator Author

@bhavya01 Yes.

@ysiraichi
Copy link
Collaborator Author

@yaoshiang Could you stamp this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants