-
Notifications
You must be signed in to change notification settings - Fork 212
Improve traceability for constant memory placement for static variables #293
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?
Conversation
…t memory spill warnings
15b1306 to
638b478
Compare
638b478 to
25d8ac1
Compare
LegNeato
left a comment
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.
Thank you and welcome to the project! Looks pretty good, just some small things
a1c7207 to
d841974
Compare
|
One more change:
Test example to trigger the error:
#![no_std]
#![crate_type = "staticlib"]
use cuda_std::*;
// 35KB per array, 3 arrays = 105KB total (well above 64KB limit)
const ARRAY_SIZE: usize = 35 * 1024 / 4;
static BIG_ARRAY_1: [u32; ARRAY_SIZE] = [111u32; ARRAY_SIZE];
static BIG_ARRAY_2: [u32; ARRAY_SIZE] = [222u32; ARRAY_SIZE];
static BIG_ARRAY_3: [u32; ARRAY_SIZE] = [333u32; ARRAY_SIZE];
pub unsafe fn test_kernel(out: *mut u32) {
*out = BIG_ARRAY_1[0] + BIG_ARRAY_2[0] + BIG_ARRAY_3[0];
}
use std::path::PathBuf;
fn main() {
let manifest_dir = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap());
let out_path = PathBuf::from(std::env::var("OUT_DIR").unwrap());
println!("cargo::warning=Building GPU kernels with constant memory ENABLED");
println!("cargo::warning=This should trigger constant memory overspill errors");
cuda_builder::CudaBuilder::new(manifest_dir.join("kernels"))
.copy_to(out_path.join("kernels.ptx"))
// Enable constant memory placement to trigger the overspill error
.use_constant_memory_space(true)
.build()
.unwrap();
}
[workspace]
[package]
name = "const_memory_overflow"
version = "0.1.0"
edition = "2021"
[dependencies]
cust = { path = "../../../crates/cust" }
[build-dependencies]
cuda_builder = { path = "../../../crates/cuda_builder", features = ["rustc_codegen_nvvm"] }
[workspace]
[package]
name = "const_memory_overflow_kernels"
version = "0.1.0"
edition = "2021"
[dependencies]
cuda_std = { path = "../../../../crates/cuda_std" }
[lib]
crate-type = ["cdylib", "rlib"]
use cust::prelude::*;
fn main() {
println!("This example is meant to fail at compile time!");
println!("It demonstrates the improved error messages for constant memory overflow.");
}Expected Failure |
|
Option 1: Move static BIG_ARRAY_1: [u32; ARRAY_SIZE] = [111u32; ARRAY_SIZE];
#[cuda_std::address_space(global)]
static BIG_ARRAY_2: [u32; ARRAY_SIZE] = [222u32; ARRAY_SIZE];Option 2: Disable feature competely .use_constant_memory_space(false) |
|
Hi @LegNeato , I addressed the requested changes, please take a look at it when you have a moment |
LegNeato
left a comment
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.
Looks great! Can you put the code in #293 (comment) as a fail compiletest?
To clarify, you want me to add a simple test file to verify the error? like this: |
|
Yep! |
|
Hi @LegNeato , test added: Please let me know what you what you think |
PMQ9
left a comment
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.
compiletest added
For this issue: #218