[PATCH] D24225: [LCG] Add the necessary functionality to the LazyCallGraph to support inlining.

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D24225#536730, @sanjoy wrote:

> Hi Chandler,
>
> I've not yet had a chance to look at this in detail, so I only have some superficial comments inline.  However, I don't (yet! :) ) believe you when you say that inlining only does the three trivial transforms you outlined above.  Because `InlineFunction` does some basic simplification I think it can introduce new SCCs.  For instance, if I run `opt -always-inline` (and nothing else) on
>
>   define void @f(void()* %p) alwaysinline {
>     call void %p()
>     ret void
>   }
>  
>   define void @g() {
>     call void @f(void()* @h)
>     ret void
>   }
>  
>   define void @h() {
>     call void @g()
>     ret void
>   }
>
>
> I get
>
>   ; ModuleID = 'inl.ll'
>   source_filename = "inl.ll"
>  
>   ; Function Attrs: alwaysinline
>   define void @f(void ()* %p) #0 {
>     call void %p()
>     ret void
>   }
>  
>   define void @g() {
>     call void @h()
>     ret void
>   }
>  
>   define void @h() {
>     call void @g()
>     ret void
>   }
>  
>   attributes #0 = { alwaysinline }
>
>
> That is, we form a //new// SCC containing `@g` and `@h`.


Thanks for the example! I hadn't really thought a lot about devirtualization happening during the simplification, but of course it can easily happen as you show here.

I stand by my claim that this patch tests the *basic* graph updates necessary for inlining. ;] This is a bit more than "just" inlining, not that our APIs make that visible to the caller in any useful way.

I'll add this as a high-level test case for the inliner port patch and update it to take this into account. That may in turn motivate more unittests of the core LCG functionality, but even if so I'd prefer to put those extra tests in a separate patch if that's OK?


https://reviews.llvm.org/D24225