diff mbox series

match: Fix `(convert)abs((convert)a)` pattern for integer overflow [PR127689]

Message ID 20261003081313.1609669-1-andrew.pinski@oss.qualcomm.com
State New
Headers
Series match: Fix `(convert)abs((convert)a)` pattern for integer overflow [PR127689] |

Commit Message

Andrea Pinski Oct. 3, 2026, 8:13 a.m. UTC 
  There are 2 patterns that are similar in match.pd for changing into
a smaller abs. One handles `abs((ext)a)` and one for `(trunc)abs((ext)a)`.
The one with the truncate afterwards does not use absu for integer types
which can introduce undefined behavior in some cases.
So let's restrict the second one to non integral types as the first one
will handle those always; the check for casting from unsigned is another
difference but the casting from unsigned is handled by a different abs
pattern for non-negative values.

Bootstrapped and tested on x86_64-linux-gnu.

	PR tree-optimization/127689

gcc/ChangeLog:

	* match.pd (`(trunc)abs((ext)a)`): Move right
	after `abs((ext)a)` pattern.  Restrict to
	non integral types.

gcc/testsuite/ChangeLog:

	* gcc.dg/torture/pr127689-1.c: New test.

Signed-off-by: Andrea Pinski <andrew.pinski@oss.qualcomm.com>
---
 gcc/match.pd                              | 22 +++++++++---------
 gcc/testsuite/gcc.dg/torture/pr127689-1.c | 28 +++++++++++++++++++++++
 2 files changed, 39 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr127689-1.c

Comments

Richard Biener Oct. 3, 2026, 10:26 a.m. UTC | #1 
> Am 03.10.2026 um 10:15 schrieb Andrea Pinski <andrew.pinski@oss.qualcomm.com>:
> 
> There are 2 patterns that are similar in match.pd for changing into
> a smaller abs. One handles `abs((ext)a)` and one for `(trunc)abs((ext)a)`.
> The one with the truncate afterwards does not use absu for integer types
> which can introduce undefined behavior in some cases.
> So let's restrict the second one to non integral types as the first one
> will handle those always; the check for casting from unsigned is another
> difference but the casting from unsigned is handled by a different abs
> pattern for non-negative values.
> 
> Bootstrapped and tested on x86_64-linux-gnu.

Ok

Richard 

