[Bug target/63408] GCC emits incorrect FMA instruction on Cortex-M4 target

[Bug target/63408] GCC emits incorrect FMA instruction on Cortex-M4 target

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Are you sure 1120403456 does not encode -100.0 ?

[Bug target/63408] GCC emits incorrect FMA instruction on Cortex-M4 target

[itay at phobotic dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #2 from Itay Perl <itay at phobotic dot com> ---
Yes, I am certain.

1. Replacing the minus sign with a plus sign results in the same code
(including the constant)

2.  

    >>> struct.unpack('I', struct.pack('f', 100))[0]
    1120403456

[Bug target/63408] [4.8 regression] GCC emits incorrect FMA instruction on Cortex-M4 target

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-09-30
                 CC|                            |ramana at gcc dot gnu.org
            Summary|GCC emits incorrect FMA     |[4.8 regression] GCC emits
                   |instruction on Cortex-M4    |incorrect FMA instruction
                   |target                      |on Cortex-M4 target
     Ever confirmed|0                           |1

--- Comment #3 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
Confirmed.  The compiler thinks (incorrectly) that the conversion of b to float
can be merged with the negation by using the fixed-to-float conversion
operation in the back-end.  That is, it thinks

(insn 10 9 11 2 (set (reg:SF 121 [ D.4093 ])
        (mult:SF (float:SF (reg:SI 0 r0 [ b ]))
            (const_double:SF -4.656612873077392578125e-10 [-0x0.8p-30])))

is a legitimate instruction.

vfp3_const_double_for_fract_bits() needs to be made aware that negative values
cannot be used in this context.

This is almost certainly a regression since the pattern was added.

[Bug target/63408] [4.8 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.4

[Bug target/63408] [4.8/4.9/5 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P2
            Summary|[4.8 regression] GCC emits  |[4.8/4.9/5 regression] GCC
                   |incorrect fixed->fp         |emits incorrect fixed->fp
                   |conversion instruction on   |conversion instruction on
                   |Cortex-M4 target            |Cortex-M4 target

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
But supposedly not only on the 4.8 branch.

[Bug target/63408] [4.8/4.9/5 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug target/63408] [4.8/4.9/5 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ramana at gcc dot gnu.org

--- Comment #6 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
Mine.

[Bug target/63408] [4.8/4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[itay at phobotic dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #7 from Itay Perl <itay at phobotic dot com> ---
Any news? This bug still prevents us from compiling our code with
optimizations.

Thanks.

[Bug target/63408] [4.8/4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.5                       |4.9.3

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
The gcc-4_8-branch is being closed, re-targeting regressions to 4.9.3.

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #9 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
(In reply to Itay Perl from comment #7)
> Any news? This bug still prevents us from compiling our code with
> optimizations.
> 
> Thanks.

Sorry to have dropped the ball on this one - does replacing the vfma
instruction with vfms work for you in the example i.e. can you provide me with
a couple of input values for a and b that break for me to verify this is fine
across a range of implementations ?

Ramana

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[itay at phobotic dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #10 from Itay Perl <itay at phobotic dot com> ---
A few examples:

a=100, b=0x7fffffff          -O0: 0.0   -O3: 200.0
a=0,   b=0x7fffffff/100.0f   -O0: -1.0  -O3: 1.0
a=0,   b=-0x7fffffff/100.0f  -O0: 1.0   -O3: -1.0

Replacing vfma with vfms sounds correct to me, and returns the correct result
for the above inputs.

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #11 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
(In reply to Itay Perl from comment #10)
> A few examples:
> 
> a=100, b=0x7fffffff          -O0: 0.0   -O3: 200.0
> a=0,   b=0x7fffffff/100.0f   -O0: -1.0  -O3: 1.0
> a=0,   b=-0x7fffffff/100.0f  -O0: 1.0   -O3: -1.0
> 
> Replacing vfma with vfms sounds correct to me, and returns the correct
> result for the above inputs.

Thanks - I'm testing a fix.

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #12 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
Created attachment 35833
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35833&action=edit
current patch.

Patch I'm testing - this is based on a 4.9 tree where I could easily replicate
the problem but applies cleanly to trunk.

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #13 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
Author: ramana
Date: Wed Jun 24 08:28:08 2015
New Revision: 224879

URL: https://gcc.gnu.org/viewcvs?rev=224879&root=gcc&view=rev
Log:
Fix PR target/63408

The attached patch fixes PR target/63408 and adds a regression test
for the same. The problem is essentially that
vfp3_const_double_for_fract_bits() needs to be aware that negative
values cannot be used in this context.

Tested with a bootstrap and regression test run on armhf. Applied.




2015-06-24  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        PR target/63408
        * config/arm/arm.c (vfp3_const_double_for_fract_bits): Disable
        for negative numbers.

2015-06-24  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        PR target/63408
        * gcc.target/arm/pr63408.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/arm/pr63408.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #14 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
Author: ramana
Date: Thu Jun 25 08:36:03 2015
New Revision: 224933

URL: https://gcc.gnu.org/viewcvs?rev=224933&root=gcc&view=rev
Log:
Fix PR target/63408

Backport fix for PR target/63408 from mainline

The attached patch fixes PR target/63408 and adds a regression test
for the same. The problem is essentially that
vfp3_const_double_for_fract_bits() needs to be aware that negative
values cannot be used in this context.

Tested with a bootstrap and regression test run on armhf. Applied.

2015-06-25  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        Backport from mainline.
        2015-06-24  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>
        PR target/63408
        * config/arm/arm.c (vfp3_const_double_for_fract_bits): Disable
        for negative numbers.

2015-06-25  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        Backport from mainline.
        2015-06-24  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>
        PR target/63408
        * gcc.target/arm/pr63408.c: New test.


Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/pr63408.c
      - copied unchanged from r224879,
trunk/gcc/testsuite/gcc.target/arm/pr63408.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/arm/arm.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.9.3 has been released.

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.3                       |4.9.4

[Bug target/63408] [4.9/5/6 regression] GCC emits incorrect fixed->fp conversion instruction on Cortex-M4 target

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63408

--- Comment #16 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
Author: ramana
Date: Wed Jul  1 08:01:57 2015
New Revision: 225226

URL: https://gcc.gnu.org/viewcvs?rev=225226&root=gcc&view=rev
Log:
Fix PR target/63408 on the 4.9 branch.

2015-07-01  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        Backport from mainline
        2015-06-24  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        PR target/63408
        * config/arm/arm.c (vfp3_const_double_for_fract_bits): Disable
        for negative numbers.

2015-07-01  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        Backport from mainline
        2015-06-24  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        PR target/63408
        * gcc.target/arm/pr63408.c: New test.



Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/arm/pr63408.c
      - copied unchanged from r224879,
trunk/gcc/testsuite/gcc.target/arm/pr63408.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/arm/arm.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog