Skip to content

Preliminary work for incremental ThinLTO. #52266

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

Merged
merged 7 commits into from
Jul 14, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 68 additions & 1 deletion src/librustc_codegen_llvm/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ use rustc::hir::def_id::LOCAL_CRATE;
use rustc::middle::exported_symbols::SymbolExportLevel;
use rustc::session::config::{self, Lto};
use rustc::util::common::time_ext;
use rustc_data_structures::fx::FxHashMap;
use time_graph::Timeline;
use {ModuleCodegen, ModuleLlvm, ModuleKind, ModuleSource};

use libc;

use std::ffi::CString;
use std::ffi::{CString, CStr};
use std::ptr;
use std::slice;
use std::sync::Arc;
Expand Down Expand Up @@ -776,3 +777,69 @@ impl ThinModule {
Ok(module)
}
}


#[derive(Debug)]
pub struct ThinLTOImports {
// key = llvm name of importing module, value = list of modules it imports from
imports: FxHashMap<String, Vec<String>>,
}

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels sort of overly nasty in terms of converting a type from C++ to Rust, but then again I'm not sure of a better way to do this! In any case I do agree though that we'll want to free the raw_data here at the end of this function (probably via a dtor or something like that)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't like it either :) Suggestions on how to make this nicer are welcome.

I thought I could keep the complex data structure on the C++ and provide a C interface but that quickly seemed like a lot of infrastructure for just passing a map around.

I'll just implement a free function for the data structure. Or I could make it callback based... that would make it a bit less nasty, memory-management-wise.


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
}

let imported_module_name = CStr::from_ptr(imported_module_name)
.to_str()
.expect("Non-utf8 LLVM module name encountered")
.to_owned();

imported_modules.push(imported_module_name);
entry_ptr = entry_ptr.offset(1);
}

imports.insert(importing_module_name, imported_modules);

module_ptr = module_ptr.offset(1);
module_index += 1;
}

assert_eq!(module_index, imports.len());

ThinLTOImports {
imports
}
}
}
8 changes: 8 additions & 0 deletions src/librustc_llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,11 @@ pub enum ThinLTOData {}
/// LLVMRustThinLTOBuffer
pub enum ThinLTOBuffer {}

/// LLVMRustThinLTOModuleName
pub type ThinLTOModuleName = *const c_char;
/// LLVMRustThinLTOModuleImports
pub type ThinLTOModuleImports = *const ThinLTOModuleName;

/// LLVMRustThinLTOModule
#[repr(C)]
pub struct ThinLTOModule {
Expand Down Expand Up @@ -1778,6 +1783,9 @@ extern "C" {
Data: *const ThinLTOData,
Module: ModuleRef,
) -> bool;
pub fn LLVMRustGetThinLTOModuleImports(
Data: *const ThinLTOData,
) -> *const ThinLTOModuleImports;
pub fn LLVMRustFreeThinLTOData(Data: *mut ThinLTOData);
pub fn LLVMRustParseBitcodeForThinLTO(
Context: ContextRef,
Expand Down
56 changes: 56 additions & 0 deletions src/rustllvm/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1099,6 +1104,52 @@ LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) {
return true;
}

/// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe say "regular malloc'ed C string" or something along those lines, just so someone doesn't have to scan the code for those calls to strndup ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note: In the current version, the allocations here are just leaked. I should probably fix that.

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;

for (const auto & module : Data->ImportLists) {
StringRef module_id = module.getKey();
const auto& imports = 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];

// 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;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could've also provided an iterator interface, like a bunch other LLVM APIs we support.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Searching for Iterator in src/librustc_llvm and src/rustllvm is enough. One of them is ArchiveIterator:

rust/src/librustc_llvm/ffi.rs

Lines 1687 to 1692 in 7afa0cc

pub fn LLVMRustArchiveIteratorNew(AR: ArchiveRef) -> ArchiveIteratorRef;
pub fn LLVMRustArchiveIteratorNext(AIR: ArchiveIteratorRef) -> ArchiveChildRef;
pub fn LLVMRustArchiveChildName(ACR: ArchiveChildRef, size: *mut size_t) -> *const c_char;
pub fn LLVMRustArchiveChildData(ACR: ArchiveChildRef, size: *mut size_t) -> *const c_char;
pub fn LLVMRustArchiveChildFree(ACR: ArchiveChildRef);
pub fn LLVMRustArchiveIteratorFree(AIR: ArchiveIteratorRef);


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
// problem is that we've got in-memory LLVM modules after we generate and
// optimize all codegen-units for one compilation in rustc. To be compatible
Expand Down Expand Up @@ -1280,6 +1331,11 @@ LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) {
report_fatal_error("ThinLTO not available");
}

extern "C" LLVMRustThinLTOModuleImports
LLVMRustGetLLVMRustThinLTOModuleImports(const LLVMRustThinLTOData *Data) {
report_fatal_error("ThinLTO not available");
}

extern "C" void
LLVMRustFreeThinLTOData(LLVMRustThinLTOData *Data) {
report_fatal_error("ThinLTO not available");
Expand Down