Re: [PATCH v2 4/6] [powerpc] libc_feholdsetround_noex_ppc_ctx: optimize FPSCR write

[Paul Clarke <pc@us.ibm.com>]

Thanks for the review, Paul!  Question...

On 9/23/19 10:54 AM, Paul E Murphy wrote:
> On 9/19/19 1:46 PM, Paul A. Clarke wrote:
>> libc_feholdsetround_noex_ppc_ctx currently does, basically:
> The listing reads awkwardly for me. I suggest a little cleanup. There is no need to post a new patch against my suggestion.

Here you say "listing", and there is a list (1-2-3) immediately below this text, and below you say "commit message", .  Where would you like to see improvement?  Happy to change, but it's hard for me to see what you're seeing.  (And, of course, it's all perfectly clear to me since I wrote it!  ;-)

>> 1. Read FPSCR, save to context.
>> 2. Create new FPSCR value: clear enables and set new rounding mode.
>> 3. Write new value to FPSCR.
>>
>> Since other bits just pass through, there is no need to write them.
>>
>> Instead, write just the changed values (enables and rounding mode),
>> which can be a bit more efficient.
>>
>> 2019-09-19  Paul A. Clarke  <pc@us.ibm.com>
>>
>>     * sysdeps/powerpc/fpu/fenv_private.h
>>     (libc_feholdsetround_noex_ppc_ctx): Call fesetenv_mode instead
>>     of fesetenv_register.
>> ---
>> v2: No change.
>>
>>   sysdeps/powerpc/fpu/fenv_private.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/powerpc/fpu/fenv_private.h b/sysdeps/powerpc/fpu/fenv_private.h
>> index 9496026..ade0bfa 100644
>> --- a/sysdeps/powerpc/fpu/fenv_private.h
>> +++ b/sysdeps/powerpc/fpu/fenv_private.h
>> @@ -142,7 +142,7 @@ libc_feholdsetround_noex_ppc_ctx (struct rm_ctx *ctx, int r)
>>     if (__glibc_unlikely (new.l != old.l))
>>       {
>>         __TEST_AND_ENTER_NON_STOP (old.l, 0ULL);
>> -      fesetenv_register (new.fenv);
>> +      fesetenv_mode (new.fenv);
>>         ctx->updated_status = true;
>>       }
>>     else
>>
> 
> OK, with suggested cleanup to commit message.
> 
> Reviewed-By: Paul E Murphy <murphyp@linux.ibm.com>

PC