Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LRH.2.02.2001241604050.13947@key0.esi.com.au>
Date: Fri, 24 Jan 2020 17:08:54 +1100 (AEDT)
From: Damian McGuckin <damianm@....com.au>
To: musl@...ts.openwall.com
Subject: Re: Considering x86-64 fenv.s to C

On Thu, 23 Jan 2020, Rich Felker wrote:

> Note that it's not necessary to write that logic in asm just because
> it's in the arch-defined part. It could simply be C that fixes up the
> value before/after the asm.

My rule was to have no logic in the embedded assembler. Just get or set 
the register and quit. Your skill levels may let you do more. I tried to 
keep these routine super simple.  Here they are for the PowerPC.

 	static inline double get_fpscr_f(void)
 	{
 		double fpscr;

 		__asm__ __volatile__ ("mffs %0" : "=d"(fpscr));
 		return fpscr;
 	}
 	static inline unsigned int get_csr(void)
 	{
 		return (unsigned int) (union {double f; long i;}) {get_fpscr_f()}.i;
 	}
 	static inline void set_fpscr_f(double fpscr)
 	{
 		__asm__ __volatile__ ("mtfsf 255, %0" : : "d"(fpscr));
 	}
 	static inline void set_csr(unsigned int fpscr)
 	{
 		set_fpscr_f((union {long i; double f;}) {(long) fpscr}.f);
 	}

> But indeed the following may be nicer anyway:
......
>> 	if (excepts & FE_INVALID)
>> 		excepts |= FE_ALL_INVALID;
>> and
>> 	if (excepts & FE_INVALID)
>> 		excepts |= FE_INVALID_SOFTWARE;
>>
>> An optimize should see the OR with zero for most architectures and
>> then ignore that whole 'if' statement because the action was a
>> NO-OP.
>
> Yes, this looks nice (although I would probably include FE_INVALID in 
> FE_ALL_INVALID just out of principle of least surprise; it should 
> generate the same code either way.

Do you mean

  	if (excepts & FE_INVALID)
  		excepts |= FE_ALL_INVALID | FE_INVALID;

If so, will it really be optimized away if FE_ALL_INVALID is zero (0)?

>>>>    case (FE_OVERFLOW | FE_INEXACT):
>
> One more style thing: no need for parens here.

Sorry. Too much cutting and pasting from the original 'if' that I realized 
(at the last minute) should be a switch.

> Indeed, I think if we make that change it would be nice to factor it
> as a separate change later.

Please. Sounds wise.

> I do think I want to make it, though,

I think the idea of a SOFT_FPSR is a really good idea.

> feclearexcept would do:
>
> 	soft_fpsr = (soft_fpsr | get_sr()) & ~except;
> 	clear_sr();
>
> At that point, FE_INVALID is clearable independent of the
> specific-reason flags and doesn't seem to need any special treatment.

??? - I need to wrap my head around that one over the weekend.

> Note that just using clear_sr() rather than set_sr() here is a huge 
> optimization on i386 too -- it avoids the need to read-modify-write the 
> full fenv.

Definitely. The Intel architectures are a world unto themselves.

> You should consider arch/*/bits/fenv.h as essentially immutable; the
> types and macros there are ABI or API conventions. The arch-specific
> definitions for fenv stuff should go in arch/$(ARCH)/fenv_arch.h.

OK.

Regards - Damian

Pacific Engineering Systems International, 277-279 Broadway, Glebe NSW 2037
Ph:+61-2-8571-0847 .. Fx:+61-2-9692-9623 | unsolicited email not wanted here
Views & opinions here are mine and not those of any past or present employer

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.