Re: [PATCH] PowerPC: removed branch prediction from rint implementation

[Adhemerval Zanella <azanella@linux.vnet.ibm.com>]

On 04/01/2013 12:20 AM, Ryan S. Arnold wrote:
> On Sun, Mar 31, 2013 at 8:56 PM, Adhemerval Zanella
> <azanella@linux.vnet.ibm.com> wrote:
>> On 03/31/2013 10:24 PM, Ryan S. Arnold wrote:
>>> On Sun, Mar 31, 2013 at 8:02 PM, Adhemerval Zanella
>>> <azanella@linux.vnet.ibm.com> wrote:
>>>> This patch removes the branch prediction instructions for PowerPC rint
>>>> implementation, replacing them for normal instructions. These instructions
>>>> are actually hurting performance, it is safer and generally better to let
>>>> the hardware handle it.
>>> This is true on Power7.  I believe it was not the case on older hardware.
>> Indeed, but assembly implementation is based on sysdeps/powerpc/fpu/s_rint.c, and the C
>> implementation built with gcc 4.7.3 plus -O3 -mcpu=powerX does not generate any branch
>> prediction instruction for neither power4, power5, power6, or power7.
>>
>> The assembly implementation make two assumptions: 1. 'fabs (x) < 2^52' is unlikely and
>> 2. 'x > 0.0' is unlike (if 1. is true). So usually the algorithm expect path is
>> 'x < 0.0' and 'abs(x) < 2^52'. Since it a general floating point function, expected input
>> is not bounded and I think the restrictions for branch prediction does not make sense.
>>
>>>> I added a rint benchmark test on benchtest framework. The results on a POWER7
>>>> 64 bits are:
>>>>
>>>> MASTER: rint: ITERS:1e+09: TOTAL:12.0901s, MAX:48.908ns, MIN:5.312ns, 8.27124e+07 iter/s
>>>> PATCH:  rint: ITERS:1e+09: TOTAL:5.29302s, MAX:37.826ns, MIN:3.922ns, 1.88928e+08 iter/s
>>>>
>>>> Any tips, comments, advices?
>>> We could conditionally exclude the branch hints on power7 and later
>>> hardware as an alternative.
>> Based on my previous comments, I don't think it is worth to add extra logic to handle specific
>> platforms.
> Alright, I stand corrected! This looks good to me.
>
> Ryan
>
Pushed as 60c414c346a1d5ef0510ffbdc0ab75f288ee4d3f.