[PATCH] D16439: [PlaceSafepoints] Introduce a -spp-no-statepoints flag

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

In http://reviews.llvm.org/D16439#333829, @reames wrote:

> In http://reviews.llvm.org/D16439#333783, @sanjoy wrote:
>
> > In http://reviews.llvm.org/D16439#333624, @reames wrote:
> >
> > > Sanjoy, I don't see anything wrong with this patch per say, but I'm a bit confused about how you're expecting this to fit together with RS4GC.  That pass currently has two modes, one which consumes statepoints, and one which consumes calls with deopt.  You don't seem to be emitting either form here.  What am I missing?
> >
> >
> > That's a really good question.
> >
> > For environments that don't care about deoptimization at all (which I
> >  think includes LLILC), RS4GC can be run with
> >  `-rs4gc-allow-statepoint-with-no-deopt-info`.  Then RS4GC will work as
> >  expected with calls / invokes with no `"deopt"` operand bundles
> >  (i.e. the deopt section in the gc.statepoints it creates will be empty
> >  for such calls).
> >
> > Environments that **do** care about deoptimization and want to run
> >  PlaceSafepoints will have to provide their own deopt info, and attach
> >  them to all non-leaf calls including the call in the safepoint poll
> >  slow path, if there is one.  For sanity checking, in these cases we
> >  should set `-rs4gc-allow-statepoint-with-no-deopt-info` to `false`
> >  (unless not having deopt info for some gc.statepoint is actually a
> >  valid thing to do in this case).
> >
> > And the third possibility is to not use PlaceSafepoints at all, and
> >  instead emit all safepoint polls early (with appropriate deopt state),
> >  and have an optimization pass remove unnecessary polls as appropriate;
> >  and run RS4GC afterwards.  For some strange reason, this third
> >  approach sounds very familiar. :)
> >
> > Are there other possibilities that you can think of?
>
>
> Hm, the proliferation of options here doesn't seem like a good thing.
>
> What is we tried the following:
>
> - PlaceSafepoints *only* inserts calls to poll functions.  No statepoints at all. (i.e. mostly this patch, but make it unconditional)


Yes, that's the eventual goal, as stated in the commit message.  But I
want to give the LLILC developers a chance to

a. Test that this new pass pipeline works for them
 b. Change the frontend to emit gc-transition arguments as operand bundles

> - RS4GC excepts only calls, optionally with deopt info.  We completely drop the old statepoint input case.  We allow GC's to opt into a requirement that all calls have deopt operands via a predicate on the gc name (and eventually, the strategy).


+1

> I'm also wondering if we should upstream the optimize poll placement pass we have (since it shares so much with PlaceSafepoints) and gut the smarts from PlaceSafepoints.  We could either have both passes call the same logic, or simply run one after the other.  I'm really bothered by the code duplication here.  I understand your argument (previously) that late insertion is better for folks who don't have deopt, but I'm bugged by the divergent code it has caused.


I think there is a possibility to have this upstream PlaceSafepoints
and the to-be-upstream-now-downstream OptimizeSafepointPolls pass
share //code// i.e. despite being logically different optimization
passes, they could call into a unified set of helper routines.


http://reviews.llvm.org/D16439