* [PATCH] testsuite: Add test for r11-4123 @ 2022-08-22 16:57 Marek Polacek 2022-08-22 17:16 ` Andrew Pinski 0 siblings, 1 reply; 4+ messages in thread From: Marek Polacek @ 2022-08-22 16:57 UTC (permalink / raw) To: GCC Patches, Richard Biener r11-4123 came without a test but I happened upon a nice test case that got fixed by that revision. So I think it'd be good to add it. The ICE was: phi-1.C: In constructor 'ElementManager::ElementManager()': phi-1.C:28:1: error: missing definition 28 | ElementManager::ElementManager() : array_(makeArray()) {} | ^~~~~~~~~~~~~~ for SSA_NAME: _12 in statement: _10 = PHI <_12(3), _11(5)> PHI argument _12 for PHI node _10 = PHI <_12(3), _11(5)> during GIMPLE pass: fixup_cfg phi-1.C:28:1: internal compiler error: verify_ssa failed Incidentally, do you think it's safe to backport r11-4123 to GCC 8 (downstream)? Thanks. Tested on x86_64-pc-linux-gnu, ok for trunk? gcc/testsuite/ChangeLog: * g++.dg/tree-ssa/phi-1.C: New test. --- gcc/testsuite/g++.dg/tree-ssa/phi-1.C | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phi-1.C diff --git a/gcc/testsuite/g++.dg/tree-ssa/phi-1.C b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C new file mode 100644 index 00000000000..26cade052e3 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C @@ -0,0 +1,28 @@ +// { dg-do compile { target c++11 } } +// { dg-options "-O --param early-inlining-insns=14" } + +struct Element; +template <int _Nm> struct __array_traits { typedef Element _Type[_Nm]; }; +template <int _Nm> struct array { + typename __array_traits<_Nm>::_Type _M_elems; +}; +bool logLevel(); +struct LogCapture { + void stream(); +}; +struct Element { + Element(); + long data_; +}; +using ElementArray = array<6>; +struct ElementManager { + ElementManager(); + ElementArray array_; +}; +static ElementArray makeArray() { + if (logLevel()) + LogCapture().stream(); + ElementArray foo; + return foo; +} +ElementManager::ElementManager() : array_(makeArray()) {} base-commit: b6316324fceaef431799a8b386de5cc9881d6898 -- 2.37.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] testsuite: Add test for r11-4123 2022-08-22 16:57 [PATCH] testsuite: Add test for r11-4123 Marek Polacek @ 2022-08-22 17:16 ` Andrew Pinski 2022-08-22 17:45 ` [PATCH v2] " Marek Polacek 0 siblings, 1 reply; 4+ messages in thread From: Andrew Pinski @ 2022-08-22 17:16 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Richard Biener On Mon, Aug 22, 2022 at 9:58 AM Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > r11-4123 came without a test but I happened upon a nice test case that > got fixed by that revision. So I think it'd be good to add it. The > ICE was: > > phi-1.C: In constructor 'ElementManager::ElementManager()': > phi-1.C:28:1: error: missing definition > 28 | ElementManager::ElementManager() : array_(makeArray()) {} > | ^~~~~~~~~~~~~~ > for SSA_NAME: _12 in statement: > _10 = PHI <_12(3), _11(5)> > PHI argument > _12 > for PHI node > _10 = PHI <_12(3), _11(5)> > during GIMPLE pass: fixup_cfg > phi-1.C:28:1: internal compiler error: verify_ssa failed > > Incidentally, do you think it's safe to backport r11-4123 to GCC 8 > (downstream)? Thanks. > > Tested on x86_64-pc-linux-gnu, ok for trunk? > > gcc/testsuite/ChangeLog: > > * g++.dg/tree-ssa/phi-1.C: New test. > --- > gcc/testsuite/g++.dg/tree-ssa/phi-1.C | 28 +++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phi-1.C > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/phi-1.C b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C > new file mode 100644 > index 00000000000..26cade052e3 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C > @@ -0,0 +1,28 @@ > +// { dg-do compile { target c++11 } } > +// { dg-options "-O --param early-inlining-insns=14" } It seems better to put it in g++.dg/torture and remove -O part. And then it will be tested for more than before. Maybe even just mark makeArray as always_inline/inline and you might be able to remove the param also. Thanks, Andrew Pinski > + > +struct Element; > +template <int _Nm> struct __array_traits { typedef Element _Type[_Nm]; }; > +template <int _Nm> struct array { > + typename __array_traits<_Nm>::_Type _M_elems; > +}; > +bool logLevel(); > +struct LogCapture { > + void stream(); > +}; > +struct Element { > + Element(); > + long data_; > +}; > +using ElementArray = array<6>; > +struct ElementManager { > + ElementManager(); > + ElementArray array_; > +}; > +static ElementArray makeArray() { > + if (logLevel()) > + LogCapture().stream(); > + ElementArray foo; > + return foo; > +} > +ElementManager::ElementManager() : array_(makeArray()) {} > > base-commit: b6316324fceaef431799a8b386de5cc9881d6898 > -- > 2.37.2 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] testsuite: Add test for r11-4123 2022-08-22 17:16 ` Andrew Pinski @ 2022-08-22 17:45 ` Marek Polacek 2022-08-23 6:27 ` Richard Biener 0 siblings, 1 reply; 4+ messages in thread From: Marek Polacek @ 2022-08-22 17:45 UTC (permalink / raw) To: Andrew Pinski; +Cc: GCC Patches, Richard Biener On Mon, Aug 22, 2022 at 10:16:23AM -0700, Andrew Pinski wrote: > On Mon, Aug 22, 2022 at 9:58 AM Marek Polacek via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > r11-4123 came without a test but I happened upon a nice test case that > > got fixed by that revision. So I think it'd be good to add it. The > > ICE was: > > > > phi-1.C: In constructor 'ElementManager::ElementManager()': > > phi-1.C:28:1: error: missing definition > > 28 | ElementManager::ElementManager() : array_(makeArray()) {} > > | ^~~~~~~~~~~~~~ > > for SSA_NAME: _12 in statement: > > _10 = PHI <_12(3), _11(5)> > > PHI argument > > _12 > > for PHI node > > _10 = PHI <_12(3), _11(5)> > > during GIMPLE pass: fixup_cfg > > phi-1.C:28:1: internal compiler error: verify_ssa failed > > > > Incidentally, do you think it's safe to backport r11-4123 to GCC 8 > > (downstream)? Thanks. > > > > Tested on x86_64-pc-linux-gnu, ok for trunk? > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/tree-ssa/phi-1.C: New test. > > --- > > gcc/testsuite/g++.dg/tree-ssa/phi-1.C | 28 +++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phi-1.C > > > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/phi-1.C b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C > > new file mode 100644 > > index 00000000000..26cade052e3 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C > > @@ -0,0 +1,28 @@ > > +// { dg-do compile { target c++11 } } > > +// { dg-options "-O --param early-inlining-insns=14" } > > It seems better to put it in g++.dg/torture and remove -O part. > And then it will be tested for more than before. Sure, I can move it. > Maybe even just mark makeArray as always_inline/inline and you might > be able to remove the param also. I can't, it doesn't reproduce the issue then. -- >8 -- r11-4123 came without a test but I happened upon a nice test case that got fixed by that revision. So I think it'd be good to add it. The ICE was: phi-1.C: In constructor 'ElementManager::ElementManager()': phi-1.C:28:1: error: missing definition 28 | ElementManager::ElementManager() : array_(makeArray()) {} | ^~~~~~~~~~~~~~ for SSA_NAME: _12 in statement: _10 = PHI <_12(3), _11(5)> PHI argument _12 for PHI node _10 = PHI <_12(3), _11(5)> during GIMPLE pass: fixup_cfg phi-1.C:28:1: internal compiler error: verify_ssa failed gcc/testsuite/ChangeLog: * g++.dg/torture/phi-1.C: New test. --- gcc/testsuite/g++.dg/torture/phi-1.C | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 gcc/testsuite/g++.dg/torture/phi-1.C diff --git a/gcc/testsuite/g++.dg/torture/phi-1.C b/gcc/testsuite/g++.dg/torture/phi-1.C new file mode 100644 index 00000000000..69fb3d7ba38 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/phi-1.C @@ -0,0 +1,28 @@ +// { dg-do compile { target c++11 } } +// { dg-options "--param early-inlining-insns=14" } + +struct Element; +template <int _Nm> struct __array_traits { typedef Element _Type[_Nm]; }; +template <int _Nm> struct array { + typename __array_traits<_Nm>::_Type _M_elems; +}; +bool logLevel(); +struct LogCapture { + void stream(); +}; +struct Element { + Element(); + long data_; +}; +using ElementArray = array<6>; +struct ElementManager { + ElementManager(); + ElementArray array_; +}; +static ElementArray makeArray() { + if (logLevel()) + LogCapture().stream(); + ElementArray foo; + return foo; +} +ElementManager::ElementManager() : array_(makeArray()) {} base-commit: b6316324fceaef431799a8b386de5cc9881d6898 -- 2.37.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] testsuite: Add test for r11-4123 2022-08-22 17:45 ` [PATCH v2] " Marek Polacek @ 2022-08-23 6:27 ` Richard Biener 0 siblings, 0 replies; 4+ messages in thread From: Richard Biener @ 2022-08-23 6:27 UTC (permalink / raw) To: Marek Polacek; +Cc: Andrew Pinski, GCC Patches On Mon, 22 Aug 2022, Marek Polacek wrote: > On Mon, Aug 22, 2022 at 10:16:23AM -0700, Andrew Pinski wrote: > > On Mon, Aug 22, 2022 at 9:58 AM Marek Polacek via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > r11-4123 came without a test but I happened upon a nice test case that > > > got fixed by that revision. So I think it'd be good to add it. The > > > ICE was: > > > > > > phi-1.C: In constructor 'ElementManager::ElementManager()': > > > phi-1.C:28:1: error: missing definition > > > 28 | ElementManager::ElementManager() : array_(makeArray()) {} > > > | ^~~~~~~~~~~~~~ > > > for SSA_NAME: _12 in statement: > > > _10 = PHI <_12(3), _11(5)> > > > PHI argument > > > _12 > > > for PHI node > > > _10 = PHI <_12(3), _11(5)> > > > during GIMPLE pass: fixup_cfg > > > phi-1.C:28:1: internal compiler error: verify_ssa failed Interesting. I suspect the issue isn't actually in fixup_cfg though. > > > Incidentally, do you think it's safe to backport r11-4123 to GCC 8 > > > (downstream)? Thanks. It should be safe - the comment says it won't do any edge / PHI arg re-ordering (which has proved to cause issues in the past). > > > Tested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > gcc/testsuite/ChangeLog: > > > > > > * g++.dg/tree-ssa/phi-1.C: New test. > > > --- > > > gcc/testsuite/g++.dg/tree-ssa/phi-1.C | 28 +++++++++++++++++++++++++++ > > > 1 file changed, 28 insertions(+) > > > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phi-1.C > > > > > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/phi-1.C b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C > > > new file mode 100644 > > > index 00000000000..26cade052e3 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/tree-ssa/phi-1.C > > > @@ -0,0 +1,28 @@ > > > +// { dg-do compile { target c++11 } } > > > +// { dg-options "-O --param early-inlining-insns=14" } > > > > It seems better to put it in g++.dg/torture and remove -O part. > > And then it will be tested for more than before. > > Sure, I can move it. The adjusted testcase is OK. Thanks, Richard. > > Maybe even just mark makeArray as always_inline/inline and you might > > be able to remove the param also. > > I can't, it doesn't reproduce the issue then. > > -- >8 -- > r11-4123 came without a test but I happened upon a nice test case that > got fixed by that revision. So I think it'd be good to add it. The > ICE was: > > phi-1.C: In constructor 'ElementManager::ElementManager()': > phi-1.C:28:1: error: missing definition > 28 | ElementManager::ElementManager() : array_(makeArray()) {} > | ^~~~~~~~~~~~~~ > for SSA_NAME: _12 in statement: > _10 = PHI <_12(3), _11(5)> > PHI argument > _12 > for PHI node > _10 = PHI <_12(3), _11(5)> > during GIMPLE pass: fixup_cfg > phi-1.C:28:1: internal compiler error: verify_ssa failed > > gcc/testsuite/ChangeLog: > > * g++.dg/torture/phi-1.C: New test. > --- > gcc/testsuite/g++.dg/torture/phi-1.C | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/torture/phi-1.C > > diff --git a/gcc/testsuite/g++.dg/torture/phi-1.C b/gcc/testsuite/g++.dg/torture/phi-1.C > new file mode 100644 > index 00000000000..69fb3d7ba38 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/torture/phi-1.C > @@ -0,0 +1,28 @@ > +// { dg-do compile { target c++11 } } > +// { dg-options "--param early-inlining-insns=14" } > + > +struct Element; > +template <int _Nm> struct __array_traits { typedef Element _Type[_Nm]; }; > +template <int _Nm> struct array { > + typename __array_traits<_Nm>::_Type _M_elems; > +}; > +bool logLevel(); > +struct LogCapture { > + void stream(); > +}; > +struct Element { > + Element(); > + long data_; > +}; > +using ElementArray = array<6>; > +struct ElementManager { > + ElementManager(); > + ElementArray array_; > +}; > +static ElementArray makeArray() { > + if (logLevel()) > + LogCapture().stream(); > + ElementArray foo; > + return foo; > +} > +ElementManager::ElementManager() : array_(makeArray()) {} > > base-commit: b6316324fceaef431799a8b386de5cc9881d6898 > -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-23 6:27 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-22 16:57 [PATCH] testsuite: Add test for r11-4123 Marek Polacek 2022-08-22 17:16 ` Andrew Pinski 2022-08-22 17:45 ` [PATCH v2] " Marek Polacek 2022-08-23 6:27 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).