[PATCH] D93838: [LLVM] [SCCP] : Add Function Specialization pass

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D93838#2701489 <https://reviews.llvm.org/D93838#2701489>, @SjoerdMeijer wrote:

> In D93838#2700463 <https://reviews.llvm.org/D93838#2700463>, @ChuanqiXu wrote:
>
>> In D93838#2695205 <https://reviews.llvm.org/D93838#2695205>, @SjoerdMeijer wrote:
>>
>>> @ChuanqiXu: I have started looking at increased compile-times, and experimented with the biggest outlier you reported:
>>>
>>>   500.perlbench_r	27%
>>>
>>> In my experiment I think I see less than 1% compile time increase with this patch.
>>> Would you mind double checking this for me with the latest patch? Would be good to get a confirmation to make sure we are on the same page.
>>
>> I just re-build 500.perlbench_r and find 6.5% increase based on the revision 330436. I think the successive revision just didn't contribute to compile-time. I would do other experiments to confirm this.
>> BTW, the first time I tried to do experiment I made a mistake which didn't pass the argument to the linker actually. I mean:
>>
>>   - LDOPTIMIZE: -flto=full -fuse-ld=lld -mllvm -function-specialize-level=aggressive
>>   + LDOPTIMIZE: -flto=full -fuse-ld=lld -Wl,-mllvm -Wl,-function-specialize-level=aggressive
>
> Many thanks for looking into this again. And 6.5% is already a lot better than 20+%. It is still not great of course and not really acceptable if that was the case, but I am suspecting that there's still quite a bit of noise going on there (I didn't see it triggering on 500.perlbench_r, so 6.5% to walk over a view functions sounds excessive to me, plus you were testing the aggressive mode for what's that worth).  What I am going to do is park the compile-time analysis for now, and focus on getting a first version to get the framework in. For that I am going to address a functional issue, your remark how a case like `foo(1, 2, 3, 4);` is currently treated. After the initial commit of the framework, we can start looking into caching some part of the analysis. This makes sense in my opinion, because the current data so far doesn't show it's a disaster that would be difficult to fix. Quite the opposite is think: the reason I like sqlite amalgamation is because it's a single ginormous file with a lot of functions, function specialisation triggers, is really stable as a compile time test as runs form more than a minute, and the 1 - 2% compile time increase is acceptable.

I would still do testing for compile-time to check it. It would be good if another one is interested in this patch and OK to run SPEC2017, which is a little hard to expect.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93838/new/

https://reviews.llvm.org/D93838