From b822e699c3e811f929fd69046095c1d6eb9abd22 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 16 Jul 2018 08:57:49 +0200 Subject: [PATCH] Revert "Use callback-based interface to load ThinLTO import data into rustc." This reverts commit e045a6cd8c0235a26ef11e6cd9a13ebd817f1265. --- src/librustc_codegen_llvm/back/lto.rs | 76 ++++++++++++++++----------- src/librustc_codegen_llvm/base.rs | 2 +- src/librustc_llvm/ffi.rs | 11 ++-- src/rustllvm/PassWrapper.cpp | 61 +++++++++++++++------ 4 files changed, 94 insertions(+), 56 deletions(-) diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index 55499552f5c..a68f22b2651 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/src/librustc_codegen_llvm/back/lto.rs @@ -805,7 +805,7 @@ pub struct ThinLTOImports { impl ThinLTOImports { - pub fn new() -> ThinLTOImports { + pub fn new_empty() -> ThinLTOImports { ThinLTOImports { imports: FxHashMap(), } @@ -813,46 +813,58 @@ impl ThinLTOImports { /// Load the ThinLTO import map from ThinLTOData. unsafe fn from_thin_lto_data(data: *const llvm::ThinLTOData) -> ThinLTOImports { + let raw_data: *const llvm::ThinLTOModuleImports = + llvm::LLVMRustGetThinLTOModuleImports(data); - fn module_name_to_str(c_str: &CStr) -> &str { - match c_str.to_str() { - Ok(s) => s, - Err(e) => { - bug!("Encountered non-utf8 LLVM module name `{}`: {}", - c_str.to_string_lossy(), - e) + assert!(!raw_data.is_null()); + + let mut imports = FxHashMap(); + let mut module_ptr = raw_data; + let mut module_index = 0; + + loop { + let mut entry_ptr: *const llvm::ThinLTOModuleName = *module_ptr; + + if entry_ptr.is_null() { + break; + } + + let importing_module_name = CStr::from_ptr(*entry_ptr) + .to_str() + .expect("Non-utf8 LLVM module name encountered") + .to_owned(); + + entry_ptr = entry_ptr.offset(1); + + let mut imported_modules = vec![]; + + loop { + let imported_module_name = *entry_ptr; + + if imported_module_name.is_null() { + break } - } - } - unsafe extern "C" fn imported_module_callback(payload: *mut libc::c_void, - importing_module_name: *const libc::c_char, - imported_module_name: *const libc::c_char) { - let map = &mut* (payload as *mut ThinLTOImports); + let imported_module_name = CStr::from_ptr(imported_module_name) + .to_str() + .expect("Non-utf8 LLVM module name encountered") + .to_owned(); - let importing_module_name = CStr::from_ptr(importing_module_name); - let importing_module_name = module_name_to_str(&importing_module_name); - let imported_module_name = CStr::from_ptr(imported_module_name); - let imported_module_name = module_name_to_str(&imported_module_name); - - if !map.imports.contains_key(importing_module_name) { - map.imports.insert(importing_module_name.to_owned(), vec![]); + imported_modules.push(imported_module_name); + entry_ptr = entry_ptr.offset(1); } - map.imports - .get_mut(importing_module_name) - .unwrap() - .push(imported_module_name.to_owned()); + imports.insert(importing_module_name, imported_modules); + + module_ptr = module_ptr.offset(1); + module_index += 1; } - let mut map = ThinLTOImports { - imports: FxHashMap(), - }; + assert_eq!(module_index, imports.len()); - llvm::LLVMRustGetThinLTOModuleImports(data, - imported_module_callback, - &mut map as *mut _ as *mut libc::c_void); - map + ThinLTOImports { + imports + } } pub fn save_to_file(&self, path: &Path) -> io::Result<()> { diff --git a/src/librustc_codegen_llvm/base.rs b/src/librustc_codegen_llvm/base.rs index 451e5cf9a60..603ee78585e 100644 --- a/src/librustc_codegen_llvm/base.rs +++ b/src/librustc_codegen_llvm/base.rs @@ -1360,7 +1360,7 @@ fn load_thin_lto_imports(sess: &Session) -> lto::ThinLTOImports { ); if !path.exists() { - return lto::ThinLTOImports::new(); + return lto::ThinLTOImports::new_empty(); } match lto::ThinLTOImports::load_from_file(&path) { diff --git a/src/librustc_llvm/ffi.rs b/src/librustc_llvm/ffi.rs index aafd09ac12d..9b2309a4e2b 100644 --- a/src/librustc_llvm/ffi.rs +++ b/src/librustc_llvm/ffi.rs @@ -351,9 +351,10 @@ pub enum ThinLTOData {} /// LLVMRustThinLTOBuffer pub enum ThinLTOBuffer {} -// LLVMRustModuleNameCallback -pub type ThinLTOModuleNameCallback = - unsafe extern "C" fn(*mut c_void, *const c_char, *const c_char); +/// LLVMRustThinLTOModuleName +pub type ThinLTOModuleName = *const c_char; +/// LLVMRustThinLTOModuleImports +pub type ThinLTOModuleImports = *const ThinLTOModuleName; /// LLVMRustThinLTOModule #[repr(C)] @@ -1785,9 +1786,7 @@ extern "C" { ) -> bool; pub fn LLVMRustGetThinLTOModuleImports( Data: *const ThinLTOData, - ModuleNameCallback: ThinLTOModuleNameCallback, - CallbackPayload: *mut c_void, - ); + ) -> *const ThinLTOModuleImports; pub fn LLVMRustFreeThinLTOData(Data: *mut ThinLTOData); pub fn LLVMRustParseBitcodeForThinLTO( Context: ContextRef, diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp index 1f96b9042ba..30f585efedc 100644 --- a/src/rustllvm/PassWrapper.cpp +++ b/src/rustllvm/PassWrapper.cpp @@ -798,6 +798,11 @@ LLVMRustPGOAvailable() { #endif } +// We encode the ThinLTO module import map as a nested null-terminated list to +// get it into Rust. +typedef const char* LLVMRustThinLTOModuleName; +typedef LLVMRustThinLTOModuleName* LLVMRustThinLTOModuleImports; + #if LLVM_VERSION_GE(4, 0) // Here you'll find an implementation of ThinLTO as used by the Rust compiler @@ -1099,28 +1104,50 @@ LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) { return true; } -extern "C" typedef void (*LLVMRustModuleNameCallback)(void*, // payload - const char*, // importing module name - const char*); // imported module name +/// Converts the LLVMRustThinLTOData::ImportLists map into a nested list. The +/// first level is a null-terminated array with an entry for each module. Each +/// entry is a pointer that points to a null-termined array of module names. The +/// first entry is always the name of the *importing* module, the following +/// entries are the names of the modules it imports from. Each module name is +/// a regular C string. +extern "C" LLVMRustThinLTOModuleImports* +LLVMRustGetThinLTOModuleImports(const LLVMRustThinLTOData *Data) { + // Allocate number of module +1. This is a null-terminated array. + LLVMRustThinLTOModuleImports* thinLTOModuleImports = + new LLVMRustThinLTOModuleImports[Data->ImportLists.size() + 1]; + size_t module_index = 0; -// Calls `module_name_callback` for each module import done by ThinLTO. -// The callback is provided with regular null-terminated C strings. -extern "C" void -LLVMRustGetThinLTOModuleImports(const LLVMRustThinLTOData *data, - LLVMRustModuleNameCallback module_name_callback, - void* callback_payload) { - for (const auto& importing_module : data->ImportLists) { - const std::string importing_module_id = importing_module.getKey().str(); + for (const auto & module : Data->ImportLists) { + StringRef module_id = module.getKey(); + const auto& imports = module.getValue(); - const auto& imports = importing_module.getValue(); + // Allocate number of imported module + 2, one extra for the name of the + // importing module and another one for null-termination. + LLVMRustThinLTOModuleImports imports_array = + new LLVMRustThinLTOModuleName[imports.size() + 2]; - for (const auto& imported_module : imports) { - const std::string imported_module_id = imported_module.getKey().str(); - module_name_callback(callback_payload, - importing_module_id.c_str(), - imported_module_id.c_str()); + // The first value is always the name of the *importing* module. + imports_array[0] = strndup(module_id.data(), module_id.size()); + + size_t imports_array_index = 1; + for (const auto imported_module_id : imports.keys()) { + // The following values are the names of the imported modules. + imports_array[imports_array_index] = strndup(imported_module_id.data(), + imported_module_id.size()); + imports_array_index += 1; } + + assert(imports_array_index == imports.size() + 1); + imports_array[imports_array_index] = nullptr; + + thinLTOModuleImports[module_index] = imports_array; + module_index += 1; } + + assert(module_index == Data->ImportLists.size()); + thinLTOModuleImports[module_index] = nullptr; + + return thinLTOModuleImports; } // This struct and various functions are sort of a hack right now, but the