-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
Changes from 1 commit
9df56ca
8dc7ddb
2c5cd9c
f6894eb
94b32ad
dd3f445
e045a6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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,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. | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||||||||
} | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have an example? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Searching for Lines 1687 to 1692 in 7afa0cc
|
||||||||||||||
|
||||||||||||||
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 | ||||||||||||||
|
@@ -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"); | ||||||||||||||
|
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 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)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.
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.