| Message ID | 20240610144901.3723532-1-pan2.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match | expand |
pan2.li@intel.com writes: > From: Pan Li <pan2.li@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.li@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; > +} This testcases ICEs for me on x86-64 too (without your patch) with just -O2. Can you move it out of the riscv suite? (I suspect the other fails on x86-64 too). > 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); > match_unsigned_saturation_add (&gsi, psi.phi ()); > }
On 6/10/24 8:49 AM, pan2.li@intel.com wrote: > From: Pan Li <pan2.li@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. So the patch looks fine. I'm just trying to parse the testing. If you did an x86 bootstrap & regression test, you wouldn't be using newlib. That would be a native bootstrap & regression test which would use whatever C library is already installed on the system. I'm assuming that's what you did. If my assumption is correct, then this is fine for the trunk. jeff
On 6/10/24 8:49 AM, pan2.li@intel.com wrote: > 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. I did a fresh x86_64 bootstrap and regression test and pushed this. jeff
Thank a lot, Jeff. Pan -----Original Message----- From: Jeff Law <jeffreyalaw@gmail.com> Sent: Tuesday, June 11, 2024 4:15 AM To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org Cc: juzhe.zhong@rivai.ai; kito.cheng@gmail.com; richard.guenther@gmail.com Subject: Re: [PATCH v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match On 6/10/24 8:49 AM, pan2.li@intel.com wrote: > 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. I did a fresh x86_64 bootstrap and regression test and pushed this. jeff
Hi Sam, > This testcases ICEs for me on x86-64 too (without your patch) with just -O2. > Can you move it out of the riscv suite? (I suspect the other fails on x86-64 too). Sure thing, but do you have any suggestion about where should I put these 2 cases? There are sorts of sub-directories under gcc/testsuite, I am not very familiar that where is the best reasonable location. Pan -----Original Message----- From: Sam James <sam@gentoo.org> Sent: Monday, June 10, 2024 11:33 PM To: Li, Pan2 <pan2.li@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; richard.guenther@gmail.com Subject: Re: [PATCH v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match pan2.li@intel.com writes: > From: Pan Li <pan2.li@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.li@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; > +} This testcases ICEs for me on x86-64 too (without your patch) with just -O2. Can you move it out of the riscv suite? (I suspect the other fails on x86-64 too). > 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); > match_unsigned_saturation_add (&gsi, psi.phi ()); > }
On 6/10/24 7:28 PM, Li, Pan2 wrote: > Hi Sam, > >> This testcases ICEs for me on x86-64 too (without your patch) with just -O2. >> Can you move it out of the riscv suite? (I suspect the other fails on x86-64 too). > > Sure thing, but do you have any suggestion about where should I put these 2 cases? > There are sorts of sub-directories under gcc/testsuite, I am not very familiar that where > is the best reasonable location. gcc.dg/torture would be the most natural location I think. jeff
Got it, thanks. Let me prepare the patch after test. Pan -----Original Message----- From: Jeff Law <jeffreyalaw@gmail.com> Sent: Tuesday, June 11, 2024 9:42 AM To: Li, Pan2 <pan2.li@intel.com>; Sam James <sam@gentoo.org> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; richard.guenther@gmail.com Subject: Re: [PATCH v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match On 6/10/24 7:28 PM, Li, Pan2 wrote: > Hi Sam, > >> This testcases ICEs for me on x86-64 too (without your patch) with just -O2. >> Can you move it out of the riscv suite? (I suspect the other fails on x86-64 too). > > Sure thing, but do you have any suggestion about where should I put these 2 cases? > There are sorts of sub-directories under gcc/testsuite, I am not very familiar that where > is the best reasonable location. gcc.dg/torture would be the most natural location I think. jeff
On Mon, Jun 10, 2024 at 4:49 PM <pan2.li@intel.com> wrote: > > From: Pan Li <pan2.li@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.li@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 >
Thanks Richard for comments. > This should use gsi_after_labels (bb); otherwise you'll ICE when there's a label > in the BB. > Please fix the label issue though. Sure. > 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. Got it, the PHI matching code ensured this. But I may lose the point about the scenario you mentioned, aka "a first stmt return twice since you may not insert anything before that". Could you help to explain more about it? Thanks a lot. Pan -----Original Message----- From: Richard Biener <richard.guenther@gmail.com> Sent: Tuesday, June 11, 2024 3:07 PM To: Li, Pan2 <pan2.li@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com Subject: Re: [PATCH v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match On Mon, Jun 10, 2024 at 4:49 PM <pan2.li@intel.com> wrote: > > From: Pan Li <pan2.li@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.li@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 >
On Tue, Jun 11, 2024 at 9:45 AM Li, Pan2 <pan2.li@intel.com> wrote: > > Thanks Richard for comments. > > > This should use gsi_after_labels (bb); otherwise you'll ICE when there's a label > > in the BB. > > Please fix the label issue though. > > Sure. > > > 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. > > Got it, the PHI matching code ensured this. > But I may lose the point about the scenario you mentioned, aka > "a first stmt return twice since you may not insert anything before that". > Could you help to explain more about it? Thanks a lot. When we have a setjmp call an incoming abnormal edge represents the alternate return from the call from any point that can do longjmp. Inserting before the call would be inserting on that edge which cannot be done. It's a bit of an awkward representation since in reality we'd have to split the call into the actual call and the longjmp receiver. Richard. > > Pan > > -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Tuesday, June 11, 2024 3:07 PM > To: Li, Pan2 <pan2.li@intel.com> > Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com > Subject: Re: [PATCH v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match > > On Mon, Jun 10, 2024 at 4:49 PM <pan2.li@intel.com> wrote: > > > > From: Pan Li <pan2.li@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.li@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 > >
Got it. Thanks Richard. Pan -----Original Message----- From: Richard Biener <richard.guenther@gmail.com> Sent: Tuesday, June 11, 2024 5:31 PM To: Li, Pan2 <pan2.li@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com Subject: Re: [PATCH v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match On Tue, Jun 11, 2024 at 9:45 AM Li, Pan2 <pan2.li@intel.com> wrote: > > Thanks Richard for comments. > > > This should use gsi_after_labels (bb); otherwise you'll ICE when there's a label > > in the BB. > > Please fix the label issue though. > > Sure. > > > 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. > > Got it, the PHI matching code ensured this. > But I may lose the point about the scenario you mentioned, aka > "a first stmt return twice since you may not insert anything before that". > Could you help to explain more about it? Thanks a lot. When we have a setjmp call an incoming abnormal edge represents the alternate return from the call from any point that can do longjmp. Inserting before the call would be inserting on that edge which cannot be done. It's a bit of an awkward representation since in reality we'd have to split the call into the actual call and the longjmp receiver. Richard. > > Pan > > -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Tuesday, June 11, 2024 3:07 PM > To: Li, Pan2 <pan2.li@intel.com> > Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com > Subject: Re: [PATCH v1] Widening-Mul: Fix one ICE of gcall insertion for PHI match > > On Mon, Jun 10, 2024 at 4:49 PM <pan2.li@intel.com> wrote: > > > > From: Pan Li <pan2.li@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.li@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 > >
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); match_unsigned_saturation_add (&gsi, psi.phi ()); }