oraluben commented on code in PR #300:
URL: https://github.com/apache/tvm-ffi/pull/300#discussion_r2630151136


##########
cmake/Utils/EmbedCubin.cmake:
##########
@@ -15,205 +15,163 @@
 # specific language governing permissions and limitations
 # under the License.
 
+# Do not let cmake to link cudart.
+set(CMAKE_CUDA_RUNTIME_LIBRARY None)

Review Comment:
   I don't think this is feasible with the current design.
   
   First, when `CMAKE_CUDA_RUNTIME_LIBRARY` is not set, the default behavior is 
to statically link `cudart` (this isn’t clearly stated in the CMake docs). This 
imposes an extra constraint on the driver version - it requires the exact CUDA 
driver bundled with that `cudart`, not just a compatible one.
   
   Second, when `CMAKE_CUDA_RUNTIME_LIBRARY` is set, it becomes the default 
value of `CUDA_RUNTIME_LIBRARY` for all future targets. However, our CMake 
utilities do not create targets for users — we only modify or use targets that 
users have already created. (We do define helper targets like 
`add_tvm_ffi_{cubin,fatbin}`, but those are compatibility utilities for CMake < 
3.27; users are still encouraged to create their own CUDA targets and set 
`CUDA_{CUBIN,FATBIN}_COMPILATION` directly.) Therefore, 
`CMAKE_CUDA_RUNTIME_LIBRARY` must be configured before any targets are defined, 
and its value must persist throughout configuration.
   
   Maybe we can explicitly document this behavior and log a warning in CMake to 
make it clearer?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to