[PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

Hi All,

The inlining of lrint isn't valid in all cases on ILP32 when
-fno-math-errno is used because an inexact exception is raised in
certain circumstances.

For ILP32 I now restrict the inlining only when -fno-trapping-math
is also specified.

This fixed PR/81800.

Regtested on aarch64-none-linux-gnu and no regressions.

Ok for trunk?

Thanks,
Tamar

gcc/
2017-08-11  Tamar Christina  <tamar.christina@arm.com>

	PR target/81800
	* config/aarch64/aarch64.md (lrint<GPF:mode><GPI:mode>2): Add flag_trapping_math.

gcc/testsuite/
2017-08-11  Tamar Christina  <tamar.christina@arm.com>

	* gcc.target/aarch64/inline-lrint_2.c (dg-options): Add -fno-trapping-math.

-- 

[-- Attachment #2: 8025-diff.patch --]
[-- Type: text/x-diff, Size: 1129 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index b1ef0252c4b0b93d368abbd9bb88cb740115a829..6617ea0b95f71fd91534f63de9ddfd2f400bb787 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5122,7 +5122,7 @@
 (define_expand "lrint<GPF:mode><GPI:mode>2"
   [(match_operand:GPI 0 "register_operand")
    (match_operand:GPF 1 "register_operand")]
-  "TARGET_FLOAT"
+  "TARGET_FLOAT && (!TARGET_ILP32 || !flag_trapping_math)"
 {
   rtx cvt = gen_reg_rtx (<GPF:MODE>mode);
   emit_insn (gen_rint<GPF:mode>2 (cvt, operands[1]));
diff --git a/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c b/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c
index 6080e186d8f0c6f5ede81c6438e059e8b976378f..bd0c73c8d34a2cb52d0a453a175bedd59bba5457 100644
--- a/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target ilp32 } */
-/* { dg-options "-O3 -fno-math-errno" } */
+/* { dg-options "-O3 -fno-math-errno -fno-trapping-math" } */
 
 #include "lrint-matherr.h"
 

Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[Andrew Pinski]

On Fri, Aug 11, 2017 at 2:58 AM, Tamar Christina
<tamar.christina@arm.com> wrote:
> Hi All,
>
> The inlining of lrint isn't valid in all cases on ILP32 when
> -fno-math-errno is used because an inexact exception is raised in
> certain circumstances.
>
> For ILP32 I now restrict the inlining only when -fno-trapping-math
> is also specified.
>
> This fixed PR/81800.
>
> Regtested on aarch64-none-linux-gnu and no regressions.
>
> Ok for trunk?

I can't approve this patch but it looks good except for your changelog
does not match what the code is doing.

Maybe:
Add check on !ilp32 or !flag_trapping_math.

Thanks,
Andrew

>
> Thanks,
> Tamar
>
> gcc/
> 2017-08-11  Tamar Christina  <tamar.christina@arm.com>
>
>         PR target/81800
>         * config/aarch64/aarch64.md (lrint<GPF:mode><GPI:mode>2): Add flag_trapping_math.
>
> gcc/testsuite/
> 2017-08-11  Tamar Christina  <tamar.christina@arm.com>
>
>         * gcc.target/aarch64/inline-lrint_2.c (dg-options): Add -fno-trapping-math.
>
> --

Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[Tamar Christina]

Hi Andrew,

Yes I noticed the changelog after I sent it.

But I have a new version of the patch on internal review
Which is why I didn't ping this one. I'll send it out after the Cauldron.

Thanks,
Tamar
________________________________________
From: Andrew Pinski <pinskia@gmail.com>
Sent: Saturday, September 9, 2017 7:57:00 AM
To: Tamar Christina
Cc: GCC Patches; nd; James Greenhalgh; Richard Earnshaw; Marcus Shawcroft
Subject: Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

On Fri, Aug 11, 2017 at 2:58 AM, Tamar Christina
<tamar.christina@arm.com> wrote:
> Hi All,
>
> The inlining of lrint isn't valid in all cases on ILP32 when
> -fno-math-errno is used because an inexact exception is raised in
> certain circumstances.
>
> For ILP32 I now restrict the inlining only when -fno-trapping-math
> is also specified.
>
> This fixed PR/81800.
>
> Regtested on aarch64-none-linux-gnu and no regressions.
>
> Ok for trunk?

I can't approve this patch but it looks good except for your changelog
does not match what the code is doing.

Maybe:
Add check on !ilp32 or !flag_trapping_math.

Thanks,
Andrew

>
> Thanks,
> Tamar
>
> gcc/
> 2017-08-11  Tamar Christina  <tamar.christina@arm.com>
>
>         PR target/81800
>         * config/aarch64/aarch64.md (lrint<GPF:mode><GPI:mode>2): Add flag_trapping_math.
>
> gcc/testsuite/
> 2017-08-11  Tamar Christina  <tamar.christina@arm.com>
>
>         * gcc.target/aarch64/inline-lrint_2.c (dg-options): Add -fno-trapping-math.
>
> --

Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[James Greenhalgh]

On Wed, Sep 13, 2017 at 04:00:24PM +0100, Tamar Christina wrote:
> Hi All,
> 
> The inlining of lrint isn't valid in all cases on ILP32 when
> -fno-math-errno is used because an inexact exception is raised in
> certain circumstances.
> 
> Instead the restriction is placed such that the integer mode has to
> be larger or equal to the float mode in addition to either inexacts being
> allowed or not caring about trapping math.
> 
> This prevents the overflow, and the inexact errors that may arise.
> 
> Unfortunately I can't create a test for this as there is a bug where
> the pattern is always passed DI as the smallest mode,
> and later takes a sub-reg of it to SI. This would prevent an overflow
> where one was expected.
> 
> This fixed PR/81800.
> 
> Regtested on aarch64-none-linux-gnu and no regressions.
> 
> Ok for trunk?

OK.

Reviewed By James Greenhalgh  <james.greenhalgh@arm.com>

Thanks,
James

> 
> Thanks,
> 
> 	PR target/81800
> 	* config/aarch64/aarch64.md (lrint<GPF:mode><GPI:mode>2): Add flag_trapping_math
> 	and flag_fp_int_builtin_inexact.
> 
> gcc/testsuite/
> 2017-09-13  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* gcc.target/aarch64/inline-lrint_2.c (dg-options): Add -fno-trapping-math.
> 
> -- 

Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[Tamar Christina]

Ping.

Any objections to the patch?
________________________________________
From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> on behalf of Tamar Christina <Tamar.Christina@arm.com>
Sent: Monday, October 16, 2017 9:54:23 AM
To: gcc-patches@gcc.gnu.org
Cc: nd; James Greenhalgh; Richard Earnshaw; Marcus Shawcroft; pinskia@gmail.com
Subject: Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

Ping?
________________________________________
From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> on behalf of Tamar Christina <tamar.christina@arm.com>
Sent: Wednesday, September 13, 2017 4:00:24 PM
To: gcc-patches@gcc.gnu.org
Cc: nd; James Greenhalgh; Richard Earnshaw; Marcus Shawcroft; pinskia@gmail.com
Subject: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

Hi All,

The inlining of lrint isn't valid in all cases on ILP32 when
-fno-math-errno is used because an inexact exception is raised in
certain circumstances.

Instead the restriction is placed such that the integer mode has to
be larger or equal to the float mode in addition to either inexacts being
allowed or not caring about trapping math.

This prevents the overflow, and the inexact errors that may arise.

Unfortunately I can't create a test for this as there is a bug where
the pattern is always passed DI as the smallest mode,
and later takes a sub-reg of it to SI. This would prevent an overflow
where one was expected.

This fixed PR/81800.

Regtested on aarch64-none-linux-gnu and no regressions.

Ok for trunk?

Thanks,
Tamar

gcc/
2017-09-13  Tamar Christina  <tamar.christina@arm.com>

        PR target/81800
        * config/aarch64/aarch64.md (lrint<GPF:mode><GPI:mode>2): Add flag_trapping_math
        and flag_fp_int_builtin_inexact.

gcc/testsuite/
2017-09-13  Tamar Christina  <tamar.christina@arm.com>

        * gcc.target/aarch64/inline-lrint_2.c (dg-options): Add -fno-trapping-math.

--

Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[Tamar Christina]

Ping?
________________________________________
From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> on behalf of Tamar Christina <tamar.christina@arm.com>
Sent: Wednesday, September 13, 2017 4:00:24 PM
To: gcc-patches@gcc.gnu.org
Cc: nd; James Greenhalgh; Richard Earnshaw; Marcus Shawcroft; pinskia@gmail.com
Subject: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

Hi All,

The inlining of lrint isn't valid in all cases on ILP32 when
-fno-math-errno is used because an inexact exception is raised in
certain circumstances.

Instead the restriction is placed such that the integer mode has to
be larger or equal to the float mode in addition to either inexacts being
allowed or not caring about trapping math.

This prevents the overflow, and the inexact errors that may arise.

Unfortunately I can't create a test for this as there is a bug where
the pattern is always passed DI as the smallest mode,
and later takes a sub-reg of it to SI. This would prevent an overflow
where one was expected.

This fixed PR/81800.

Regtested on aarch64-none-linux-gnu and no regressions.

Ok for trunk?

Thanks,
Tamar

gcc/
2017-09-13  Tamar Christina  <tamar.christina@arm.com>

        PR target/81800
        * config/aarch64/aarch64.md (lrint<GPF:mode><GPI:mode>2): Add flag_trapping_math
        and flag_fp_int_builtin_inexact.

gcc/testsuite/
2017-09-13  Tamar Christina  <tamar.christina@arm.com>

        * gcc.target/aarch64/inline-lrint_2.c (dg-options): Add -fno-trapping-math.

--

[PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]

Hi All,

The inlining of lrint isn't valid in all cases on ILP32 when
-fno-math-errno is used because an inexact exception is raised in
certain circumstances.

Instead the restriction is placed such that the integer mode has to
be larger or equal to the float mode in addition to either inexacts being
allowed or not caring about trapping math.

This prevents the overflow, and the inexact errors that may arise.

Unfortunately I can't create a test for this as there is a bug where
the pattern is always passed DI as the smallest mode,
and later takes a sub-reg of it to SI. This would prevent an overflow
where one was expected.

This fixed PR/81800.

Regtested on aarch64-none-linux-gnu and no regressions.

Ok for trunk?

Thanks,
Tamar

gcc/
2017-09-13  Tamar Christina  <tamar.christina@arm.com>

	PR target/81800
	* config/aarch64/aarch64.md (lrint<GPF:mode><GPI:mode>2): Add flag_trapping_math
	and flag_fp_int_builtin_inexact.

gcc/testsuite/
2017-09-13  Tamar Christina  <tamar.christina@arm.com>

	* gcc.target/aarch64/inline-lrint_2.c (dg-options): Add -fno-trapping-math.

-- 

[-- Attachment #2: 8025-diff.patch --]
[-- Type: text/x-diff, Size: 1221 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 64b60a903ed7c0090298989333894442a277ccd4..1efc56dd40de8ab1f6ee1b743b804f0c3ed887b2 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5122,7 +5122,9 @@
 (define_expand "lrint<GPF:mode><GPI:mode>2"
   [(match_operand:GPI 0 "register_operand")
    (match_operand:GPF 1 "register_operand")]
-  "TARGET_FLOAT"
+  "TARGET_FLOAT
+   && ((GET_MODE_SIZE (<GPF:MODE>mode) <= GET_MODE_SIZE (<GPI:MODE>mode))
+   || !flag_trapping_math || flag_fp_int_builtin_inexact)"
 {
   rtx cvt = gen_reg_rtx (<GPF:MODE>mode);
   emit_insn (gen_rint<GPF:mode>2 (cvt, operands[1]));
diff --git a/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c b/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c
index 6080e186d8f0c6f5ede81c6438e059e8b976378f..bd0c73c8d34a2cb52d0a453a175bedd59bba5457 100644
--- a/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/inline-lrint_2.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target ilp32 } */
-/* { dg-options "-O3 -fno-math-errno" } */
+/* { dg-options "-O3 -fno-math-errno -fno-trapping-math" } */
 
 #include "lrint-matherr.h"