[llvm] r228090 - Add a pass for inserting safepoints into (nearly) arbitrary IR

[Hal Finkel]

----- Original Message -----
> From: "Philip Reames" <listmail at philipreames.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Sunday, April 26, 2015 2:49:22 PM
> Subject: Re: [llvm] r228090 - Add a pass for inserting safepoints into (nearly) arbitrary IR
> 
> On 04/26/2015 12:38 PM, Hal Finkel wrote:
> >>>> +    // We need to stop going forward as soon as we see a call
> >>>> > >>that
> >>>> > >>can
> >>>> > >>+    // grow the stack (i.e. the call target has a non-zero
> >>>> > >>frame
> >>>> > >>+    // size).
> >>>> > >>+    if (CallSite CS = cursor) {
> >>>> > >>+      (void)CS; // Silence an unused variable warning by
> >>>> > >>gcc
> >>>> > >>4.8.2
> >>>> > >>+      if (IntrinsicInst *II =
> >>>> > >>dyn_cast<IntrinsicInst>(cursor)) {
> >>>> > >>+        // llvm.assume(...) are not really calls.
> >>>> > >>+        if (II->getIntrinsicID() == Intrinsic::assume) {
> >>> > >
> >>> > >assume is not the only intrinsic, I imagine, that has this
> >>> > >property. Maybe you want to refactor things to call
> >>> > >isAssumeLikeIntrinsic in lib/Analysis/ValueTracking.cpp?
> >> >Hm, probably not.  The "right thing" is probably to use
> >> >TLI->isLoweredToCall().  The real issue here isn't trapping, it's
> >> >stack
> >> >growth.  This is trying to make sure we get a safepoint before
> >> >any
> >> >potentially recursive call (i.e. potential stack overflow).
> >> >
> >> >Side node: isAssumeLikeIntrinsic is not exactly a name with an
> >> >obvious
> >> >meaning.  Any chance you could change that to something like
> >> >isNonTrappingIntrinsic?
> > I'd be happy to; I completely agree. Also, as the FIXME in the
> > function says, the current list is copied from (and should perhaps
> > be kept in sync with) NoTTI::getIntrinsicCost. In and of itself,
> > this should also be fixed (I only want to have this list in one
> > place). Better yet, I should probably make TableGen generate it.
> So, we should really be passing TTI in from the call sites of
> ValueTracking?
> 
> (Having the non-target specific aspects live somewhere other than TTI
> would also seem reasonable.)
> >
> > Although you're right that isAssumeLikeIntrinsic is supposed to
> > represent the fact that the intrinsics can't trap, it is currently
> > the same list as TTI has of "artificial" intrinsics (calls that
> > serve only to provide the optimizer (or debugger, etc.) with
> > information, and for which no code is generated). As a practical
> > matter, isAssumeLikeIntrinsic exists because there are intrinsics
> > that are tagged as potentially having side effects, but only to
> > maintain control dependencies. Maybe we should add some kind of
> > ControlDependenciesOnly attribute these intrinsics?
> That would be reasonable.  I think the problem is that no one has
> volunteered to the do the work.  :)

I'll do it. Not this week, likely, but likely next week. There are two aspects here potentially worth representing separately. One is the "cannot trap", and the other is "informational only" (which I view as being stronger than "free", because it also implies that side-effect-free instructions that contribute only to computing the arguments of the intrinsic can be expected to themselves not contribute to the final code generation). Thoughts?

 -Hal

> >
> >> >If you move the isSafeToSpeculate call
> >> >inside
> >> >this function, it would make the purpose more obvious in the
> >> >callers.
> >> >
> >> >p.s. Doesn't LICM have or need something very like that function?
> >> >Can
> >> >we
> >> >common those.
> > The other place this exists is within TTI, and I think that other
> > passes use it from there.
> TTI was actually what I was thinking of.
> >
> > Thanks again,
> > Hal
> >
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory