Subject: Re:  New: use of FPU_SETCW or FPU_GETCW causes
 illegal instruction on armel

On Thu, 23 Apr 2009, aurelien at aurel32 dot net wrote:

> /usr/include/fpu_control.h on ARM EABI defines FPU_[SG]ETCW as VFP
> coprocessor instructions, whereas armel is soft-float.
> 
> #define _FPU_GETCW(cw) \
>   __asm__ __volatile__ ("mrc p10, 7, %0, cr1, cr0, 0" : "=r" (cw))
> /* This is fmxr fpscr, %0.  */
> #define _FPU_SETCW(cw) \
>   __asm__ __volatile__ ("mcr p10, 7, %0, cr1, cr0, 0" : : "r" (cw))
> 
> This causes an illegal instruction trap on hardware that does not have a VFP 
> unit.

I'm not convinced this is a bug; these macros are inherently nonportable 
and should only be used by code that knows it is being built for a 
particular FPU and knows about how to use it.  Normal code should use 
<fenv.h> instead.  That said, making the contents compile-time conditional 
like the MIPS version would be reasonable; I don't think it can sensibly 
be made run-time conditional.

Created attachment 3907 [details]
Patch

> That said, making the contents compile-time conditional 
> like the MIPS version would be reasonable; I don't think it can sensibly 
> be made run-time conditional.

I agree. The patch I just attached, inspired by the mips version should to 
the trick.

Subject: Re:  use of FPU_SETCW or FPU_GETCW causes illegal
 instruction on armel

On Fri, 24 Apr 2009, aurelien at aurel32 dot net wrote:

> ------- Additional Comments From aurelien at aurel32 dot net  2009-04-24 09:44 -------
> > That said, making the contents compile-time conditional 
> > like the MIPS version would be reasonable; I don't think it can sensibly 
> > be made run-time conditional.
> 
> I agree. The patch I just attached, inspired by the mips version should to 
> the trick.

I think you need to condition on _LIBC not being defined as well, or 
you'll break the runtime VFP detection in the fenv.h functions (which use 
fpu_control.h).  I'd be surprised if glibc even built for soft-float with 
that patch applied, since fesetenv.c uses _FPU_IEEE which the patch would 
make undefined in the soft-float case.

I admit I hadn't made a build test, it takes long on ARM. I have just
started one, I'll come with the final patch when it is built.

Created attachment 3908 [details]
v2 of the patch

I have just added a new version of the patch, with the change you suggested. 
This version has been compile tested.

Thanks, I have committed this patch.  Note that the path in your
ChangeLog entry was wrong.