From 01de0ff26fe9894c38c2a6e73ac320bf41188d9a Mon Sep 17 00:00:00 2001 From: Karol Herbst Date: Sat, 30 May 2026 20:20:38 +0200 Subject: [PATCH] rusticl/program: store log as a CString Reviewed-by: @LingMan Part-of: --- src/gallium/frontends/rusticl/api/program.rs | 2 +- src/gallium/frontends/rusticl/core/program.rs | 20 +++++----- .../rusticl/mesa/compiler/clc/spirv.rs | 38 +++++++++++-------- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/gallium/frontends/rusticl/api/program.rs b/src/gallium/frontends/rusticl/api/program.rs index b5674983ba0..f4e20599670 100644 --- a/src/gallium/frontends/rusticl/api/program.rs +++ b/src/gallium/frontends/rusticl/api/program.rs @@ -100,7 +100,7 @@ unsafe impl CLInfoObj for cl_program { match q { CL_PROGRAM_BINARY_TYPE => v.write::(prog.bin_type(dev)), CL_PROGRAM_BUILD_GLOBAL_VARIABLE_TOTAL_SIZE => v.write::(0), - CL_PROGRAM_BUILD_LOG => v.write::<&str>(&prog.log(dev)), + CL_PROGRAM_BUILD_LOG => v.write::<&CStr>(&prog.log(dev)), CL_PROGRAM_BUILD_OPTIONS => v.write::<&str>(&prog.options(dev)), CL_PROGRAM_BUILD_STATUS => v.write::(prog.status(dev)), // CL_INVALID_VALUE if param_name is not one of the supported values diff --git a/src/gallium/frontends/rusticl/core/program.rs b/src/gallium/frontends/rusticl/core/program.rs index 7e04ea42148..5a9442c20c6 100644 --- a/src/gallium/frontends/rusticl/core/program.rs +++ b/src/gallium/frontends/rusticl/core/program.rs @@ -14,6 +14,8 @@ use mesa_rust::compiler::clc::*; use mesa_rust::compiler::nir::*; use mesa_rust::util::disk_cache::*; use mesa_rust_gen::*; +use mesa_rust_util::string::CStringExt; +use mesa_rust_util::string::Join; use rusticl_llvm_gen::*; use rusticl_opencl_gen::*; @@ -142,7 +144,7 @@ impl ProgramBuild { convert_spirv_to_nir(build, kernel_name, &args, &mut self.spec_constants, dev) else { build.status = CL_BUILD_ERROR; - build.log = "Internal compilation error".to_owned(); + build.log = c"Internal compilation error".to_owned(); return; }; kernel_info_set.insert(build_result.kernel_info); @@ -195,7 +197,7 @@ pub struct DeviceProgramBuild { spirv: Option, status: cl_build_status, options: String, - log: String, + log: CString, bin_type: cl_program_binary_type, pub kernels: HashMap>, } @@ -266,7 +268,7 @@ impl DeviceProgramBuild { if let Some(log) = log { for line in log { - eprintln!("{}", line); + eprintln!("{line:?}"); } }; @@ -504,7 +506,7 @@ impl Program { self.build_info().dev_build(dev).status } - pub fn log(&self, dev: &Device) -> String { + pub fn log(&self, dev: &Device) -> CString { self.build_info().dev_build(dev).log.clone() } @@ -646,7 +648,7 @@ impl Program { let (spirv, log) = match &self.src { ProgramSourceType::Il(spirv) => { if Platform::dbg().allow_invalid_spirv { - (Some(spirv.clone()), String::new()) + (Some(spirv.clone()), CString::default()) } else { spirv.clone_on_validate(&val_options) } @@ -692,7 +694,7 @@ impl Program { if Platform::dbg().validate_spirv { if let Some(spirv) = spirv { let (res, spirv_msgs) = spirv.validate(&val_options); - (res.then_some(spirv), format!("{}\n{}", msgs, spirv_msgs)) + (res.then_some(spirv), [msgs, spirv_msgs].join(c"\n")) } else { (None, msgs) } @@ -854,7 +856,7 @@ impl Program { if let Some(spirv) = spirv { let val_options = clc_validator_options(device); let (res, spirv_msgs) = spirv.validate(&val_options); - (res.then_some(spirv), format!("{}\n{}", log, spirv_msgs)) + (res.then_some(spirv), [log, spirv_msgs].join(c"\n")) } else { (None, log) } @@ -863,7 +865,7 @@ impl Program { }; build.spirv = spirv; - build.log.push_str(&log); + build.log.push_cstr(&log); if build.spirv.is_some() { build.status = CL_BUILD_SUCCESS as cl_build_status; @@ -942,7 +944,7 @@ fn debug_logging(p: &Program, devs: &[&Device]) { for dev in devs { let msg = p.log(dev); if !msg.is_empty() { - eprintln!("{}", msg); + eprintln!("{msg:?}"); } } } diff --git a/src/gallium/frontends/rusticl/mesa/compiler/clc/spirv.rs b/src/gallium/frontends/rusticl/mesa/compiler/clc/spirv.rs index 7bac2a88736..3bac9f72233 100644 --- a/src/gallium/frontends/rusticl/mesa/compiler/clc/spirv.rs +++ b/src/gallium/frontends/rusticl/mesa/compiler/clc/spirv.rs @@ -58,9 +58,15 @@ impl Debug for CLCHeader<'_> { } unsafe fn callback_impl(data: *mut c_void, msg: *const c_char) { - let data = data as *mut Vec; + if msg.is_null() { + return; + } + + let data = data as *mut Vec; let msgs = unsafe { data.as_mut() }.unwrap(); - msgs.push(c_string_to_string(msg)); + + // SAFETY: msg is a valid C string. + msgs.push(unsafe { CStr::from_ptr(msg) }.to_owned()); } unsafe extern "C" fn spirv_msg_callback(data: *mut c_void, msg: *const c_char) { @@ -82,7 +88,7 @@ unsafe extern "C" fn spirv_to_nir_msg_callback( } } -fn create_clc_logger(msgs: &mut Vec) -> clc_logger { +fn create_clc_logger(msgs: &mut Vec) -> clc_logger { clc_logger { priv_: ptr::from_mut(msgs).cast(), error: Some(spirv_msg_callback), @@ -105,7 +111,7 @@ impl SPIRVBin { features: clc_optional_features, spirv_extensions: &[&CStr], address_bits: u32, - ) -> (Option, String) { + ) -> (Option, CString) { let mut hash_key = None; let has_includes = args.iter().any(|a| a.as_bytes()[0..2] == *b"-I"); @@ -130,7 +136,7 @@ impl SPIRVBin { let mut key = cache.gen_key(&key); if let Some(data) = cache.get(&mut key) { - return (Some(Self::from_bin(&data)), String::from("")); + return (Some(Self::from_bin(&data)), CString::default()); } hash_key = Some(key); @@ -163,7 +169,7 @@ impl SPIRVBin { c_compatible: false, address_bits: address_bits, }; - let mut msgs: Vec = Vec::new(); + let mut msgs = Vec::new(); let logger = create_clc_logger(&mut msgs); let mut out = clc_binary::default(); @@ -187,11 +193,11 @@ impl SPIRVBin { None }; - (res, msgs.join("")) + (res, msgs.join(c"")) } // TODO cache linking, parsing is around 25% of link time - pub fn link(spirvs: &[&SPIRVBin], library: bool) -> (Option, String) { + pub fn link(spirvs: &[&SPIRVBin], library: bool) -> (Option, CString) { let bins: Vec<_> = spirvs.iter().map(|s| ptr::from_ref(&s.spirv)).collect(); let linker_args = clc_linker_args { @@ -200,7 +206,7 @@ impl SPIRVBin { create_library: library as u32, }; - let mut msgs: Vec = Vec::new(); + let mut msgs = Vec::new(); let logger = create_clc_logger(&mut msgs); let mut out = clc_binary::default(); @@ -218,18 +224,18 @@ impl SPIRVBin { spirv: out, info: info, }); - (res, msgs.join("")) + (res, msgs.join(c"")) } - pub fn validate(&self, options: &clc_validator_options) -> (bool, String) { - let mut msgs: Vec = Vec::new(); + pub fn validate(&self, options: &clc_validator_options) -> (bool, CString) { + let mut msgs = Vec::new(); let logger = create_clc_logger(&mut msgs); let res = unsafe { clc_validate_spirv(&self.spirv, &logger, options) }; - (res, msgs.join("")) + (res, msgs.join(c"")) } - pub fn clone_on_validate(&self, options: &clc_validator_options) -> (Option, String) { + pub fn clone_on_validate(&self, options: &clc_validator_options) -> (Option, CString) { let (res, msgs) = self.validate(options); (res.then(|| self.clone()), msgs) } @@ -289,7 +295,7 @@ impl SPIRVBin { library: bool, clc_shader: *const nir_shader, options: SPIRVToNirOptions, - log: Option<&mut Vec>, + log: Option<&mut Vec>, ) -> spirv_to_nir_options { let global_addr_format; let offset_addr_format; @@ -331,7 +337,7 @@ impl SPIRVBin { spirv_to_nir_options: SPIRVToNirOptions, libclc: &NirShader, spec_constants: &mut nir_spirv_specialization, - log: Option<&mut Vec>, + log: Option<&mut Vec>, ) -> Option { let spirv_options = Self::get_spirv_options(false, libclc.get_nir(), spirv_to_nir_options, log);