-
Notifications
You must be signed in to change notification settings - Fork 514
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
base: master
Are you sure you want to change the base?
Add clang
link to PATH
.
#9053
Conversation
There was a problem hiding this 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)?
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 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. |
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… |
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 |
@bhavya01 Yes. |
@yaoshiang Could you stamp this PR? |
This PR install the
clang
andclang++
alternative links usingupdate-alternatives
. This is so we don't need to specify a the Clang version insidebazelrc
in #8665. Therefore, instead of settingCC=/usr/lib/llvm-17/bin/clang
, we can just setCC=clang
.