[PATCH] Move ownership of GCStrategy objects to LLVMContext

[Chandler Carruth]

(correct, none of the below has anything to do with this patch)

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

> So, I'm not sold on this being in the verifier at all. But that seems
> quite likely a separate discussion.
>
> Why?  Feel free to forward this to llvm-dev depending on the answer and
> it's length.
>
> Being able to state things like: "statepoints in functions with GC X only
> relocate values in an address space X expects" seems to be entirely
> reasonable and desirable to me.


Absolutely, but I don't think it belongs in the IR verifier. The verifier
needs to be extremely light weight and I don't think it makes sense to
check these kinds of constraints there. The contract of the verifier is
that it validates those properties which you should feel free to assert()
on when writing other passes. I don't think we should add such properties
lightly.

But of course it makes sense to check these properties. One way is through
the verifyAnalysis functionality in the old pass manager (and there will be
some largely-automated analog in the new one). This allows an analysis to
check that its analysis specific invariants continue to hold. Another
strategy is the (somewhat poorly named) Lint pass. The idea is that for
some invariants it may make more sense to add a custom pass which does
nothing other than check and diagnose violations of an invariant.

If checking this invariant in other ways were really hard to do, then maybe
it would make sense in the verifier. But I'm not seeing anything like that
at this point.


>
>
>   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.
>
> So, first of all, the Verifier does not use the analysis pass mechanism.
> It natively constructs and recalculates a DominatorTree which would
> normally be obtained through the DomTreeWrapperAnalysis pass.  Is this what
> you meant?
>

To an extent.


>
> The analogous thing would be a GCStrategyCache and a wrapping
> GCStrategyCacheWrapper pass.  I have no objection to doing this if that's
> what the community would prefer, but I believe this is undesirable.  My
> reasoning is:
> 1) There is no possibility of invalidation.  A GCStrategy is immutable and
> can only be changed at compile time (of LLVM itself).


There are actually quite a few things that have no invalidation criteria
but I think make sense being exposed via the pass machinery.
TargetLibraryInfo is an example I've just been touching.


> 2) If we have multiple instances of GCStrategyCache, we have multiple
> copies of each GCStrategy floating around with different lifetimes.  This
> seems potentially confusing at best.  (Particularly for any out of tree
> users... I'd really like the lifetime model to be simple.)
>

I don't think this is necessary. You could easily have these be strict
lazily created singletons.


> 3) We're adding an IMHO utterly unnecessary abstraction which only
> increases the complexity of an already fairly complex mechanism.  I see no
> benefit here.


I think there is a significant benefit to separating the concerns of GC
strategy from the concerns of the core IR. I also clearly don't think the
complexity is going to be that large.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150115/b4b8bb7d/attachment.html>