[PATCH] Move ownership of GCStrategy objects to LLVMContext
listmail at philipreames.com
Wed Jan 14 10:39:24 PST 2015
On 01/13/2015 06:08 PM, Chandler Carruth wrote:
> On Tue, Jan 13, 2015 at 5:58 PM, Philip Reames
> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
> This is pretty much exactly what I have in mind. I had hoped that
> this would be suitable as an intermediate step, it sounds like the
> answer is no. I'll look at trying to pull GCStrategy further into
> IR to avoid the lowering issue. If I can get the header across the
> line, do either of you object if some of the *implementation*
> stays in CodeGen for the moment? (As in, for a few days, until I
> can follow on changes in.) I'm really trying to avoid one massive
> change that rips the entire thing apart and puts it back together.
> This will break builds of folks that don't link CodeGen at all.
I had frankly completely forgotten about this use case. Do we have an
in tree example of this? If I get a clean build, can I be certain I'm
not breaking these users?
Having said that, I think the move I would actually do would still be
fine. The only users of the implementation which would stay
(temporarily) in CodeGen are in CodeGen themselves.
> Since CodeGen depends on IR already, you can safely do a mechanical
> move from one library to the other, and then do the refactorings you
> have in mind if that makes things cleaner for you?
Possibly. I'll play with this more when I get a chance. It may take me
a few days to actually get back to this, the time I'd had to devote to
this has since mostly disappeared. And I've missed the 3.6 window, so
there's really no hurry any more.
> Just to be clear, does anyone have concern about the *actual
> ownership* change?
> I actually do have some concerns, but I don't really have any good
> answers yet. Here is my current thinking, which I'm not confident in
> and may be quite broken:
> I feel like this isn't an IR construct so much as a utility for
> examining or analyzing the IR construct, and the IR construct is just
> a function attribute. Would it make sense to structure the code that
> way? I'm imagining a GCStrategyAnalysis and MachineGCStrategyAnalysis
> or something similar which can operate both at the IR level and at the
> MI level. But I've not worked enough with GCStrategy to know whether
> this works, or if it doesn't, why it doesn't and whether the problem
> is fundamental or fixable.
I would phrase this a bit different (but I'm also still working through
what exactly GCStrategy was/is/will be). I view the current GCStrategy
as being a) a collection of facts about the GC for a particular
function, b) choices that effect lowering strategy applied and c) the
actual implementation of gc.root lowering. Currently, all of these apply
only to MI level transforms, but with gc.statepoint, I want to
generalize this. (a) will be used to enable selected optimizations
based on properties of the GC. For examples, see the gc.relocate cases
in InstCombineCalls and their TODOs. It will also be used to add
stricter verification in the verifier for gc.statepoint. (i.e. no
mixing gc.root and gc.statepoint!). (b) will still have the same
purpose, but for both gc.root, the barrier stuff, and gc.statepoint (c)
will be split out into it's own set of passes. Long term, I'd don't
believe the lowering logic should be part of GCStrategy at all.
(Actually, looking at the code again, the lowering parts are much less
coupled than I'd remembered. It might make sense to do that split *first*.)
Currently, there is a GCModuleInfo analysis which could be used to get
the GCStrategy. Having to add an analysis to every transform, just to
get a collection of facts about the GC associated with that function
seems less than ideal. I'm not utterly opposed to this approach - i.e.
I'll do what I have to get things in tree - but this *really* doesn't
seem like a clean approach to me. Also, the current GCModuleInfo
contains details of the gc.root lowering that I want to factor out of
anything shared by gc.root and gc.statepoint. That's addressable through
different channels of course.
Overall, my view is that GCStrategy should be a collection of facts
about a GC. Each function should have easy access to that collection of
facts. The exact mechanism isn't particularly important.
One secondary benefit of having GCStrategy be an IR level object would
be the possibility to have a new GCStrategy provided from an external
consumer of the JIT APIs. I wouldn't say this is a *goal* right now,
but I'd like to not design it out of existence either. If GCStrategies
are owned by analysis passes, we'd have to expose a creation callback or
the registry itself (yuck!). If GCStrategies are just IR objects, then
one could simply be added to the LLVMContext during IR generation.
p.s. Thanks for taking the time to think about this. Having someone to
discuss with helps clarify things for me and will definitely lead to an
overall better design.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits