[PATCH] D16890: Fix PR25339: ARM Constant Island

[Zhao, Weiming via llvm-commits]

Hi Renato,

I uploaded the test case to bugzilla. 
https://llvm.org/bugs/show_bug.cgi?id=25339

This patch doesn't reduce the threshold to 15. It tries to accelerate 
the converge after 15th iteration by ignoring the "best growth".

Thanks,
Weiming

On 2/8/2016 6:51 AM, Renato Golin wrote:
> rengolin added a comment.
>
> In http://reviews.llvm.org/D16890#345099, @weimingz wrote:
>
>> I'll post the test case shortly. Since it's based on proprietary code,
>>   it needs to complete some internal review process.
>
> I completely understand. No worries.
>
>> So far, I can only give a brief description of the problematic code.
>>   Please see https://llvm.org/bugs/show_bug.cgi?id=25339
>
> Ah, excellent! That bug was on my radar, and I'm glad someone is looking at it. When you submit a review based on a bug, it's good to put the bug number like "Addresses bug #NNNN", so we can grab the rest of the context.
>
>> The number 15 is half of the max threshold of 30.
>
> Right. If the max is hard-coded, then maybe say max/2 in there?
>
> It's also not clear to me why 15 is better than 30. If the algorithm fails to converge on both, than clearly either hard-coded numbers would be ok as long as the safe option is implemented. This patch seems to be doing both: forcing a convergence, even if a bad one (which would also work with 30 iterations, I think), and reducing the threshold to 15 (which doesn't make much sense per se).
>
>> I did a measurement of building SPEC and the highest iteration number I
>>   see is 3.
>
> This means that the application itself is problematic in that respect, so any number beyond 5 would be a good bet, with higher numbers better because they would only hit the worse cases.
>
> What's the impact if you keep the original threshold?
>
> cheers,
> --renato
>
>
> Repository:
>    rL LLVM
>
> http://reviews.llvm.org/D16890
>
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation