[RFC] LTO: deallocating llvm::Module inside lto_codegen_add_module

Project Infrastructure LLVM Dev List Archives
#1

LTOCodeGenerator::addModule (which is wrapped by lto_codegen_add_module) takes
an LTOModule as an argument and links the latter's llvm::Module into its own.
It does not change the latter's llvm::Module.

The lto_module_dispose API call destroys an LTOModule. This frees the
LTOModule's llvm::Module, but also invalidates strings returned by, e.g.,
lto_module_get_symbol_name.

I propose that LTOCodeGenerator::addModule be changed to also deallocate the
(consumed and redundant) llvm::Module and llvm::TargetMachine objects left
behind in LTOModule. I.e., something akin to:

    diff --git a/include/llvm/LTO/LTOModule.h b/include/llvm/LTO/LTOModule.h
    index c70afa4..1180e58 100644
    --- a/include/llvm/LTO/LTOModule.h
    +++ b/include/llvm/LTO/LTOModule.h
    @@ -164,6 +164,11 @@ public:
         return _asm_undefines;
       }
     
    + void destroyLLVMModule() {
    + _module.reset();
    + _target.reset();
    + }

#2

My 2 cents...

1. Is this considered a change to the C API (and, thus, banned)?

I think this would indeed be a significant semantic change to the C API.

2. Are there any consumers that rely on llvm::Module being accessible after
   LTOCodeGenerator::addModule? What about llvm::TargetMachine?

I think we have to assume so. That's the down side to saying it is a stable
C API.

3. Would the proposed behaviour be especially surprising?

No, the behavior you describe seems sensible. I would just expect it to
need to go under a separate/new API to preserve backwards compatibility.

#3

Thanks Chandler. I’ll go ahead with new API.