Skip to content

Feat: Cuda Device Properties#6

Open
micahcc wants to merge 16 commits intooddity-ai:mainfrom
micahcc:main
Open

Feat: Cuda Device Properties#6
micahcc wants to merge 16 commits intooddity-ai:mainfrom
micahcc:main

Conversation

@micahcc
Copy link

@micahcc micahcc commented Aug 7, 2025

  • Add function to get device properties
  • There was one breaking change in the cudaDeviceProps struct between cuda 10 and 11 (they inserted persistingL2CacheMaxSize), but other than that they have only added.
  • Its nice to have the type defined in rust, because its easier for users, but it may be worth generating the type with bindgen if it gets changed again.

Different versions of cudaDeviceProps struct:
2025-08-26T11-29-49

@gerwin3
Copy link
Contributor

gerwin3 commented Aug 27, 2025

Oh so the version in your PR, to which version does it correspond in CUDA exactly?

EDIT: @micahcc And could you also add the padding to it? I think technically without it the struct may be incompatible due to alignment. Also a static check on the size of the struct would be nice :)

@micahcc
Copy link
Author

micahcc commented Nov 21, 2025

@gerwin3 I think without codegen this might get pretty ugly. I think we would want to make a C version of the cudaDeviceProperties with our own layout and then manually copy each field. That way we can guarantee the memory layout in rust. The cuda versions are listed in the diff view I posted (10, 11.8, 12.8).

I'm fine doing the grunt work of doing the manual copy if you're ok with the long term maintenance burden of it (I don't think its high, but some people might balk)

Honestly though the memcpy I have right now will handle any lengthening or shortening of the struct, the only risk is if they add something in the middle, but nothing should result in buffer overruns, at worst it would lead to corrupt data.

@gerwin3
Copy link
Contributor

gerwin3 commented Nov 25, 2025

Actually as shown in your diff the CUDA team has already taken this into account. They added padding to the end of the struct to ensure it is the same size across versions (it is basically a promise they will never go bigger than that). You can just embed the padding on the Rust-side. As long as the struct has repr(C) that will ensure the types are compatible regardless of any changes on the CUDA side.

@gerwin3
Copy link
Contributor

gerwin3 commented Nov 25, 2025

Also, could you split this PR in 2:

  • Device props
  • DeviceTensor

That way I can review and accept them separately.

@micahcc
Copy link
Author

micahcc commented Nov 25, 2025

Sorry yeah, shouldn't have merged main, let me make branches

@micahcc
Copy link
Author

micahcc commented Nov 25, 2025

@gerwin3 ok, updated.

  • Added more testing
  • Added up to the padding from 12.6
  • Removed the tensor changes (can make a new PR, that is for some TensorRT stuff)

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.

2 participants