Fix PR 93044: extra cast is not removed

Message ID 20230724234851.1736326-1-apinski@marvell.com
State Accepted
Headers
Series Fix PR 93044: extra cast is not removed |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski July 24, 2023, 11:48 p.m. UTC
  In this case we are not removing convert to a bigger size
back to the same size (or smaller) if signedness does not
match.
For an example:
```
  signed char _1;
...
  _1 = *a_4(D);
  b_5 = (short unsigned int) _1;
  _2 = (unsigned char) b_5;
```
The inner cast is not needed and can be removed but was not.
The match pattern for removing the extra cast is overly
complex so decided to add a new case for rather than trying
to modify the current if statement here.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* match.pd (nested int casts): A truncation (to the same size or smaller)
	can always remove the inner cast.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/cast-1.c: New test.
	* gcc.dg/tree-ssa/cast-2.c: New test.
---
 gcc/match.pd                           | 10 ++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/cast-1.c | 12 ++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/cast-2.c | 12 ++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cast-1.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cast-2.c
  

Comments

Richard Biener July 25, 2023, 7:02 a.m. UTC | #1
On Tue, Jul 25, 2023 at 1:49 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In this case we are not removing convert to a bigger size
> back to the same size (or smaller) if signedness does not
> match.
> For an example:
> ```
>   signed char _1;
> ...
>   _1 = *a_4(D);
>   b_5 = (short unsigned int) _1;
>   _2 = (unsigned char) b_5;
> ```
> The inner cast is not needed and can be removed but was not.
> The match pattern for removing the extra cast is overly
> complex so decided to add a new case for rather than trying
> to modify the current if statement here.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
>         * match.pd (nested int casts): A truncation (to the same size or smaller)
>         can always remove the inner cast.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/cast-1.c: New test.
>         * gcc.dg/tree-ssa/cast-2.c: New test.
> ---
>  gcc/match.pd                           | 10 ++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/cast-1.c | 12 ++++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/cast-2.c | 12 ++++++++++++
>  3 files changed, 34 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cast-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cast-2.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index bfd15d6cd4a..bc8a3ab17eb 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4257,6 +4257,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>          && ! (final_ptr && inside_prec != inter_prec))
>       (ocvt @0))
>
> +   /* `(signed:M)(signed:N)(signed:O)`
> +      can be convert it to `(signed:M)a`

Better say

             (outer:M)(inter:N) a:O
            can be converted to (outer:M) a

since as you say below the signedness doesn't really matter and this
would then also mention 'a'

> +      if M <= O && N >= O. No matter what signedness of the cast,

of the casts

OK with those comment changes.

Thanks,
Richard.

> +      as the final is either a truncation from the original or just
> +      a sign change of the type. */
> +   (if (inside_int && inter_int && final_int
> +        && final_prec <= inside_prec
> +       && inter_prec >= inside_prec)
> +    (convert @0))
> +
>      /* A truncation to an unsigned type (a zero-extension) should be
>         canonicalized as bitwise and of a mask.  */
>      (if (GIMPLE /* PR70366: doing this in GENERIC breaks -Wconversion.  */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cast-1.c b/gcc/testsuite/gcc.dg/tree-ssa/cast-1.c
> new file mode 100644
> index 00000000000..0f33ab58b3e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/cast-1.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-optimized" } */
> +
> +
> +void f(signed char *a, unsigned char *c)
> +{
> +  unsigned short b = *a;
> +  *c = ((unsigned char)b);
> +}
> +
> +
> +/* { dg-final { scan-tree-dump-not "\\(short unsigned int\\)" "optimized"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cast-2.c b/gcc/testsuite/gcc.dg/tree-ssa/cast-2.c
> new file mode 100644
> index 00000000000..d665e924831
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/cast-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-optimized" } */
> +
> +
> +void f(signed short *a, unsigned char *c)
> +{
> +  unsigned long b = *a;
> +  *c = ((unsigned char)b);
> +}
> +
> +
> +/* { dg-final { scan-tree-dump-not "\\(long unsigned int\\)" "optimized"} } */
> --
> 2.31.1
>
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index bfd15d6cd4a..bc8a3ab17eb 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4257,6 +4257,16 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	 && ! (final_ptr && inside_prec != inter_prec))
      (ocvt @0))
 
+   /* `(signed:M)(signed:N)(signed:O)`
+      can be convert it to `(signed:M)a`
+      if M <= O && N >= O. No matter what signedness of the cast,
+      as the final is either a truncation from the original or just
+      a sign change of the type. */
+   (if (inside_int && inter_int && final_int
+        && final_prec <= inside_prec
+	&& inter_prec >= inside_prec)
+    (convert @0))
+
     /* A truncation to an unsigned type (a zero-extension) should be
        canonicalized as bitwise and of a mask.  */
     (if (GIMPLE /* PR70366: doing this in GENERIC breaks -Wconversion.  */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cast-1.c b/gcc/testsuite/gcc.dg/tree-ssa/cast-1.c
new file mode 100644
index 00000000000..0f33ab58b3e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/cast-1.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+
+void f(signed char *a, unsigned char *c)
+{
+  unsigned short b = *a;
+  *c = ((unsigned char)b);
+}
+
+
+/* { dg-final { scan-tree-dump-not "\\(short unsigned int\\)" "optimized"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cast-2.c b/gcc/testsuite/gcc.dg/tree-ssa/cast-2.c
new file mode 100644
index 00000000000..d665e924831
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/cast-2.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+
+void f(signed short *a, unsigned char *c)
+{
+  unsigned long b = *a;
+  *c = ((unsigned char)b);
+}
+
+
+/* { dg-final { scan-tree-dump-not "\\(long unsigned int\\)" "optimized"} } */