[2/2] PR libgcc/59714 complex division is surprising on aarch64

Message ID 1507785715-11384-1-git-send-email-vladimir.mezentsev@oracle.com
State New
Headers show
Series
  • Untitled series #7743
Related show

Commit Message

Vladimir Mezentsev Oct. 12, 2017, 5:21 a.m.
From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>

FMA (floating-point multiply-add) instructions are supported on aarch64.
These instructions can produce different result if two operations executed separately.
-ffp-contract=off doesn't allow the FMA instructions.

Tested on aarch64-linux-gnu.
No regression. Two failed tests now passed.

ChangeLog:
2017-10-11  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>

PR libgcc/59714
* libgcc/config/aarch64/t-aarch64 (HOST_LIBGCC2_CFLAGS): Add -ffp-contract=off
---
 libgcc/config/aarch64/t-aarch64 | 1 +
 1 file changed, 1 insertion(+)

Comments

Richard Earnshaw Oct. 12, 2017, 10:40 a.m. | #1
On 12/10/17 06:21, vladimir.mezentsev@oracle.com wrote:
> From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
> 
> FMA (floating-point multiply-add) instructions are supported on aarch64.
> These instructions can produce different result if two operations executed separately.
> -ffp-contract=off doesn't allow the FMA instructions.
> 
> Tested on aarch64-linux-gnu.
> No regression. Two failed tests now passed.
> 
> ChangeLog:
> 2017-10-11  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
> 
> PR libgcc/59714
> * libgcc/config/aarch64/t-aarch64 (HOST_LIBGCC2_CFLAGS): Add -ffp-contract=off
> ---
>  libgcc/config/aarch64/t-aarch64 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libgcc/config/aarch64/t-aarch64 b/libgcc/config/aarch64/t-aarch64
> index 3af933c..e33bef0 100644
> --- a/libgcc/config/aarch64/t-aarch64
> +++ b/libgcc/config/aarch64/t-aarch64
> @@ -18,4 +18,5 @@
>  # along with GCC; see the file COPYING3.  If not see
>  # <http://www.gnu.org/licenses/>.
>  
> +HOST_LIBGCC2_CFLAGS += -ffp-contract=off
>  LIB2ADD += $(srcdir)/config/aarch64/sync-cache.c
> 

Why would we want to do this on AArch64 only?  If it's right for us,
then surely it would be right for everyone and the option should be
applied at the top level.

Hint: I'm not convinced on the evidence here that it is right across the
whole of libgcc.  Before moving forward on this particular PR I think we
need to understand what behaviour we do want out of the compiler.

R.
Vladimir Mezentsev Oct. 13, 2017, 5:28 p.m. | #2
On 10/12/2017 03:40 AM, Richard Earnshaw wrote:
> On 12/10/17 06:21, vladimir.mezentsev@oracle.com wrote:
>> From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
>>
>> FMA (floating-point multiply-add) instructions are supported on aarch64.
>> These instructions can produce different result if two operations executed separately.
>> -ffp-contract=off doesn't allow the FMA instructions.
>>
>> Tested on aarch64-linux-gnu.
>> No regression. Two failed tests now passed.
>>
>> ChangeLog:
>> 2017-10-11  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
>>
>> PR libgcc/59714
>> * libgcc/config/aarch64/t-aarch64 (HOST_LIBGCC2_CFLAGS): Add -ffp-contract=off
>> ---
>>  libgcc/config/aarch64/t-aarch64 | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libgcc/config/aarch64/t-aarch64 b/libgcc/config/aarch64/t-aarch64
>> index 3af933c..e33bef0 100644
>> --- a/libgcc/config/aarch64/t-aarch64
>> +++ b/libgcc/config/aarch64/t-aarch64
>> @@ -18,4 +18,5 @@
>>  # along with GCC; see the file COPYING3.  If not see
>>  # <http://www.gnu.org/licenses/>.
>>  
>> +HOST_LIBGCC2_CFLAGS += -ffp-contract=off
>>  LIB2ADD += $(srcdir)/config/aarch64/sync-cache.c
>>
> Why would we want to do this on AArch64 only?  If it's right for us,
> then surely it would be right for everyone and the option should be
> applied at the top level.

  It is a machine dependent option.
We don't need this option, for example, on sparc or intel machines.

