[PATCH] Move ownership of GCStrategy objects to LLVMContext

[Chandler Carruth]

When I said "construct" I really did just mean "build an instance of one of
the known strategies", and was imagining it then being put into the map and
re-used everywhere. I think we essentially in agreement about how this
works.

On Thu, Jan 15, 2015 at 3:23 PM, Philip Reames <listmail at philipreames.com>
wrote:

> If so, have you looked at possibly making this a function analysis that
>> has a cache of these which it populates exactly the same way you populate
>> the context? That should still allow you to use essentially the same lookup
>> path here.
>>
> My understanding is that this would not work inside the Verifier if used
> via verifyFunction(F).  Is that untrue?  If not, your approach would not be
> sufficient.


Ahh, you want it in the verifier...

So, I'm not sold on this being in the verifier at all. But that seems quite
likely a separate discussion. However, even if it is in the verifier, I
think it should still be an analysis pass, and the verifier should
explicitly construct it. This is how the verifier uses the dominator trees.
The nice thing is that it keeps the verifier from depending on the current
state of an analysis pass which might hide actual errors in the IR. So the
verifier just directly computes the GCStrategy, and then you have an
analysis that provides access to a nicely cached one.


>
>  The reason I suggest this approach is that it helps simplify the core IR
>> a bit by letting the IR just deal in an opaque attribute and relying on an
>> analysis to reason about it. I think the code would be almost identical to
>> what you have here just shuffled around a bit. I'd be happy with that
>> either as the first version or with that happening as the immediate
>> follow-up refactoring after this patch.
>>
> I would strongly prefer to land this as is, then incrementally improve.
> I've got several changes I'd like to make - which are themselves
> uncontroversial and almost boring - which are blocked by this.


Again, I don't really care which order. But I think it would be better to
not have this in the Function API, and instead be an analysis over the
function.


>
>
>> Also a few trivial comments inline.
>>
> I will fix these.  By preference I'd do them as a follow up commit since
> I'm just moving existing code.


One of them was in the changed code which is no longer owning the
GCStrategies. For moving code, I'm OK with fixing before or after the move.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150115/e355a79a/attachment.html>