Open rnburn opened 1 year ago
Great idea. And after careful thinking about cuda_objects
and cuda_library
with rdc=True
, The currect design is flawed.
In rdc case, objects propagation for dlink follows the table, and in non-rdc case, objects does not propagate
rule | dep rule | dep position | defines from deps | objects from deps[^4] | handling of objects from deps |
---|---|---|---|---|---|
co^1 | co | deps | use and propagate | propagate for dlink | - |
co | co | impl^3 | use | not propagate? | - |
co | cl^2 | disallowed | - | - | - |
cl | co | deps | use and propagate | propagate for dlink | use for dlink, archive |
cl | co | impl | use | not propagate | use for dlink, archive |
cl | cl | deps | use and propagate | propagate for dlink | use for dlink |
cl | cl | impl | use | not propagate | use for dlink |
NOTE: co for cuda_objects, cl for cuda_library
[^4]: deps propagation is the same
In rdc case, objects from deps should always propagate for dlink, and in non-rdc case, objects does not propagate
I didn't put together an issue yet. But I was also noticing that library dependencies set on cuda_objects don't get linked in.
For our unit testing framework catch2, I had to add the dependencies again to our cc_test target even though they're already specified on the cuda_object in order to get linking for the tests to work: https://github.com/spaceandtimelabs/blitzar/blob/main/bazel/sxt_build_system.bzl#L61-L62
That is because cuda_objects only provides a CcInfo with only compilation_context, but no linking context. As the original design, cuda_objects is just compilation actions, whose actifacts are to be fed into a cuda_library for final dlink. Move it to implementation_deps
also clarify the purpose here.
That is because cuda_objects only provides a CcInfo with only compilation_context, but no linking context. As the original design, cuda_objects is just compilation actions, whose actifacts are to be fed into a cuda_library for final dlink.
I don't think that scales well to larger projects with external dependencies. In our case, nearly everything in our project is a cuda_object and we only combine it into a cuda_library at the end when we're linking the shared library: https://github.com/spaceandtimelabs/blitzar/blob/main/cbindings/BUILD#L10-L21
It works ok right now, as we don't have many external dependencies. But if we did start using external library, we'd have to keep track of everything used and add them as dependencies to the final cuda_library -- that could start to become a maintenance burden.
Is there any reason why the cuda_objects can't carry linking dependencies?
Is there any reason why the cuda_objects can't carry linking dependencies?
It is not "can't" per se, it is just not implemented.
I don't think that scales well to larger projects with external dependencies. In our case, nearly everything in our project is a cuda_object...
I am not worried about this. The reason for cuda_objects all over around in your repo is because cuda_library does not work for shared library linking. once we fix them, you should be able to dlink multiple times. But we can impl it anyway.
Another problem arised when it comes to transitive rdc (allow cuda_library to consume cuda_library and propagate the rdc objects). Previously, it was pretty simple and rdc=True
was designed to mark a dlink step in the rule. Then the produced archive is a self contained and is turned back to normal library. Now we need handle the transitive device link issue.
implementation_deps are an option with cc_library and allow you to add deps that don't propagage https://bazel.build/reference/be/c-cpp#cc_library.implementation_deps