[powerpc] fe{en,dis}ableexcept optimize bit translations
Commit Message
On 8/28/19 7:58 AM, Adhemerval Zanella wrote:
> On 03/08/2019 00:22, Paul A. Clarke wrote:
>> The exceptions passed to fe{en,dis}ableexcept() are defined in the ABI
>> as a bitmask, a combination of FE_INVALID, FE_OVERFLOW, etc.
>> Within the functions, these bits must be translated to/from the corresponding
>> enable bits in the Floating Point Status Control Register (FPSCR).
>> This translation is currently done bit-by-bit. The compiler generates
>> a series of conditional bit operations. Nicely, the "FE" exception
>> bits are all a uniform offset from the FPSCR enable bits, so the bit-by-bit
>> operation can instead be performed by a shift with appropriate masking.
>>
>> 2019-08-02 Paul A. Clarke <pc@us.ibm.com>
>>
>> * sysdeps/powerpc/fpu/fenv_libc.h: Define FPSCR bitmasks.
>> (fenv_reg_to_exceptions): Replace bitwise operations with mask-shift.
>> (fenv_exceptions_to_reg): New.
>> * sysdeps/powerpc/fpu/fedisblxcpt.c (fedisableexcept): Replace bitwise
>> operation with call to fenv_exceptions_to_reg().
>> * sysdeps/powerpc/fpu/feenablxcpt.c (feenableexcept): Likewise.
>>
>> This patch is a prerequisite for the two patches I sent over the past two days:
>> - [powerpc] fe{en,dis}ableexcept, fesetmode: optimize FPSCR accesses
>> - [powerpc] SET_RESTORE_ROUND improvements
>> Apologies for sending these out-of-order. I forgot about this one during the
>> freeze window.
>
> LGTM with a suggestion below.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
>> index 9861f18..853239f 100644
>> --- a/sysdeps/powerpc/fpu/fenv_libc.h
>> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
>> @@ -131,57 +131,103 @@ __fesetround_inline_nocheck (const int round)
>> /* Definitions of all the FPSCR bit numbers */
>> enum {
>> FPSCR_FX = 0, /* exception summary */
>> +#define FPSCR_FX_MASK (1 << (31 - FPSCR_FX))
>> FPSCR_FEX, /* enabled exception summary */
>> +#define FPSCR_FEX_MASK (1 << (31 - FPSCR_FEX))
[...]
>> +#define FPSCR_RN_MASK (FPSCR_RN_hi_MASK|FPSCR_RN_lo_MASK)
>> +#define FPSCR_ENABLES_MASK \
>> + (FPSCR_VE_MASK|FPSCR_OE_MASK|FPSCR_UE_MASK|FPSCR_ZE_MASK|FPSCR_XE_MASK)
>> +#define FPSCR_BASIC_EXCEPTIONS_MASK \
>> + (FPSCR_VX_MASK|FPSCR_OX_MASK|FPSCR_UX_MASK|FPSCR_ZX_MASK|FPSCR_XX_MASK)
>> +
>> +#define FPSCR_CONTROL_MASK (FPSCR_ENABLES_MASK|FPSCR_NI_MASK|FPSCR_RN_MASK)
> It is ok, but one suggestion I have is it would be simple to just define a
> macro to create the mask and use it. Like:
>
> #define FPSCR_MASK(bit) (1 << (31 - (bit)))
>
> #define FPSCR_RN_MASK (FPSCR_MASK (FPSCR_RN_hi) | FPSCR_MASK (FPSCR_RN_lo))
> #define FPSCR_ENABLES_MASK \
> (FPSCR_MASK(FPSCR_VE) | FPSCR_MASK (FPSCR_OE) | FPSCR_MASK (FPSCR_UE) \
> | FPSCR_MASK (FPSCR_ZE) | FPSCR_MASK (FPSCR_XE))
> #define FPSCR_BASIC_EXCEPTIONS_MASK \
> (FPSCR_MASK (FPSCR_VX) | FPSCR_MASK (FPSCR_OX) | FPSCR_MASK (FPSCR_UX) \
> | FPSCR_MASK (FPSCR_ZX) | FPSCR_MASK (FPSCR_XX))
Thanks very much for the review, Adhemerval!
Apologies are due, as I forgot to include your "Reviewed-by" in the commit. :-/
This is the essence of what I checked in. I just used the newly suggested
FPSCR_MASK for all of the bits, and the more complex masks stayed the same.
@@ -128,60 +128,108 @@ __fesetround_inline_nocheck (const int round)
asm volatile ("mtfsfi 7,%0" : : "i" (round));
}
+#define FPSCR_MASK(bit) (1 << (31 - (bit)))
+
/* Definitions of all the FPSCR bit numbers */
enum {
FPSCR_FX = 0, /* exception summary */
+#define FPSCR_FX_MASK (FPSCR_MASK (FPSCR_FX))
FPSCR_FEX, /* enabled exception summary */
+#define FPSCR_FEX_MASK (FPSCR_MASK FPSCR_FEX))
[...]
+#define FPSCR_RN_MASK (FPSCR_RN_hi_MASK|FPSCR_RN_lo_MASK)
+#define FPSCR_ENABLES_MASK \
+ (FPSCR_VE_MASK|FPSCR_OE_MASK|FPSCR_UE_MASK|FPSCR_ZE_MASK|FPSCR_XE_MASK)
+#define FPSCR_BASIC_EXCEPTIONS_MASK \
+ (FPSCR_VX_MASK|FPSCR_OX_MASK|FPSCR_UX_MASK|FPSCR_ZX_MASK|FPSCR_XX_MASK)
+
+#define FPSCR_CONTROL_MASK (FPSCR_ENABLES_MASK|FPSCR_NI_MASK|FPSCR_RN_MASK)
PC