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

Philip Reames listmail at philipreames.com
Sun Apr 26 12:49:22 PDT 2015


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.  :)
>
>> >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
>




More information about the llvm-commits mailing list