[PATCH] libLTO: Allow linker to choose context of modules and codegen

Duncan P. N. Exon Smith dexonsmith at apple.com
Tue Nov 11 15:12:28 PST 2014


Thanks for the review!

Committed r221726, r221728, r221730 and r221733, with all your changes
included.

> On 2014-Nov-11, at 12:16, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
> 
> It is awesome to see this happen!
> 
> +LTOCodeGenerator::LTOCodeGenerator(std::unique_ptr<LLVMContext> Context)
> +    : OwnedContext(std::move(Context)), Context(*OwnedContext),
> +      IRLinker(new Module("ld-temp.o", *OwnedContext)), TargetMach(nullptr),
> +      EmitDwarfDebugInfo(false), ScopeRestrictionsDone(false),
> +      CodeModel(LTO_CODEGEN_PIC_MODEL_DEFAULT), DiagHandler(nullptr),
> +      DiagContext(nullptr) {
> 
> This is mostly duplicated with the existing constructor. Can you add
> an init the sets everything bug the OwnedContext and have both
> constructors call it?
> 
> Not sure I understand why patch 3 is necessary. Using mismatched
> context sounds like a bug in the caller, and not an error. As such it
> should be an assert.
> 
> + * All modules added using \a lto_codegen_add_module() must have been created
> + * in the global context (default) or via \a
> + * lto_module_create_in_codegen_context() with this code generator.
> 
> Maybe just " All modules added using \a lto_codegen_add_module() must
> have been created in the same context as the codegen"? As written it
> sounds like you can add a module created in the global context to a cg
> that uses a local one.
> 
> LGTM other than that!
> 
> 
> On 11 November 2014 14:16, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> The attached patches add API for specifying which `LLVMContext` each
>> `lto_module_t` and `lto_code_gen_t` is in.  combined.patch has the
>> combine diff.
>> 
>> This enables the following flow:
>> 
>>    for (auto &File : Files) {
>>      lto_module_t M = lto_module_create_in_local_context(File...);
>>      querySymbols(M);
>>      lto_module_dispose(M);
>>    }
>> 
>>    lto_code_gen_t CG = lto_codegen_create_in_local_context();
>>    for (auto &File : FilesToLink) {
>>      lto_module_t M = lto_module_create_in_codegen_context(File..., CG);
>>      lto_codegen_add_module(CG, M);
>>      lto_module_dispose(M);
>>    }
>>    lto_codegen_compile(CG);
>>    lto_codegen_write_merged_modules(CG, ...);
>>    lto_codegen_dispose(CG);
>> 
>> This flow has a few benefits.
>> 
>>  - Only one module (two if you count the combined module in the code
>>    generator) is in memory at a time.
>> 
>>  - Metadata (and constants) from files that are parsed to query symbols
>>    but not linked into the code generator don't pollute the global
>>    context.
>> 
>>  - The first for loop can be parallelized, since each module is in its
>>    own context.
>> 
>>  - When the code generator is disposed, the memory from LTO gets freed.
>> 
>> I'm not really sure how best to test this.  Let me know if you have
>> any ideas.
>> 





More information about the llvm-commits mailing list