Re: [Qemu-devel] [PATCH v2 3/7] softfloat: Convert `*_default_nan' variables into inline functions

[Maciej W. Rozycki]

On Sat, 31 Jan 2015, Peter Maydell wrote:

> >> >  Hmm, so perhaps my idea for a later improvement:
> >> >
> >> >>  Eventually we might want to move the new inline functions into a
> >> >> separate header to be included from softfloat.h instead of softfloat.c,
> >> >> but let's make changes one step at a time.
> >> >
> >> > will actually have to be made right away.  I suspect GCC is more liberal
> >> > here due to its convoluted extern/static/inline semantics history.
> >> > Sigh...
> >>
> >> I would suggest just using "static inline", as we do elsewhere
> >> for little utility functions.
> >
> >  Yes, that's exactly what they'd have to be moved into a separate header
> > for.
> 
> Why do they need to be moved into a different header to do this?
> I must be missing something...

 This is because fpu/softfloat-specialize.h is an implementation header 
private to SoftFloat and therefore such inline definitions won't be seen 
by users outside SoftFloat, such as target-mips/msa_helper.c.  And IMO 
they shouldn't be moved into include/fpu/softfloat.h itself as this header 
contains generic stuff and is supposed to have no TARGET_foo stuff, as 
observed by current usage and inferred from comments in fpu/softfloat.c.

 So ultimately I think the newly converted `*_default_nan' inline 
functions will need to go into include/fpu/softfloat-public-specialize.h 
or suchlike, which will be pulled from include/fpu/softfloat.h for general 
availability.

 Overall I think the whole arrangement in fpu/softfloat-specialize.h has a 
potential to being cleaned up by removing all the chains of #ifdef's and 
splitting the conditional bits into separate headers matching the current 
variants.  E.g. the presence of this construct:

#if defined(TARGET_SPARC)
const float64 float64_default_nan = const_float64(LIT64( 0x7FFFFFFFFFFFFFFF ));
#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA)
const float64 float64_default_nan = const_float64(LIT64( 0x7FF8000000000000 ));
#elif SNAN_BIT_IS_ONE
const float64 float64_default_nan = const_float64(LIT64( 0x7FF7FFFFFFFFFFFF ));
#else
const float64 float64_default_nan = const_float64(LIT64( 0xFFF8000000000000 ));
#endif

asks for 4 independent headers defining the 4 bit pattern styles used by 
targets for symbolic FP data, one each -- perhaps even pulled indirectly 
via target headers rather than copying the chain of #ifdef's above around 
#include's from wherever they'll be pulled.

 Please correct me if I am wrong, but it appears to me possible right away 
by removing fpu/softfloat-specialize.h and instead creating individual 
target-*/softfloat-specialize.h headers that'll pull the right variant 
each target requires from a pool of templates made available in fpu/.  
Then include/fpu/softfloat.h could do the same for the public stuff (i.e. 
these NaN bits concerned here) placed similarly in include/fpu/.

 FWIW,

  Maciej