-Vladimir
>
> Hint: I'm not convinced on the evidence here that it is right across the
> whole of libgcc.  Before moving forward on this particular PR I think we
> need to understand what behaviour we do want out of the compiler.
>
> R.
Richard Earnshaw Oct. 13, 2017, 9:25 p.m. | #3
On 13/10/17 18:28, vladimir.mezentsev@oracle.com wrote:
> On 10/12/2017 03:40 AM, Richard Earnshaw wrote:
>> On 12/10/17 06:21, vladimir.mezentsev@oracle.com wrote:
>>> From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
>>>
>>> FMA (floating-point multiply-add) instructions are supported on aarch64.
>>> These instructions can produce different result if two operations executed separately.
>>> -ffp-contract=off doesn't allow the FMA instructions.
>>>
>>> Tested on aarch64-linux-gnu.
>>> No regression. Two failed tests now passed.
>>>
>>> ChangeLog:
>>> 2017-10-11  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
>>>
>>> PR libgcc/59714
>>> * libgcc/config/aarch64/t-aarch64 (HOST_LIBGCC2_CFLAGS): Add -ffp-contract=off
>>> ---
>>>  libgcc/config/aarch64/t-aarch64 | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libgcc/config/aarch64/t-aarch64 b/libgcc/config/aarch64/t-aarch64
>>> index 3af933c..e33bef0 100644
>>> --- a/libgcc/config/aarch64/t-aarch64
>>> +++ b/libgcc/config/aarch64/t-aarch64
>>> @@ -18,4 +18,5 @@
>>>  # along with GCC; see the file COPYING3.  If not see
>>>  # <http://www.gnu.org/licenses/>.
>>>  
>>> +HOST_LIBGCC2_CFLAGS += -ffp-contract=off
>>>  LIB2ADD += $(srcdir)/config/aarch64/sync-cache.c
>>>
>> Why would we want to do this on AArch64 only?  If it's right for us,
>> then surely it would be right for everyone and the option should be
>> applied at the top level.
> 
>   It is a machine dependent option.
> We don't need this option, for example, on sparc or intel machines.
> 

No, it's a target-independent option (it's in the -f... option space).
It might be a no-op on some machines, but either it should be on
globally, or it should be off globally.  If it's a no-op then it won't
matter whether it's on or off.

R.

> -Vladimir
>>
>> Hint: I'm not convinced on the evidence here that it is right across the
>> whole of libgcc.  Before moving forward on this particular PR I think we
>> need to understand what behaviour we do want out of the compiler.
>>
>> R.
>
Ramana Radhakrishnan Oct. 13, 2017, 9:37 p.m. | #4
On Fri, Oct 13, 2017 at 10:25 PM, Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
> On 13/10/17 18:28, vladimir.mezentsev@oracle.com wrote:
>> On 10/12/2017 03:40 AM, Richard Earnshaw wrote:
>>> On 12/10/17 06:21, vladimir.mezentsev@oracle.com wrote:
>>>> From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
>>>>
>>>> FMA (floating-point multiply-add) instructions are supported on aarch64.
>>>> These instructions can produce different result if two operations executed separately.
>>>> -ffp-contract=off doesn't allow the FMA instructions.
>>>>
>>>> Tested on aarch64-linux-gnu.
>>>> No regression. Two failed tests now passed.
>>>>
>>>> ChangeLog:
>>>> 2017-10-11  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
>>>>
>>>> PR libgcc/59714
>>>> * libgcc/config/aarch64/t-aarch64 (HOST_LIBGCC2_CFLAGS): Add -ffp-contract=off
>>>> ---
>>>>  libgcc/config/aarch64/t-aarch64 | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/libgcc/config/aarch64/t-aarch64 b/libgcc/config/aarch64/t-aarch64
>>>> index 3af933c..e33bef0 100644
>>>> --- a/libgcc/config/aarch64/t-aarch64
>>>> +++ b/libgcc/config/aarch64/t-aarch64
>>>> @@ -18,4 +18,5 @@
>>>>  # along with GCC; see the file COPYING3.  If not see
>>>>  # <http://www.gnu.org/licenses/>.
>>>>
>>>> +HOST_LIBGCC2_CFLAGS += -ffp-contract=off
>>>>  LIB2ADD += $(srcdir)/config/aarch64/sync-cache.c
>>>>
>>> Why would we want to do this on AArch64 only?  If it's right for us,
>>> then surely it would be right for everyone and the option should be
>>> applied at the top level.
>>
>>   It is a machine dependent option.
>> We don't need this option, for example, on sparc or intel machines.
>>
>
> No, it's a target-independent option (it's in the -f... option space).
> It might be a no-op on some machines, but either it should be on
> globally, or it should be off globally.  If it's a no-op then it won't
> matter whether it's on or off.

Indeed and we do have this on AArch32 with vfpv4 so that's 2
architectures already affected. It maybe that the default builds on
x86 don't tickle fma but we may see the same behavior elsewhere
depending on (fp) architecture defaults.

I haven't tried this testcase yet on AArch32 but I suspect we're
likely to hit the same issue there too ...

regards
Ramana
>
> R.
>
>> -Vladimir
>>>
>>> Hint: I'm not convinced on the evidence here that it is right across the
>>> whole of libgcc.  Before moving forward on this particular PR I think we
>>> need to understand what behaviour we do want out of the compiler.
>>>
>>> R.
>>
>

Patch

diff --git a/libgcc/config/aarch64/t-aarch64 b/libgcc/config/aarch64/t-aarch64
index 3af933c..e33bef0 100644
--- a/libgcc/config/aarch64/t-aarch64
+++ b/libgcc/config/aarch64/t-aarch64
@@ -18,4 +18,5 @@ 
 # along with GCC; see the file COPYING3.  If not see
 # <http://www.gnu.org/licenses/>.
 
+HOST_LIBGCC2_CFLAGS += -ffp-contract=off
 LIB2ADD += $(srcdir)/config/aarch64/sync-cache.c