>    PR tree-optimization/127689
> 
> gcc/ChangeLog:
> 
>    * match.pd (`(trunc)abs((ext)a)`): Move right
>    after `abs((ext)a)` pattern.  Restrict to
>    non integral types.
> 
> gcc/testsuite/ChangeLog:
> 
>    * gcc.dg/torture/pr127689-1.c: New test.
> 
> Signed-off-by: Andrea Pinski <andrew.pinski@oss.qualcomm.com>
> ---
> gcc/match.pd                              | 22 +++++++++---------
> gcc/testsuite/gcc.dg/torture/pr127689-1.c | 28 +++++++++++++++++++++++
> 2 files changed, 39 insertions(+), 11 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr127689-1.c
> 
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 2d1161b860b..648bda9bf11 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -214,6 +214,17 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
>    (convert (absu:utype @0)))))
> 
> +/* (trunc)abs (extend x) --> abs (x) for non-integral types.  */
> +(simplify
> + (convert (abs (convert@1 @0)))
> +  (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +       && !HONOR_SNANS (@1)
> +       && types_match (type, TREE_TYPE (@0))
> +       && element_precision (type) < element_precision (TREE_TYPE (@1))
> +       && (!VECTOR_TYPE_P (type)
> +       || target_supports_op_p (type, ABS_EXPR, optab_default)))
> +   (abs @0)))
> +
> #if GIMPLE
> /* Optimize (X + (X >> (prec - 1))) ^ (X >> (prec - 1)) into abs (X).  */
> (simplify
> @@ -10340,17 +10351,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>      (IFN_COPYSIGN @0 { build_minus_one_cst (type); })
>      (IFN_COPYSIGN @0 { build_one_cst (type); })))))
> 
> -/* (trunc)abs (extend x) --> abs (x)  */
> -(simplify
> - (convert (abs (convert@1 @0)))
> -  (if (optimize
> -      && !HONOR_SNANS (@1)
> -      && types_match (type, TREE_TYPE (@0))
> -      && element_precision (type) < element_precision (TREE_TYPE (@1))
> -      && (!VECTOR_TYPE_P (type)
> -      || target_supports_op_p (type, ABS_EXPR, optab_default)))
> -   (abs @0)))
> -
> (for froms (BUILT_IN_FMAF BUILT_IN_FMA BUILT_IN_FMAL)
>      tos (IFN_FMA IFN_FMA IFN_FMA)
>  (simplify
> diff --git a/gcc/testsuite/gcc.dg/torture/pr127689-1.c b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
> new file mode 100644
> index 00000000000..cc169744bf2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
> @@ -0,0 +1,28 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-fno-tree-forwprop" } */
> +/* PR tree-optimization/127689 */
> +
> +
> +__attribute__((noipa))void check(int v)
> +{
> +    if (v != -4)
> +      __builtin_exit(1);
> +}
> +int p;
> +signed char s, v;
> +signed char t = 0x80;
> +__attribute__((noipa))
> +void y(signed char w) {
> +  int t1;
> +  t1 = __builtin_abs(t1); //256, 0x80
> +  p = t1;
> +  s = p;
> +  signed char t2 = t1; // -256, 0x80
> +  t = t2 % 3; // -2
> +  v = 2 * t; // -4
> +    check(v);
> +}
> +int main()
> +{
> +    y(0x80);
> +}
> --
> 2.34.1
>
Andrea Pinski Oct. 3, 2026, 11:27 a.m. UTC | #2 
On Sat, Oct 3, 2026 at 1:13 AM Andrea Pinski
<andrew.pinski@oss.qualcomm.com> wrote:
>
> There are 2 patterns that are similar in match.pd for changing into
> a smaller abs. One handles `abs((ext)a)` and one for `(trunc)abs((ext)a)`.
> The one with the truncate afterwards does not use absu for integer types
> which can introduce undefined behavior in some cases.
> So let's restrict the second one to non integral types as the first one
> will handle those always; the check for casting from unsigned is another
> difference but the casting from unsigned is handled by a different abs
> pattern for non-negative values.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
>         PR tree-optimization/127689
>
> gcc/ChangeLog:
>
>         * match.pd (`(trunc)abs((ext)a)`): Move right
>         after `abs((ext)a)` pattern.  Restrict to
>         non integral types.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/torture/pr127689-1.c: New test.
>
> Signed-off-by: Andrea Pinski <andrew.pinski@oss.qualcomm.com>
> ---
>  gcc/match.pd                              | 22 +++++++++---------
>  gcc/testsuite/gcc.dg/torture/pr127689-1.c | 28 +++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 11 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr127689-1.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 2d1161b860b..648bda9bf11 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -214,6 +214,17 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
>     (convert (absu:utype @0)))))
>
> +/* (trunc)abs (extend x) --> abs (x) for non-integral types.  */
> +(simplify
> + (convert (abs (convert@1 @0)))
> +  (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +       && !HONOR_SNANS (@1)
> +       && types_match (type, TREE_TYPE (@0))
> +       && element_precision (type) < element_precision (TREE_TYPE (@1))
> +       && (!VECTOR_TYPE_P (type)
> +          || target_supports_op_p (type, ABS_EXPR, optab_default)))
> +   (abs @0)))
> +
>  #if GIMPLE
>  /* Optimize (X + (X >> (prec - 1))) ^ (X >> (prec - 1)) into abs (X).  */
>  (simplify
> @@ -10340,17 +10351,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>       (IFN_COPYSIGN @0 { build_minus_one_cst (type); })
>       (IFN_COPYSIGN @0 { build_one_cst (type); })))))
>
> -/* (trunc)abs (extend x) --> abs (x)  */
> -(simplify
> - (convert (abs (convert@1 @0)))
> -  (if (optimize
> -      && !HONOR_SNANS (@1)
> -      && types_match (type, TREE_TYPE (@0))
> -      && element_precision (type) < element_precision (TREE_TYPE (@1))
> -      && (!VECTOR_TYPE_P (type)
> -         || target_supports_op_p (type, ABS_EXPR, optab_default)))
> -   (abs @0)))
> -
>  (for froms (BUILT_IN_FMAF BUILT_IN_FMA BUILT_IN_FMAL)
>       tos (IFN_FMA IFN_FMA IFN_FMA)
>   (simplify
> diff --git a/gcc/testsuite/gcc.dg/torture/pr127689-1.c b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
> new file mode 100644
> index 00000000000..cc169744bf2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
> @@ -0,0 +1,28 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-fno-tree-forwprop" } */
> +/* PR tree-optimization/127689 */
> +
> +
> +__attribute__((noipa))void check(int v)
> +{
> +    if (v != -4)
> +      __builtin_exit(1);
> +}
> +int p;
> +signed char s, v;
> +signed char t = 0x80;
> +__attribute__((noipa))
> +void y(signed char w) {
> +  int t1;

Pushed with a small update to the testcase (which I missed when I was
moving from godbolt to the testsuite; and then forgot to test the
testcase until right before pushing the commit).

Thanks,
Andrea

> +  t1 = __builtin_abs(t1); //256, 0x80
> +  p = t1;
> +  s = p;
> +  signed char t2 = t1; // -256, 0x80
> +  t = t2 % 3; // -2
> +  v = 2 * t; // -4
> +    check(v);
> +}
> +int main()
> +{
> +    y(0x80);
> +}
> --
> 2.34.1
>
Andrea Pinski Oct. 3, 2026, 11:29 a.m. UTC | #3 
On Sat, Oct 3, 2026 at 4:27 AM Andrea Pinski
<andrew.pinski@oss.qualcomm.com> wrote:
>
> On Sat, Oct 3, 2026 at 1:13 AM Andrea Pinski
> <andrew.pinski@oss.qualcomm.com> wrote:
> >
> > There are 2 patterns that are similar in match.pd for changing into
> > a smaller abs. One handles `abs((ext)a)` and one for `(trunc)abs((ext)a)`.
> > The one with the truncate afterwards does not use absu for integer types
> > which can introduce undefined behavior in some cases.
> > So let's restrict the second one to non integral types as the first one
> > will handle those always; the check for casting from unsigned is another
> > difference but the casting from unsigned is handled by a different abs
> > pattern for non-negative values.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> >         PR tree-optimization/127689
> >
> > gcc/ChangeLog:
> >
> >         * match.pd (`(trunc)abs((ext)a)`): Move right
> >         after `abs((ext)a)` pattern.  Restrict to
> >         non integral types.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.dg/torture/pr127689-1.c: New test.
> >
> > Signed-off-by: Andrea Pinski <andrew.pinski@oss.qualcomm.com>
> > ---
> >  gcc/match.pd                              | 22 +++++++++---------
> >  gcc/testsuite/gcc.dg/torture/pr127689-1.c | 28 +++++++++++++++++++++++
> >  2 files changed, 39 insertions(+), 11 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/torture/pr127689-1.c
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 2d1161b860b..648bda9bf11 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -214,6 +214,17 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >    (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
> >     (convert (absu:utype @0)))))
> >
> > +/* (trunc)abs (extend x) --> abs (x) for non-integral types.  */
> > +(simplify
> > + (convert (abs (convert@1 @0)))
> > +  (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > +       && !HONOR_SNANS (@1)
> > +       && types_match (type, TREE_TYPE (@0))
> > +       && element_precision (type) < element_precision (TREE_TYPE (@1))
> > +       && (!VECTOR_TYPE_P (type)
> > +          || target_supports_op_p (type, ABS_EXPR, optab_default)))
> > +   (abs @0)))
> > +
> >  #if GIMPLE
> >  /* Optimize (X + (X >> (prec - 1))) ^ (X >> (prec - 1)) into abs (X).  */
> >  (simplify
> > @@ -10340,17 +10351,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >       (IFN_COPYSIGN @0 { build_minus_one_cst (type); })
> >       (IFN_COPYSIGN @0 { build_one_cst (type); })))))
> >
> > -/* (trunc)abs (extend x) --> abs (x)  */
> > -(simplify
> > - (convert (abs (convert@1 @0)))
> > -  (if (optimize
> > -      && !HONOR_SNANS (@1)
> > -      && types_match (type, TREE_TYPE (@0))
> > -      && element_precision (type) < element_precision (TREE_TYPE (@1))
> > -      && (!VECTOR_TYPE_P (type)
> > -         || target_supports_op_p (type, ABS_EXPR, optab_default)))
> > -   (abs @0)))
> > -
> >  (for froms (BUILT_IN_FMAF BUILT_IN_FMA BUILT_IN_FMAL)
> >       tos (IFN_FMA IFN_FMA IFN_FMA)
> >   (simplify
> > diff --git a/gcc/testsuite/gcc.dg/torture/pr127689-1.c b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
> > new file mode 100644
> > index 00000000000..cc169744bf2
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
> > @@ -0,0 +1,28 @@
> > +/* { dg-do run } */
> > +/* { dg-additional-options "-fno-tree-forwprop" } */
> > +/* PR tree-optimization/127689 */
> > +
> > +
> > +__attribute__((noipa))void check(int v)
> > +{
> > +    if (v != -4)
> > +      __builtin_exit(1);
> > +}
> > +int p;
> > +signed char s, v;
> > +signed char t = 0x80;
> > +__attribute__((noipa))
> > +void y(signed char w) {
> > +  int t1;
>
> Pushed with a small update to the testcase (which I missed when I was
> moving from godbolt to the testsuite; and then forgot to test the
> testcase until right before pushing the commit).

Oh I forgot to say what the change was.
Initialize t1 to w.
That is:
int t1;
replaced with:
int t1 = w;

>
> Thanks,
> Andrea
>
> > +  t1 = __builtin_abs(t1); //256, 0x80
> > +  p = t1;
> > +  s = p;
> > +  signed char t2 = t1; // -256, 0x80
> > +  t = t2 % 3; // -2
> > +  v = 2 * t; // -4
> > +    check(v);
> > +}
> > +int main()
> > +{
> > +    y(0x80);
> > +}
> > --
> > 2.34.1
> >
diff mbox series

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 2d1161b860b..648bda9bf11 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -214,6 +214,17 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
    (convert (absu:utype @0)))))
 
