This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 24/28] powerpc: hypot refactor and optimization
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: "Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>
- Cc: libc-alpha at sourceware dot org
- Date: Mon, 8 Jul 2019 12:37:29 -0300
- Subject: Re: [PATCH 24/28] powerpc: hypot refactor and optimization
- References: <20190329133529.22523-1-adhemerval.zanella@linaro.org> <20190329133529.22523-25-adhemerval.zanella@linaro.org> <20190626195925.kpvfzjk7kcjwb3mg@tereshkova>
On 26/06/2019 16:59, Gabriel F. T. Gomes wrote:
> On Fri, Mar 29 2019, Adhemerval Zanella wrote:
>>
>> - Two reduntant checks (for y == 0 and y > two60factor && (x / y) > two60)
> ~~~~~~
> I don't understand why y == 0 is redundant. It looks like an optimization
> as much as the check for x > y * 0x1p+60. Should we actually remove the
> check for y == 0?
Indeed redundant is not the correct word, it is an optimization. I have
reinstate it.
>
>> * sysdeps/powerpc/fpu/e_hypot.c (two60, two500, two600, two1022,
>> twoM500, twoM600, two60factor, pdnum): Remove.
>> (TEST_INFO_NAN): Remove macro.
>
> Missing mention to the removal of GET_TW0_HIGH_WORD?
Ack.
>
>> - if (y == 0.0)
>> - return x;
>
> As mentioned above, is it actually redundant?
>
>> + if (y <= 0x1.fffffffffffffp+963 && x > (y * 0x1p+60))
>> + return x + y;
>
> OK. Will not overflow.
>
> Other than that the patch looks good to me.
>
> Reviewed-by: Gabriel F. T. Gomes <gabrielftg@linux.ibm.com>
>