On Thu, Aug 22, 2024 at 1:47 PM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> The problem here was a missing save_expr around arg0 since
> it is used twice, once in REALPART_EXPR and once in IMAGPART_EXPR.
> Thia adds the save_expr and reformats the code slightly so it is a
> little easier to understand. It excludes the case when arg0 is
> a COMPLEX_EXPR since in that case we'll end up with the distinct
> real and imaginary parts. This is important to retain early
> optimization in some testcases.
>
> Bootstapped and tested on x86_64-linux-gnu with no regressions, pushed.
Eh, it was me who sent this mail after 'git am' of a patch from bugzilla,
editing and testing it and then using git format-patch ...
Richard.
> PR 116454
>
> gcc/ChangeLog:
>
> * fold-const.cc (fold_binary_loc): Fix `a * +-1i`
> by wrapping arg0 with save_expr when it is not COMPLEX_EXPR.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/torture/pr116454-1.c: New test.
> * gcc.dg/torture/pr116454-2.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> Co-Authored-By: Richard Biener <rguent...@suse.de>
> ---
> gcc/fold-const.cc | 32 ++++++++++++++++-------
> gcc/testsuite/gcc.dg/torture/pr116454-1.c | 16 ++++++++++++
> gcc/testsuite/gcc.dg/torture/pr116454-2.c | 12 +++++++++
> 3 files changed, 50 insertions(+), 10 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr116454-1.c
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr116454-2.c
>
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index eeadc8db9f6..35402a59768 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -12093,17 +12093,29 @@ fold_binary_loc (location_t loc, enum tree_code
> code, tree type,
> {
> tree rtype = TREE_TYPE (TREE_TYPE (arg0));
> if (real_onep (TREE_IMAGPART (arg1)))
> - return
> - fold_build2_loc (loc, COMPLEX_EXPR, type,
> - negate_expr (fold_build1_loc (loc,
> IMAGPART_EXPR,
> - rtype, arg0)),
> - fold_build1_loc (loc, REALPART_EXPR, rtype,
> arg0));
> + {
> + if (TREE_CODE (arg0) != COMPLEX_EXPR)
> + arg0 = save_expr (arg0);
> + tree iarg0 = fold_build1_loc (loc, IMAGPART_EXPR,
> + rtype, arg0);
> + tree rarg0 = fold_build1_loc (loc, REALPART_EXPR,
> + rtype, arg0);
> + return fold_build2_loc (loc, COMPLEX_EXPR, type,
> + negate_expr (iarg0),
> + rarg0);
> + }
> else if (real_minus_onep (TREE_IMAGPART (arg1)))
> - return
> - fold_build2_loc (loc, COMPLEX_EXPR, type,
> - fold_build1_loc (loc, IMAGPART_EXPR, rtype,
> arg0),
> - negate_expr (fold_build1_loc (loc,
> REALPART_EXPR,
> - rtype, arg0)));
> + {
> + if (TREE_CODE (arg0) != COMPLEX_EXPR)
> + arg0 = save_expr (arg0);
> + tree iarg0 = fold_build1_loc (loc, IMAGPART_EXPR,
> + rtype, arg0);
> + tree rarg0 = fold_build1_loc (loc, REALPART_EXPR,
> + rtype, arg0);
> + return fold_build2_loc (loc, COMPLEX_EXPR, type,
> + iarg0,
> + negate_expr (rarg0));
> + }
> }
>
> /* Optimize z * conj(z) for floating point complex numbers.
> diff --git a/gcc/testsuite/gcc.dg/torture/pr116454-1.c
> b/gcc/testsuite/gcc.dg/torture/pr116454-1.c
> new file mode 100644
> index 00000000000..6210dcce4a4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr116454-1.c
> @@ -0,0 +1,16 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-ffast-math" } */
> +
> +static int t = 0;
> +_Complex float f()
> +{
> + t++;
> + return 0;
> +}
> +int main() {
> + t = 0;
> + /* Would cause f() to be incorrectly invoked twice. */
> + f() * 1j;
> + if (t != 1)
> + __builtin_abort();
> +}
> diff --git a/gcc/testsuite/gcc.dg/torture/pr116454-2.c
> b/gcc/testsuite/gcc.dg/torture/pr116454-2.c
> new file mode 100644
> index 00000000000..a1e1604e616
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr116454-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-ffast-math" } */
> +_Complex float arr[2];
> +
> +int main() {
> + _Complex float *ptr;
> + ptr = arr;
> + *++ptr * 1j;
> + /* ptr should only increment once, not twice. */
> + if (ptr != arr + 1)
> + __builtin_abort ();
> +}
> --
> 2.43.0
