bazel-contrib / rules_cuda

Starlark implementation of bazel rules for CUDA.
https://bazel-contrib.github.io/rules_cuda/
MIT License
92 stars 43 forks source link

replace accesses to cc_toolchain.compiler_executable with calls to cc_toolchain.get_tool_for_action() #274

Closed hypdeb closed 2 months ago

hypdeb commented 2 months ago

According to the Bazel documentation, compiler_executable is going to be phased out. It is already an issue to use it for people who are trying to work with the new rules_cc implementation of the cc rules.

hypdeb commented 2 months ago

Would want to mark as draft until I've tested it, but not sure how 🤔 .

Found it.

hypdeb commented 2 months ago

Not as easy as I thought. The current error message suggests that find_cpp_toolchain is returning the cuda toolchain instead of the resolved cpp one. Absolutely no idea how this could happen.

hypdeb commented 2 months ago

That seems to do it, but I'm not sure exactly what these fragments are. Advice would be greatly appreciated.

hypdeb commented 2 months ago

A quick question: are all the include directories of the host compiler supposed to be passed to nvcc? It seems it's not the case with my custom cc_toolchain. It's probably unrelated to this MR though.

cloudhan commented 2 months ago

Otherwise, looks good to me.

NOTE: If the env_entry supports iteration like flag_group, we can change it to use the configured cc_toolchain from the action and pass it via the compile/link variable.

cloudhan commented 2 months ago

are all the include directories of the host compiler supposed to be passed to nvcc

@hypdeb Yes. The nvcc is just a wrapper in the host compiling case. So all the envs (automatically transitive if you don't specify some weird env flag) and flags (-Xcompiler) should be passed to host compiler to make it work as expected.

hypdeb commented 2 months ago

@hypdeb Yes. The nvcc is just a wrapper in the host compiling case. So all the envs (automatically transitive if you don't specify some weird env flag) and flags (-Xcompiler) should be passed to host compiler to make it work as expected.

Alright then there's probably some incompatibility between how rules_cc creates compiler flags and how they are imported here that causes my issue. I'll dig deeper into it. It's not directly related to that PR.

cloudhan commented 2 months ago

/test

cloudhan commented 2 months ago

Please rebase. I might trigger the integration test after merge. Seems some actions update contains breaking change for /test comment trigger