Re: [PATCH] powerpc: Fix feraiseexcept and feclearexcept macros

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 04/03/2020 06:24, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 03/03/2020 15:20, Matheus Castanho wrote:
>>> A recent change to fenvinline.h modified the check if __e is a
>>> a power of 2 inside feraiseexcept and feclearexcept macros.  It
>>> introduced the use of the powerof2 macro but also removed the
>>> if statement checking whether __e != 0 before issuing an mtfsb*
>>> instruction.  This is problematic because powerof2 (0) evaluates
>>> to 1 and without the removed if __e is allowed to be 0 when
>>> __builtin_clz is called.  In that case the value 32 is passed
>>> to __MTFSB*, which is invalid.
>>>
>>> This commit uses __builtin_popcount instead of powerof2 to fix this
>>> issue and avoid the extra check for __e != 0.  This was the approach
>>> used by the initial versions of that previous patch.
> 
> Sorry about that.  I convinced myself that the change was valid, even
> after remembering that there was something weird with the powerof2
> macro.
> 
>> This code is becoming convoluted and I think these micro-optimization
>> are hardly wildly used and even more being a possible hotspot in 
>> realword cases (non-default rounding are used only on specific cases 
>> and excepting handling are done most likely only on exceptions cases).
> 
> It's not an optimization because it's a compile-time-evaluated
> expression.  The question is how to express this in a succinct way, now
> that the powerof2 macro is out.  __builtin_popcount (e) == 1 does not
> seem to be unreasonable to express this condition.
> 
> Or do you object to the existence of the feclearexcept macro as a whole?

I mean the fenvinline.h altogether and the macro dance to avoid function
calls.  It is similar to the ones on string{2,3}.h macros to optimize
some function calls and I think it would be worth to avoid such optimization
on glibc side and try to leverage them on compiler (as it does for various
math functions with builtins).