Re: Which POWER implementations need the HWCAP_DFP check?

[Florian Weimer <fweimer at redhat dot com>]

On 05/14/2018 05:22 PM, Tulio Magno Quites Machado Filho wrote:
> Florian Weimer <fweimer@redhat.com> writes:
>
> > I think this run-time check is unnecessary on POWER6 and later:
> >
> > 	  double d = (env); \
> > 	  if(GLRO(dl_hwcap) & PPC_FEATURE_HAS_DFP) \
> > 	    asm volatile (".machine push; " \
> > 			  ".machine \"power6\"; " \
> > 			  "mtfsf 0xff,%0,1,0; " \
> > 			  ".machine pop" : : "f" (d)); \
> > 	  else \
> > 	    asm volatile ("mtfsf 0xff,%0" : : "f" (d)); \
>
> I think it's still necessary when building glibc for an older processor,
> e.g. POWER4, and run it on processors that support DFP, e.g. POWER6.

Right.

> > In particularly, we do not need it on ppc64le.  Correct?
>
> Agreed, unless the kernel has the right to disable DFP support.

Ugh, I had not thought of that.  Isn't DFP support part of the userspace 
ABI?

> > It's one of the GLIBC_PRIVATE references in libm.so.6, which is why I'm
> > asking.
>
> Would the following code solve this problem?
>
> #include <tls.h>
> /* Equivalent to fesetenv, but takes a fenv_t instead of a pointer.  */
> #define fesetenv_register(env) \
> 	do { \
> 	  double d = (env); \
> 	  if(THREAD_GET_HWCAP() & PPC_FEATURE_HAS_DFP) \
> 	    asm volatile (".machine push; " \
> 			  ".machine \"power6\"; " \
> 			  "mtfsf 0xff,%0,1,0; " \
> 			  ".machine pop" : : "f" (d)); \
> 	  else \
> 	    asm volatile ("mtfsf 0xff,%0" : : "f" (d)); \
> 	} while(0)

It avoids the GLIBC_PRIVATE reference, but it does not avoid the 
dependency on library internals.  So in a certain sense, it is way worse 
than what we have today (in particular if you are interested in pre-2.23 
glibc versions still).

I would have suggested something like

#define DL_HWCAP_HAS_DFP (GLRO(dl_hwcap) & PPC_FEATURE_HAS_DFP)

in the general powerpc sysdeps file, and

#define DL_HWCAP_HAS_DFP 1

for power6 (which I expect will propagate upwards).  I expect that this 
will result in somewhat nicer machine code, too.

Thanks,
Florian