[PATCH] Inliner Enhancement

[Xinliang David Li]

On Thu, Mar 19, 2015 at 8:55 PM, Jiangning Liu <liujiangning1 at gmail.com>
wrote:

> Hi David,
>
> Regarding your patch, I think the simple loop heuristic may be a
>> reasonable incremental improvement, but you really need to wait for
>> Chandler's pass manager change instead of doing all the ordering tricks.
>>
>
> Loop analysis is critical for my patch and it is expensive. The ordering
> tricks (3.b) is only for compile-time reduction. As I mentioned in replying
> Yin's comments, (3.b) itself can actually reduce compile-time a lot (~5% or
> more for llvm bootstrap) without changing performance generally. If I don't
> add (3.b), we would see ~10% compile-time regression. I don't think it is
> acceptable. So now the only difference is how we update loop info. i.e. use
> my current naive solution, or use new pass manager to help it?
>
> So do you know when Chandler's pass manager change can be ready for
> inlining use?
>

It will be done in a very near future, but Chandler can give you more
precise info.




> Why we can't just use this native method first, and I can add //FIXME to
> highlight it, and then when Chandler's great new pass manager is ready for
> inlining use, we just switch to it? Eventually we would be able to see big
> compile-time reduction.
>


You need to discuss with Chandler or other inliner maintainers.  It is
probably possible but not in the current form of patch. IMO, the patch need
to be in the current form (because of the hack to workaround limitations).

1) guarded under a master flag (so that others can easily experiment with
it)
2) assumes the infrastructure is ready and just make very simple changes to
boost inlining with bonuses (similar to existing heuristics such as vector
bonus)
3) isolate your mock implementation of the loop info access in a
centralized place
4) get rid of the ordering change hack -- basically tolerate the compile
time increase assuming it will go away (and because it is under a flag).

This is just my personal opinion. Better discuss this with Chandler.

thanks,

David



>
>
>> On the other hand, I don't see the 'same callee' heuristics is generally
>> valid.  How much size contribution does it have? Is it just skewed by one
>> benchmark?
>>
>
> OK. I agree with this. I can get it removed, and actually the code size
> impact for SPEC overall is about ~2% for this rule. I just personally think
> it's unreasonable to inline a callee being called a lot of times naively.
> Actually this is just real case for one of SPEC2000 benchmarks, but
> generally I think it is meaningful. For abnormal request like building
> Android for one of the huge functions, if you want inline everything, then
> that's fine, we can just using command line option to enable it. But
> generally for common programming, I still think it's wired to inline this
> case.
>
> Thanks,
> -Jiangning
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150320/13226c29/attachment.html>