On Thu, Oct 31, 2024 at 4:44 PM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> There are a couple of things wrong with this pattern which
> I missed during the review. First each nop_convert should
> be nop_convert1 or nop_convert2.
> Second is we need to the minus in the same type as the minus
> was originally so we don't introduce extra undefined behavior
> (signed integer overflow). And we need a convert into the new
> type too.
>
> pr117363-1.c tests not introducing extra undefined behavior.
> pr117363-2.c tests the casting to the correct final type, ldist
> introduces the cond_expr here.
OK.
> Bootstraped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/117363
>
> gcc/ChangeLog:
>
> * match.pd (`a != 0 ? a - 1 : 0`): Fix type handling
> and nop_convert handling.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/torture/pr117363-1.c: New test.
> * gcc.dg/torture/pr117363-2.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/match.pd | 5 +--
> gcc/testsuite/gcc.dg/torture/pr117363-1.c | 41 +++++++++++++++++++++++
> gcc/testsuite/gcc.dg/torture/pr117363-2.c | 12 +++++++
> 3 files changed, 56 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr117363-1.c
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr117363-2.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index c851ac56e37..9107e6a95ca 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3396,10 +3396,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> simplify (X != 0 ? X + ~0 : 0) to (X - X != 0). */
> (simplify
> (cond (ne@1 @0 integer_zerop)
> - (nop_convert? (plus (nop_convert? @0) integer_all_onesp))
> + (nop_convert1? (plus (nop_convert2?@2 @0) integer_all_onesp))
> integer_zerop)
> (if (INTEGRAL_TYPE_P (type))
> - (minus @0 (convert @1))))
> + (with { tree itype = TREE_TYPE (@2); }
> + (convert (minus @2 (convert:itype @1))))))
>
> /* Signed saturation sub, case 1:
> T minus = (T)((UT)X - (UT)Y);
> diff --git a/gcc/testsuite/gcc.dg/torture/pr117363-1.c
> b/gcc/testsuite/gcc.dg/torture/pr117363-1.c
> new file mode 100644
> index 00000000000..b8b5bea76f8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr117363-1.c
> @@ -0,0 +1,41 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-fgimple" } */
> +
> +/* PR tree-optimization/117363 */
> +
> +/* a != 0 ? (signed)(((unsigned)a) - 1) : 0
> + Should not be transformed into doing the
> + plus in a signed type which could cause an overflow.` */
> +
> +__attribute__((noinline))
> +signed __GIMPLE ()
> +test2 (int n)
> +{
> + unsigned t;
> + _Bool _4;
> + if (n_1(D) > 0) goto unreachable;
> + else goto normal;
> +normal:
> + t_2 = (unsigned)n_1(D);
> + t_3 = t_2 - 1u;
> + n_5 = (signed) t_3;
> + _4 = n_1(D) != 0;
> + n_6 = _4 ? n_5 : 0;
> + if (n_6 > 0) goto return1;
> + else goto trap;
> +
> +return1:
> + return n_6;
> +
> +unreachable:
> + __builtin_unreachable();
> +
> +trap:
> + __builtin_trap ();
> +}
> +
> +int main()
> +{
> + unsigned t = -__INT_MAX__ - 1;
> + test2(t);
> +}
> diff --git a/gcc/testsuite/gcc.dg/torture/pr117363-2.c
> b/gcc/testsuite/gcc.dg/torture/pr117363-2.c
> new file mode 100644
> index 00000000000..f608380e62f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr117363-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +
> +/* PR tree-optimization/117363 */
> +/* ldist produces `s != 0 ? s - 1 : 0` (with casts) and that
> + the match pattern which messed up the converts. */
> +
> +void f(int *array, long t) {
> + if (!t) return;
> + unsigned long s = ~t;
> + for (long i = 0; i < s; i++)
> + array[i] = 0;
> +}
> --
> 2.43.0
>
