This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] [powerpc] get_rounding_mode: utilize faster method to get rounding mode
On 6/6/19 9:17 AM, Tulio Magno Quites Machado Filho wrote:
> "Paul A. Clarke" <pc@us.ibm.com> writes:
>> Add support to use 'mffsl' instruction if compiled for POWER9 (or later).
>>
>> Also, mask the result to avoid bleeding unrelated bits into the result of
>> _FPU_GET_RC().
>>
>> 2019-06-05 Paul A. Clarke <pc@us.ibm.com>
>>
>> * sysdeps/powerpc/fpu/fegetexcept.c (__fegetexcept): Replace code
>> with call to equivalent function.
>> * sysdeps/powerpc/fpu/fenv_libc.h (fegetenv_status): New function.
>
> As you explained, both entries have to be removed.
Yes, sorry about the cruft.
>> * sysdeps/powerpc/fpu_control.h (_FPU_MASK_RC): New.
>> (__FPU_MFFS): New.
>> (__FPU_MFFSL): New.
>> (_FPU_GET_RC): New.
>> (_FPU_GETCW): Use __FPU_MFFS().
>
> ^ Extra spaces here. Should be only 1 tab.
OK.
>> * sysdeps/powerpc/powerpc64/get-rounding-mode.h: New file.
>
> I also think this file is unnecessary.
(discussed below)
>> diff --git a/sysdeps/powerpc/powerpc64/get-rounding-mode.h b/sysdeps/powerpc/powerpc64/get-rounding-mode.h
>> new file mode 100644
>> index 0000000..e2fdbbb
>> --- /dev/null
>> +++ b/sysdeps/powerpc/powerpc64/get-rounding-mode.h
>> @@ -0,0 +1,33 @@
>> +/* Determine floating-point rounding mode within libc. powerpc64 version.
<snip>
>> +static inline int
>> +get_rounding_mode (void)
>> +{
>> + return _FPU_GET_RC ();
>> +}
>> +
>> +#endif /* get-rounding-mode.h */
>
> I don't understand why this file/function is needed.
> Looks like another file that should be removed from this patch.
Without this file, the generic file is used, which calls _FPU_GETCW(), which is used generically to get the entire floating point status control register (FPSCR) and must resolve to using the slower "mffs". get_rounding_mode() only needs the rounding mode, and the new-in-this-patch _FPU_GET_RC() used by this new file resolves to the faster "mffsl" (if compiled for POWER9).
> LGTM if you fix the ChangeLog and keep only the changes to
> sysdeps/powerpc/fpu_control.h.
>
> Reviewed-by: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>
PC