On Mon, Jun 10, 2024 at 4:49 PM <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> When enabled the PHI handing for COND_EXPR, we need to insert the gcall
> to replace the PHI node. Unfortunately, I made a mistake that insert
> the gcall to before the last stmt of the bb. See below gimple, the PHI
> is located at no.1 but we insert the gcall (aka no.9) to the end of
> the bb. Then the use of _9 in no.2 will have no def and will trigger
> ICE when verify_ssa.
>
> 1. # _9 = PHI <_3(4), 18446744073709551615(3)> // The PHI node to be
> deleted.
> 2. prephitmp_36 = (char *) _9;
> 3. buf.write_base = string_13(D);
> 4. buf.write_ptr = string_13(D);
> 5. buf.write_end = prephitmp_36;
> 6. buf.written = 0;
> 7. buf.mode = 3;
> 8. _7 = buf.write_end;
> 9. _9 = .SAT_ADD (string.0_2, maxlen_15(D)); // Insert gcall to last bb
> by mistake
>
> This patch would like to insert the gcall to before the start of the bb
> stmt. To ensure the possible use of PHI_result will have a def exists.
> After this patch the above gimple will be:
>
> 0. _9 = .SAT_ADD (string.0_2, maxlen_15(D)); // Insert gcall to start bb
> by mistake
> 1. # _9 = PHI <_3(4), 18446744073709551615(3)> // The PHI node to be
> deleted.
> 2. prephitmp_36 = (char *) _9;
> 3. buf.write_base = string_13(D);
> 4. buf.write_ptr = string_13(D);
> 5. buf.write_end = prephitmp_36;
> 6. buf.written = 0;
> 7. buf.mode = 3;
> 8. _7 = buf.write_end;
>
> The below test suites are passed for this patch:
> * The rv64gcv fully regression test with newlib.
> * The rv64gcv build with glibc.
> * The x86 regression test with newlib.
> * The x86 bootstrap test with newlib.
>
> PR target/115387
>
> gcc/ChangeLog:
>
> * tree-ssa-math-opts.cc (math_opts_dom_walker::after_dom_children):
> Take
> the gsi of start_bb instead of last_bb.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/pr115387-1.c: New test.
> * gcc.target/riscv/pr115387-2.c: New test.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/testsuite/gcc.target/riscv/pr115387-1.c | 35 +++++++++++++++++++++
> gcc/testsuite/gcc.target/riscv/pr115387-2.c | 18 +++++++++++
> gcc/tree-ssa-math-opts.cc | 2 +-
> 3 files changed, 54 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/pr115387-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/pr115387-2.c
>
> diff --git a/gcc/testsuite/gcc.target/riscv/pr115387-1.c
> b/gcc/testsuite/gcc.target/riscv/pr115387-1.c
> new file mode 100644
> index 00000000000..a1c926977c4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/pr115387-1.c
> @@ -0,0 +1,35 @@
> +/* Test there is no ICE when compile. */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
> +
> +#define PRINTF_CHK 0x34
> +
> +typedef unsigned long uintptr_t;
> +
> +struct __printf_buffer {
> + char *write_ptr;
> + int status;
> +};
> +
> +extern void __printf_buffer_init_end (struct __printf_buffer *, char *, char
> *);
> +
> +void
> +test (char *string, unsigned long maxlen, unsigned mode_flags)
> +{
> + struct __printf_buffer buf;
> +
> + if ((mode_flags & PRINTF_CHK) != 0)
> + {
> + string[0] = '\0';
> + uintptr_t end;
> +
> + if (__builtin_add_overflow ((uintptr_t) string, maxlen, &end))
> + end = -1;
> +
> + __printf_buffer_init_end (&buf, string, (char *) end);
> + }
> + else
> + __printf_buffer_init_end (&buf, string, (char *) ~(uintptr_t) 0);
> +
> + *buf.write_ptr = '\0';
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/pr115387-2.c
> b/gcc/testsuite/gcc.target/riscv/pr115387-2.c
> new file mode 100644
> index 00000000000..7183bf18dfd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/pr115387-2.c
> @@ -0,0 +1,18 @@
> +/* Test there is no ICE when compile. */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
> +
> +#include <stddef.h>
> +#include <stdint-gcc.h>
> +
> +char *
> +test (char *string, size_t maxlen)
> +{
> + string[0] = '\0';
> + uintptr_t end;
> +
> + if (__builtin_add_overflow ((uintptr_t) string, maxlen, &end))
> + end = -1;
> +
> + return (char *) end;
> +}
> diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc
> index 173b0366f5e..fbb8e0ea306 100644
> --- a/gcc/tree-ssa-math-opts.cc
> +++ b/gcc/tree-ssa-math-opts.cc
> @@ -6102,7 +6102,7 @@ math_opts_dom_walker::after_dom_children (basic_block
> bb)
> for (gphi_iterator psi = gsi_start_phis (bb); !gsi_end_p (psi);
> gsi_next (&psi))
> {
> - gimple_stmt_iterator gsi = gsi_last_bb (bb);
> + gimple_stmt_iterator gsi = gsi_start_bb (bb);
This should use gsi_after_labels (bb); otherwise you'll ICE when there's a label
in the BB. You also have to look out for a first stmt that returns twice since
you may not insert anything before that. I would suggest to not match when
BB has abnormal incoming edges which I guess will be ensured by the PHI
matching code anyway, so I just mentioned this insertion restriction.
Please fix the label issue though.
Richard.
> match_unsigned_saturation_add (&gsi, psi.phi ());
> }
>
> --
> 2.34.1
>
