AOSC-Archive / autobuild3

AOSC OS package maintenance toolkit (version 3)
https://aosc.io
GNU General Public License v2.0
24 stars 17 forks source link

arch/armel: possible misuse of -mfpu #77

Closed MingcongBai closed 8 years ago

MingcongBai commented 9 years ago

I was notified by @Icenowy on this. He pointed that the paragraph discussing -mfpu, quotes:

-mfpu=name
    This specifies what floating-point hardware (or hardware emulation) is available on the target. Permissible names are: ‘vfp’, ‘vfpv3’, ‘vfpv3-fp16’, ‘vfpv3-d16’, ‘vfpv3-d16-fp16’, ‘vfpv3xd’, ‘vfpv3xd-fp16’, ‘neon’, ‘neon-fp16’, ‘vfpv4’, ‘vfpv4-d16’, ‘fpv4-sp-d16’, ‘neon-vfpv4’, ‘fpv5-d16’, ‘fpv5-sp-d16’, ‘fp-armv8’, ‘neon-fp-armv8’, and ‘crypto-neon-fp-armv8’.

    If -msoft-float is specified this specifies the format of floating-point values.

    If the selected floating-point hardware includes the NEON extension (e.g. -mfpu=‘neon’), note that floating-point operations are not generated by GCC's auto-vectorization pass unless -funsafe-math-optimizations is also specified. This is because NEON hardware does not fully implement the IEEE 754 standard for floating-point arithmetic (in particular denormal values are treated as zero), so the use of NEON instructions may lead to a loss of precision.

    You can also set the fpu name at function level by using the target("fpu=") function attributes (see ARM Function Attributes) or pragmas (see Function Specific Option Pragmas). 

Suggesting an extra flag, -funsafe-math-optimizations should be used, or -mfpu=neon is basically invalidated by the compiler. This option should be addressed as soon as possible.

MingcongBai commented 9 years ago

https://github.com/AOSC-Dev/autobuild3/commit/4cffc0098dfc8873abecc7fee3c5ad82587f7bee

An uncertain fix. Please read msg for more information.

Artoria2e5 commented 8 years ago

This is not a misuse. Actually, this tells us it's safe to do neon…

Artoria2e5 commented 8 years ago

Oh, whoa, 4cffc00 is a terrible mistake. Nobody wants to use unsafe optimizations everywhere.

Icenowy commented 8 years ago

It has already been fixed.