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

[Philip Reames]

On 04/26/2015 12:59 PM, Hal Finkel wrote:
> ----- 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?
Both seem like useful constructs.  The first seems like it should 
probably be a function attribute, the second is specific to intrinsics.  
Being able to express a readonly call which is safe to speculate would 
be potentially useful.  Right now, I don't believe we can express that 
notion.  The profitability of actually using it is less clear, but 
still... maybe lifting a readonly no-trap call above a conditional if 
it's the only thing holding the condition computation live and the 
probability says it's extremely frequently taken?

Actually, my example points out a problem.  Do actually do that 
transformation, we'd need to know that the call terminated.  Do we want 
to model trapping as potentially non-terminating?  I can see arguments 
for either unifying them or keeping the notions separate. Thoughts?

Philip