-
Notifications
You must be signed in to change notification settings - Fork 200
llvm 19 support #227
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: main
Are you sure you want to change the base?
llvm 19 support #227
Conversation
i'm on the fence about renaming i think it might not actually be needed and i might put it back |
@LegNeato my measuring stick here is does could you tell me if I'm close or if I'm actually missing something huge like a mountain of work I'm not seeing? |
pain |
I don't know, llvm is an area of the project I have not touched. |
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.
This should stay the same, right? The newer support should be optional / only enabled when
crates/rustc_codegen_nvvm/build.rs
Outdated
static PREBUILT_LLVM_URL: &str = | ||
"https://github.com/rust-gpu/rustc_codegen_nvvm-llvm/releases/download/LLVM-7.1.0/"; | ||
|
||
static REQUIRED_MAJOR_LLVM_VERSION: u8 = 7; | ||
static REQUIRED_MAJOR_LLVM_VERSION: u8 = 19; |
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.
The requirement doesn't bump unless targeting a higher arch? So the logic is:
- 7 if targeting arch supported by 7 and 19
- 19 if targeting arch not supported by 7
We should probably break this down, seeing as neither of us understand this space. First, we should probably add various values for arch and stuff to enums on the rust side. Some of these might need to be gated if on 7 vs 19. Then, we should get switching between 7 and a stubbed out / non working 19 via target arch. Then, we should systematically fix each issue and refactor common code on the way. |
ok... kind of a strange remark... i'm just going to work on my local branch and make kernels compile with 19.1 then i was going to work backwards and "make it upstream worthy"... |
i rented some powerful big aws spot VM in the cloud and built it's helping find issues more than a typical llvm release build fyi, just a little tip i'd share i have the "shaved yaks" opinionated way to get that instance if you want, cost me about $3 total |
3cac1eb
to
f50708b
Compare
@LegNeato i'm on the fence about having two rustc_codegen_nvvm_v{{version}} crates 85% copy and pasted but.... i got this to work. vecadd is working at least, going to see if ed25519_vanity_rs compiles later proof: |
@LegNeato i've read multiple conflicting things from multiple "official nvidia sources/documentation" that the new cuda 12.9 toolkit is based on/adds support for either llvm 18, llvm 19, or llvm 20. i can see in their i also question if we need this. this might sound dumb but... what if we used a simple official rust target like riscv64gc-unknown-none-elf, use official rust compiler (no custom nightly, no custom codegen llvm integration) to spit out llvm ir, and then patch it to work with cuda... i totally agree with you/the project's view on "use nvvm to compile llvm ir to ptx" https://github.com/brandonros/vanity-miner-rs/pull/8/files i haven't had a chance to test it yet (working on it) but in terms of thinking outside the box, i was for sure able to get nvvm to accept llvm ir and make what seems to be a "valid ptx" |
i got this working for blackwell a different way. this branch/pr/LLVM v19 integration might work just fine but it's kind of a lot to maintain if there's an "easier" (albeit hackier) way to solve this https://github.com/brandonros/vanity-miner-rs/actions/runs/15809309968/job/44558442212 Build Pipeline
let me know if you actually want this/to put time into it, otherwise blackwell+ might be able to avoid cuda_builder or i'd need to make a cuda_builder that makes this super opinionated rust -> cubin pipeline i made |
I'm re-opening because I think we want to go this route. Totally understand if you go a different route for your project! |
I plan to poke at it this week. Apologies for the previous response saying we don't know the space, I don't really know the LLVM side of the house and I misspoke. |
Nope! Here to help, let's land this! Let's land that and then I'll rebase? |
I don't have time to jam on this with you until later in the week, but I think this is a great start! Ideally both are compiled in statically or as dylibs and runtime chooses based on arch selected. But distribution with dylibs is annoying, and compiling 2 llvm versions in the same process will be annoying. So I think the first step is what this is doing, manually switching, but we should be aware where we would like it to go. |
Im willing to put some time in here if I can get some pointers. |
@tyler274 I'm also spending time on this and have followed the same approach as @brandonros. Writing extensive conditional compilation is truly a pain. Perhaps I think we should focus our efforts on this branch? Maybe we could explore how to improve this together? |
@tyler274 #229 (comment) Here are some previous hints from @LegNeato. |
@LegNeato Additionally, based on my previous open-source contributions and discussions, I've learned that both the Graphite and Turso are exploring GPU acceleration. I believe this represents a significant opportunity for us. I'm highly motivated to drive this initiative forward and position our project as the leading GPU-accelerated solution within the Rust ecosystem. |
@Firestar99 is working on Graphite's support via rust-gpu! |
@LegNeato That's awesome! The good news is, after a month of learning and hands-on practice, I've basically figured out the workflow of LLVM backend generation. The bad news is debugging conditional compilation remains quite painful. What do you think – would it be better to write conditional compilation, or maintain two separate branches? |
I think conditional is better, as I think upgrading drops support for a bunch of devices? Or is that not the case? |
what would it take to land this? |
@LegNeato Based on my research, here are the facts: Taking PassManager as an example, this component orchestrates LLVM optimizations and analyses during backend code generation. Typically, multiple passes need to collaborate (e.g., constant propagation followed by dead code elimination). The PassManager schedules passes in a predefined order or based on dependencies to ensure logical execution sequence. Prior to LLVM 9/10, implementations required inheriting from specific PassManager virtual base classes. However, starting from LLVM 14, everything has been unified into a CRTP (Curiously Recurring Template Pattern) code structure. Additionally, header files may have been relocated or modified across different LLVM versions. Therefore, if we choose conditional compilation, I would need to create CI workflows for every single version from LLVM 7 to LLVM 19 to ensure compatibility. |
I think this is a misunderstanding. NVIDIA cards either run LLVM 7 or LLVM 19, nothing in between. Please correct me if I am wrong. |
@brandonros I'm referring specifically to modifications in the LLVM backend generation logic within wrapper files like rustc_llvm_wrapper. This is not targeting NVVM specifically. |
@brandonros @LegNeato https://gist.github.com/ax3l/9489132 This source perfectly highlights that while NVIDIA officially certifies specific LLVM versions per CUDA release (as shown in the version matrix), the reality is more nuanced:
|
@brandonros I think we could start by refining the branches for LLVM 7 and LLVM 19 based on your existing work, then progressively extend support to other components like NVVM (CUDA) and additional LLVM versions. |
@LegNeato I strongly agree that prioritizing support for the newer Rust toolchain, CUDA, and LLVM versions is critical. Our project's future hinges on optimizing for cutting-edge hardware like the H100, A100, and even the GH200 (which I currently have access to). This strategic focus will enable major enterprise customers to integrate our solution into their infrastructure—the key to maximizing our long-term growth and impact. |
I will have rebased this massive thing twice now and it continues to go stale at almost 2+ months old. Are we serious about upstreaming this? Otherwise I'm hesitant to keep doing this same song and dance of "get it ready for merge, put it on the shelf". |
@LegNeato Based on the current modifications, what are the primary remaining challenges? Let's explore what additional efforts and adjustments we can make. |
@LegNeato After reviewing his code changes, I can roughly understand that it seems you don't want to create code for different LLVM versions in multiple folders. Instead, you may prefer to achieve a balance between NVVM and LLVM through conditional compilation. I sincerely request that you take the time to delve into this part. |
potentially addresses all of:
Update rustc_llvm_wrapper to optionally support LLVM v19 #226 (Updates rustc_llvm_wrapper)
GitHub Codespaces/VSCode Devcontainer support #224 (Adds Codespaces, optional, we could remove)
sha2 crate = runtime error #207 (if LLVM v19 will compile and not have same issues as LLVM v7)
CUDA 12.8.1 and LLVM 18.1.8 #197 (we can put this behind a feature flag to optionally support Blackwell+)