[PATCH] D36726: [Inliner] Teach the inliner to propagate attributes that have specific effects on inlining thresholds when we happen to inline into the entry (extended) basic block.

[Xinliang David Li via llvm-commits]

On Mon, Aug 14, 2017 at 6:44 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> On Mon, Aug 14, 2017 at 6:38 PM Xinliang David Li via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> On Mon, Aug 14, 2017 at 6:31 PM, Chandler Carruth via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> chandlerc added a comment.
>>>
>>> In https://reviews.llvm.org/D36726#841627, @davidxl wrote:
>>>
>>> > I will provide more comments on propagating the inline hint later.
>>> However, I do think it is wrong to propagate the cold attribute in inliner
>>> -- there should be already an inter-procedural attribute propagation pass
>>> that does this.
>>>
>>>
>>> While I'd love to teach our IPO attribute propagation to do that, we
>>> might still need to do it here. The inliner might make the opportunity for
>>> this propagation visible and then delete the call removing the chance to do
>>> it all within a single pass run, and the interprocedural pass never get a
>>> chance to see the intermediate state.
>>>
>>
>>
>> Not sure I understand this. Do you have an example showing that IPO prop
>> won't work?
>>
>
> The inliner initially sees a direct call from A to B. The returned value
> is a pointer which is in turn called. Neither A nor B are cold. Be returns
> the address of a function C which is cold.
>
> After inlining, A has a direct call to C (a cold function). The inliner
> can iterate to this without any other pass running and inline C into A.
>


I doubt this most simplistic scenario is common. Besides, Most likely, the
inlining to the cold callsite won't actually happen -- after we discover
that A is also cold, why would we want to inline a cold callsite into a
cold caller?  Or unless the implementation does not require the inline
transformation to happen to propagate the cold attribute.



> There is no longer a call edge to a cold function, we've lost the chance
> to propagate anything. =/
>

IMO, a powerful IPA should not depend too much on inlining, but should
instead more integrated with inter-procedural constant/value
propagation(analysis only).  There are drawbacks of relying on inlining to
do the propagation:

1) The (exposed) cold callsite is not inlined into the caller;
2)  the cold attribute gets discovered after some damage has been done.
With your example:

      a() {
         ptr = b();
         ptr();
      }

      b() {
         return &cold_c;
      }

 When cold_c is exposed to a(), we found that a() is cold too, but b() is
already inlined into a() resulting in unnecessary code bloat.  If the IPA
propagation discovers ptr's set include only cold callees, a() is marked as
cold, and b won't be inlined into a later.

Slightly more complicated cases will be missed:

      a() {
         ptr = b(s);
         ptr();
      }

      b(int s) {
        if (s)
         return &cold_c;
        else
         return &cold_d;
      }

or it has a deep call chain to get the function pointer, or for some reason
'b' can not be inlined.


3) Another example. Suppose a() is a hot function, b() is a cold function
discovered via inlining and cold attribute propagation:

    a() {
      if (...) {
          b(&cold_c);       // cold site
      }
   }

   b( void(*ptr)() ) {
     ptr();
   }


Here, we can add more code to teach inliner to discover the callsite to 'b'
is cold, but it will fail with slightly more complicated example where
bottom up propagation of values is not enough (tied with the inlining):

     x() {
         ...
       a(&cold_c);      // we see all callsites to 'a' with cold function
ptr passed in
       ..
     }

    a(void (*ptr) () ) { // hot

        if (...) {
            b(ptr);     // cold callsite -- must call cold callees from
here, but bottom up inlining won't see 'ptr''s value set.
        }
    }

    b(void (*ptr)()) {
      ptr();
    }


In this example, the cold callsite 'b' may get inlined into 'a', which
blocks it from being inlined into 'x'.

David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170815/1913fd1e/attachment.html>