<div dir="ltr">Hi Rafael,<div><br></div><div>Sorry about the delayed review.</div><div><br></div><div>Does this patch need to touch LTOModule? In any case, I think there's a bug here: LTOModule has a create method that takes a file name, which means LTOModule needs to own its underlying memory. Either LTOModule should own its MemoryBuffer, or  we should remove the LTOModule::createFromFile method. The former is consistent with the current behavior - it's probably easier if we stick with that for now.</div>
<div><br></div><div>Regarding the RuntimeDyld changes - if we can avoid it at all, I'd really like to keep the Object ownership up in the ExecutionEngine, rather than in RuntimeDyld. The future direction for the JIT is pushing ownership responsibility up to clients (or convenience adapters) and out of the JIT itself. We can't go all the way with that yet, but we should manage the ownership at the highest level that we can in-tree, which is the ExecutionEngine.</div>
<div><br></div><div>If you're happy to re-work the patch to move the ownership out that would be great, but otherwise I'll try to take a look some time soon.</div><div><br></div><div>Cheers,</div><div>Lang.</div><div>
<br></div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Aug 20, 2014 at 10:00 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On 20 August 2014 19:37, Lang Hames <<a href="mailto:lhames@gmail.com">lhames@gmail.com</a>> wrote:<br>

> Hi Rafael,<br>
><br>
> The ExecutionEngine already owns the LL module, right? I think we should<br>
> actually make addObjectFile an owning operation too, for consistency.<br>
<br>
</div>What do you think of the attached patch?<br>
<div class=""><br>
> Side note: In the long term I'd love to move to a world where<br>
> ExecutionEngine and its subclasses are non-owning and we provide convenience<br>
> classes to automatically manage ownership for clients with simple use cases.<br>
> A lot of JIT clients probably know better than ExecutionEngine how they want<br>
> their memory managed, and for them automatic ownership is just a<br>
> straightjacket.<br>
<br>
</div>Yes. In general I feel that having Foo own Bar was more useful when<br>
memory management in C++ was manual. You only had to remember to<br>
delete Foo and Bar would go away. With std::unique_ptr and std::move<br>
most of that advantage goes away.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div>