llvm dev - Mar 2017 - clang 4.0.0: Invalid code for builtin floating point function with -mfloat-abi=hard -ffast-math (ARM)

Hello,

clang/llvm 4.0.0 generates invalid calls for builtin functions with 
-mfloat-abi=hard -ffast-math.

Small example fail.c:

    // clang -O2 -target armv7a-none-none-eabi -mfloat-abi=hard 
-ffast-math -S fail.c -o -
    extern float sinf (float x);
    float sin1 (float x) {return (sinf (x));}

generates code to pass the parameter in r0 and expect the result in r0.
The same code without -ffast-math compiles correctly. It also works with 
-fno-builtin-sinf.

(-O2 is not required to trigger the bug, but makes the resulting code 
easier to read)

It seems -ffast-math changes the internal declaration of builtin 
functions, as if declared
with -mfloat-abi=soft.

Works just fine with or without -ffast-math in previous releases.

cheers,

Peter

On 3/21/2017 11:51 AM, Peter Jakubek via llvm-dev wrote:
> Hello,
>
> clang/llvm 4.0.0 generates invalid calls for builtin functions with 
> -mfloat-abi=hard -ffast-math.
>
> Small example fail.c:
>
>    // clang -O2 -target armv7a-none-none-eabi -mfloat-abi=hard 
> -ffast-math -S fail.c -o -
>    extern float sinf (float x);
>    float sin1 (float x) {return (sinf (x));}
>
> generates code to pass the parameter in r0 and expect the result in r0.
> The same code without -ffast-math compiles correctly. It also works 
> with -fno-builtin-sinf.
>
> (-O2 is not required to trigger the bug, but makes the resulting code 
> easier to read)
>
> It seems -ffast-math changes the internal declaration of builtin 
> functions,
It would be more accurate to say -ffast-math makes the compiler treat 
sin() as a builtin, and therefore recreate the declaration from scratch.
> as if declared
> with -mfloat-abi=soft.
That is probably unintentional.  Granted, using -mfloat-abi like this is 
kind of weird, but I think clang's behavior is supposed to be 
gcc-compatible.  See https://reviews.llvm.org/rL291909 and 
https://bugs.llvm.org/show_bug.cgi?id=30543 for the most recent work in 
this area.

CC'ing the author of that commit.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

Am 22.03.2017 um 02:38 schrieb Friedman, Eli:
>
> That is probably unintentional.  Granted, using -mfloat-abi like this is
> kind of weird, but I think clang's behavior is supposed to be
> gcc-compatible.  See https://reviews.llvm.org/rL291909 and
> https://bugs.llvm.org/show_bug.cgi?id=30543 for the most recent work in
> this area.
why is using -mfloat-abi weird?

And I do not agree that whether functions are treated as builtin should 
depend on -ffast-math.
(it does not with gcc)

Obviously rL291909 is related. It explicitly states -mfloat-abi should 
be ignored for builtins.
(It does not actually ignore -mfloat-abi unless -ffast-math is specified)

I do not think this is how gcc actually behaves.
Gcc does not ignore -mfloat-abi. This really changes the calling 
conventions for builtin functions to VFP.

And with gcc -ffast-math does *not* change CC. I tested this with the 
latest binary I could find (gcc version 6.3.1 20170215):

https://developer.arm.com/-/media/Files/downloads/gnu-rm/6_1-2017q1/gcc-arm-none-eabi-6-2017-q1-update-win32.exe?product=GNU%20ARM%20Embedded%20Toolchain,32-
bit,,Windows,6-2017-q1-update

Here is a more elaborate example of the problem:

fail.c:

   extern float sinf (float x);
   float sin1 (float x) {return (sinf (x) + 1.0);}

arm-none-eabi-gcc.exe -O2 -mfloat-abi=hard -ffast-math -S fail.c -o -

   sin1:
     push    {r4, lr}
     bl      sinf
     vldr.32 s15, .L3
     pop     {r4, lr}
     vadd.f32        s0, s0, s15
     bx      lr

VFP CC (and optimal code).

Compiled with clang/llvm 4.0.0:
clang.exe -target armv7a-none-none-eabi -O2 -mfloat-abi=hard -ffast-math 
-S fail.c -o -

   sin1:
     push    {r11, lr}
     mov     r11, sp
     vmov    r0, s0
     bl      sinf
     vmov.f32        d16, #1.000000e+00
     vmov    d17, r0, r0
     vadd.f32        d0, d17, d16
     pop     {r11, pc}

Using non-VFP CC - incompatible with the gcc code.

Finally, using eabihf or -fno-fast-math corrects the problem:

clang.exe -target armv7a-none-none-eabihf -O2 -mfloat-abi=hard 
-ffast-math -S fail.c -o -

   sin1:
     push    {r11, lr}
     mov     r11, sp
     vpush   {d8}
     vmov.f32        d8, #1.000000e+00
     bl      sinf
     vadd.f32        d0, d0, d8
     vpop    {d8}
     pop     {r11, pc}

This is using VFP calling conventions as expected.

(I don't like llvm's handling of r11 and d8, though - the optimizer 
needs to be optimized...:-)

IMHO the current handling is incompatible with gcc. (and with previous 
releases of clang)

Cheers,

Peter

On 22 March 2017 at 01:38, Friedman, Eli <efriedma at codeaurora.org>
wrote:
>> Small example fail.c:
>>
>>    // clang -O2 -target armv7a-none-none-eabi -mfloat-abi=hard
-ffast-math
>> -S fail.c -o -
>>    extern float sinf (float x);
>>    float sin1 (float x) {return (sinf (x));}
I changed your example slightly to make sure we're passing the
arguments, otherwise 'sin1' just becomes 'b sinf', which is
"correct"
on both hard and soft float.

extern float sinf (float x);
float sin1 (float x, float y) {return (sinf (y)+x);}

>> generates code to pass the parameter in r0 and expect the result in r0.
>> The same code without -ffast-math compiles correctly. It also works
with
>> -fno-builtin-sinf.
Right, so this is the problem. You're declaring a function that is
declared in the C library, and the compiler will try and match it with
what it understands of the ABI (via builtins).

In this case, GCC and Clang "understand" different things about them,
and I'm not saying Clang is right. But if you want to use your own
sine functions, using -fno-builtin-sinf is the *right* way to go.

So, now that you have a work around that makes sense (and can progress
without us blocking you), let's talk about the problem at hand.

> It would be more accurate to say -ffast-math makes the compiler treat sin()
> as a builtin, and therefore recreate the declaration from scratch.
Precisely.

But the problem is not as simple as it seems. We had a conversation
about this during the US LLVM last year, and the conclusion is that
soft-float ABI functions should always have soft-float calling
conventions (GCC seems to agree). But sin/cos are not soft-fp
functions at all.

Looking at the patch, sin/cos wasn't wrongly bundled with the soft-fp
nodes, so it's possible that fast-math is combining nodes, thus
changing the behaviour of the selection dag wrt calling conventions.

Saleem,

Any light on why is sin/cos bundled with soft-fp handling?

cheers,
--renato