[PATCH] D12205: LTO: Simplify merged module ownership.

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 24 15:24:39 PDT 2015


On Mon, Aug 24, 2015 at 03:15:08PM -0700, Duncan P. N. Exon Smith wrote:
> 
> > On 2015-Aug-24, at 14:32, Peter Collingbourne <peter at pcc.me.uk> wrote:
> > 
> > On Fri, Aug 21, 2015 at 05:48:10PM -0700, Duncan P. N. Exon Smith wrote:
> >>> Index: lib/LTO/LTOCodeGenerator.cpp
> >>> ===================================================================
> >>> --- lib/LTO/LTOCodeGenerator.cpp
> >>> +++ lib/LTO/LTOCodeGenerator.cpp
> >>> @@ -64,29 +64,20 @@
> >>> }
> >>> 
> >>> LTOCodeGenerator::LTOCodeGenerator()
> >>> -    : Context(getGlobalContext()), IRLinker(new Module("ld-temp.o", Context)) {
> >>> +    : Context(getGlobalContext()),
> >>> +      OwnedModule(new Module("ld-temp.o", Context)),
> >> 
> >> "OwnedContext" made sense as a name because the context isn't always
> >> owned.  We access the context through `Context`, but rely on the
> >> destructor in `OwnedContext` to clean up memory.
> >> 
> >> Here, we *always* own the module, so we don't need to distinguish.  I
> >> think a clearer name might be "MergedModule", a correctly-capitalized
> >> version of the local variable name used in a few places.
> > 
> > Done in new patch.
> > 
> 
> Latest patch LGTM (and I've checked with ld64 as well).  Thanks!

Thanks! r245891 (and an LLD API update in r245892).

-- 
Peter


More information about the llvm-commits mailing list