+/* (trunc)abs (extend x) --> abs (x) for non-integral types.  */
+(simplify
+ (convert (abs (convert@1 @0)))
+  (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+       && !HONOR_SNANS (@1)
+       && types_match (type, TREE_TYPE (@0))
+       && element_precision (type) < element_precision (TREE_TYPE (@1))
+       && (!VECTOR_TYPE_P (type)
+	   || target_supports_op_p (type, ABS_EXPR, optab_default)))
+   (abs @0)))
+
 #if GIMPLE
 /* Optimize (X + (X >> (prec - 1))) ^ (X >> (prec - 1)) into abs (X).  */
 (simplify
@@ -10340,17 +10351,6 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
      (IFN_COPYSIGN @0 { build_minus_one_cst (type); })
      (IFN_COPYSIGN @0 { build_one_cst (type); })))))
 
-/* (trunc)abs (extend x) --> abs (x)  */
-(simplify
- (convert (abs (convert@1 @0)))
-  (if (optimize
-      && !HONOR_SNANS (@1)
-      && types_match (type, TREE_TYPE (@0))
-      && element_precision (type) < element_precision (TREE_TYPE (@1))
-      && (!VECTOR_TYPE_P (type)
-	  || target_supports_op_p (type, ABS_EXPR, optab_default)))
-   (abs @0)))
-
 (for froms (BUILT_IN_FMAF BUILT_IN_FMA BUILT_IN_FMAL)
      tos (IFN_FMA IFN_FMA IFN_FMA)
  (simplify
diff --git a/gcc/testsuite/gcc.dg/torture/pr127689-1.c b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
new file mode 100644
index 00000000000..cc169744bf2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr127689-1.c
@@ -0,0 +1,28 @@ 
+/* { dg-do run } */
+/* { dg-additional-options "-fno-tree-forwprop" } */
+/* PR tree-optimization/127689 */
+
+
+__attribute__((noipa))void check(int v)
+{
+    if (v != -4)
+      __builtin_exit(1);
+}
+int p;
+signed char s, v;
+signed char t = 0x80;
+__attribute__((noipa))
+void y(signed char w) {
+  int t1;
+  t1 = __builtin_abs(t1); //256, 0x80
+  p = t1;
+  s = p;
+  signed char t2 = t1; // -256, 0x80
+  t = t2 % 3; // -2
+  v = 2 * t; // -4
+    check(v);
+}
+int main()
+{
+    y(0x80);
+}