Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/13/23 11:26, Jakub Jelinek wrote:
On Wed, Dec 13, 2023 at 11:24:42AM -0500, Jason Merrill wrote:
gcc/testsuite/ChangeLog:
* g++.dg/pr112822.C: Require C++17.
---
gcc/testsuite/g++.dg/pr112822.C | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/testsuite/g++.dg/pr112822.C b/gcc/testsuite/g++.dg/pr112822.C
index a8557522467..9949fbb08ac 100644
--- a/gcc/testsuite/g++.dg/pr112822.C
+++ b/gcc/testsuite/g++.dg/pr112822.C
@@ -1,6 +1,7 @@
/* PR tree-optimization/112822 */
/* { dg-do compile { target c++17 } } */
/* { dg-options "-w -O2" } */
+// { dg-do compile { target c++17 } }
2 dg-do compile directives?
Oops, I assumed that since my commit still applied on rebase that it
hadn't been fixed yet, and didn't see the additional discussion this
morning. Reverted.
Jason
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On Wed, Dec 13, 2023 at 11:24:42AM -0500, Jason Merrill wrote:
> gcc/testsuite/ChangeLog:
>
> * g++.dg/pr112822.C: Require C++17.
> ---
> gcc/testsuite/g++.dg/pr112822.C | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/gcc/testsuite/g++.dg/pr112822.C b/gcc/testsuite/g++.dg/pr112822.C
> index a8557522467..9949fbb08ac 100644
> --- a/gcc/testsuite/g++.dg/pr112822.C
> +++ b/gcc/testsuite/g++.dg/pr112822.C
> @@ -1,6 +1,7 @@
> /* PR tree-optimization/112822 */
> /* { dg-do compile { target c++17 } } */
> /* { dg-options "-w -O2" } */
> +// { dg-do compile { target c++17 } }
2 dg-do compile directives?
Jakub
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/12/23 21:36, Jason Merrill wrote:
On 12/12/23 17:50, Peter Bergner wrote:
On 12/12/23 1:26 PM, Richard Biener wrote:
Am 12.12.2023 um 19:51 schrieb Peter Bergner :
On 12/12/23 12:45 PM, Peter Bergner wrote:
+/* PR target/112822 */
Oops, this should be:
/* PR tree-optimization/112822 */
It's fixed on my end.
Ok
Pushed now that Martin has pushed his fix. Thanks!
This test is failing for me below C++17, I think you need
// { dg-do compile { target c++17 } }
or
// { dg-require-effective-target c++17 }
Fixed thus.
From d2b269ce30d77dbfc6c28c75887c330d4698b132 Mon Sep 17 00:00:00 2001
From: Jason Merrill
Date: Tue, 12 Dec 2023 21:33:11 -0500
Subject: [PATCH] testsuite: fix g++.dg/pr112822.C
To: gcc-patches@gcc.gnu.org
gcc/testsuite/ChangeLog:
* g++.dg/pr112822.C: Require C++17.
---
gcc/testsuite/g++.dg/pr112822.C | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/testsuite/g++.dg/pr112822.C b/gcc/testsuite/g++.dg/pr112822.C
index a8557522467..9949fbb08ac 100644
--- a/gcc/testsuite/g++.dg/pr112822.C
+++ b/gcc/testsuite/g++.dg/pr112822.C
@@ -1,6 +1,7 @@
/* PR tree-optimization/112822 */
/* { dg-do compile { target c++17 } } */
/* { dg-options "-w -O2" } */
+// { dg-do compile { target c++17 } }
/* Verify we do not ICE on the following noisy creduced test case. */
--
2.39.3
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/13/23 2:05 AM, Jakub Jelinek wrote:
> On Wed, Dec 13, 2023 at 08:51:16AM +0100, Richard Biener wrote:
>> On Tue, 12 Dec 2023, Peter Bergner wrote:
>>
>>> On 12/12/23 8:36 PM, Jason Merrill wrote:
This test is failing for me below C++17, I think you need
// { dg-do compile { target c++17 } }
or
// { dg-require-effective-target c++17 }
>>>
>>> Sorry about that. Should we do the above or should we just add
>>> -std=c++17 to dg-options? ...or do we need to do both?
>>
>> Just do the above, the C++ testsuite iterates over all standards,
>> adding -std=c++17 would just run that 5 times. But the above
>> properly skips unsupported cases.
>
> I believe if one uses explicit -std=gnu++17 or -std=c++17 in dg-options
> then it will not iterate:
> # If the testcase specifies a standard, use that one.
> # If not, run it under several standards, allowing GNU extensions
> # if there's a dg-options line.
> if ![search_for $test "-std=*++"] {
> and otherwise how many times exactly it iterates depends on what the user
> asked for or what effective target is there (normally the default is
> to iterate 4 times (98,14,17,20), one can use e.g.
> GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 to iterate 7 times, or the
> default also changes if c++23, c++26 or c++11_only effective targets
> are present somewhere in the test.
>
> But sure, if the test is valid in C++17, 20, 23, 26, then
> // { dg-do compile { target c++17 } }
> is best (unless the test is mostly language version independent and
> very expensive to compile or run).
I confirmed the test case builds with C++17, 20, 23, 26 and errors out
with C++11, so I went with your solution. Thanks for the input and
sorry for the breakage. Pushed.
Peter
testsuite: Add dg-do compile target c++17 directive for testcase [PR112822]
Add dg-do compile target directive that limits the test case to being built
on c++17 compiles or greater.
2023-12-13 Peter Bergner
gcc/testsuite/
PR tree-optimization/112822
* g++.dg/pr112822.C: Add dg-do compile target c++17 directive.
diff --git a/gcc/testsuite/g++.dg/pr112822.C b/gcc/testsuite/g++.dg/pr112822.C
index d1490405493..a8557522467 100644
--- a/gcc/testsuite/g++.dg/pr112822.C
+++ b/gcc/testsuite/g++.dg/pr112822.C
@@ -1,4 +1,5 @@
/* PR tree-optimization/112822 */
+/* { dg-do compile { target c++17 } } */
/* { dg-options "-w -O2" } */
/* Verify we do not ICE on the following noisy creduced test case. */
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On Wed, Dec 13, 2023 at 08:51:16AM +0100, Richard Biener wrote:
> On Tue, 12 Dec 2023, Peter Bergner wrote:
>
> > On 12/12/23 8:36 PM, Jason Merrill wrote:
> > > This test is failing for me below C++17, I think you need
> > >
> > > // { dg-do compile { target c++17 } }
> > > or
> > > // { dg-require-effective-target c++17 }
> >
> > Sorry about that. Should we do the above or should we just add
> > -std=c++17 to dg-options? ...or do we need to do both?
>
> Just do the above, the C++ testsuite iterates over all standards,
> adding -std=c++17 would just run that 5 times. But the above
> properly skips unsupported cases.
I believe if one uses explicit -std=gnu++17 or -std=c++17 in dg-options
then it will not iterate:
# If the testcase specifies a standard, use that one.
# If not, run it under several standards, allowing GNU extensions
# if there's a dg-options line.
if ![search_for $test "-std=*++"] {
and otherwise how many times exactly it iterates depends on what the user
asked for or what effective target is there (normally the default is
to iterate 4 times (98,14,17,20), one can use e.g.
GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 to iterate 7 times, or the
default also changes if c++23, c++26 or c++11_only effective targets
are present somewhere in the test.
But sure, if the test is valid in C++17, 20, 23, 26, then
// { dg-do compile { target c++17 } }
is best (unless the test is mostly language version independent and
very expensive to compile or run).
Jakub
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On Tue, 12 Dec 2023, Peter Bergner wrote:
> On 12/12/23 8:36 PM, Jason Merrill wrote:
> > This test is failing for me below C++17, I think you need
> >
> > // { dg-do compile { target c++17 } }
> > or
> > // { dg-require-effective-target c++17 }
>
> Sorry about that. Should we do the above or should we just add
> -std=c++17 to dg-options? ...or do we need to do both?
Just do the above, the C++ testsuite iterates over all standards,
adding -std=c++17 would just run that 5 times. But the above
properly skips unsupported cases.
Richard.
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/12/23 8:36 PM, Jason Merrill wrote:
> This test is failing for me below C++17, I think you need
>
> // { dg-do compile { target c++17 } }
> or
> // { dg-require-effective-target c++17 }
Sorry about that. Should we do the above or should we just add
-std=c++17 to dg-options? ...or do we need to do both?
Peter
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/12/23 17:50, Peter Bergner wrote:
On 12/12/23 1:26 PM, Richard Biener wrote:
Am 12.12.2023 um 19:51 schrieb Peter Bergner :
On 12/12/23 12:45 PM, Peter Bergner wrote:
+/* PR target/112822 */
Oops, this should be:
/* PR tree-optimization/112822 */
It's fixed on my end.
Ok
Pushed now that Martin has pushed his fix. Thanks!
This test is failing for me below C++17, I think you need
// { dg-do compile { target c++17 } }
or
// { dg-require-effective-target c++17 }
Jason
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/12/23 1:26 PM, Richard Biener wrote: >> Am 12.12.2023 um 19:51 schrieb Peter Bergner : >> >> On 12/12/23 12:45 PM, Peter Bergner wrote: >>> +/* PR target/112822 */ >> >> Oops, this should be: >> >> /* PR tree-optimization/112822 */ >> >> It's fixed on my end. > > Ok Pushed now that Martin has pushed his fix. Thanks! Peter
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
> Am 12.12.2023 um 19:51 schrieb Peter Bergner : > > On 12/12/23 12:45 PM, Peter Bergner wrote: >> +/* PR target/112822 */ > > Oops, this should be: > > /* PR tree-optimization/112822 */ > > It's fixed on my end. Ok Richard > Peter > > > >
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/12/23 12:45 PM, Peter Bergner wrote: > +/* PR target/112822 */ Oops, this should be: /* PR tree-optimization/112822 */ It's fixed on my end. Peter
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
On 12/12/23 10:50 AM, Martin Jambor wrote:
> The testcase has reasonable size but it is specific to ppc64le and its
> altivec vectors. My plan is to ask the bug reporter to massage it into
> a target specific testcase in bugzilla. Alternatively I can try to
> craft a testcase from scratch but that will take time.
I rewrote the Altivec specific part of the testcase to use generic C code
and it still ICEs for me on ppc64le using an unpatched compiler. Therefore,
I think we can just add the updated testcase to the generic g++ tests.
I'll note I was wrong in the bugzilla comments, -O3 -mcpu=power10 is not
required to hit the ICE. A simple -O2 on ppc64le is enough to hit the ICE.
Ok for trunk?
Peter
testsuite: Add testcase for already fixed PR [PR112822]
gcc/testsuite/
PR tree-optimization/112822
* g++.dg/pr112822.C: New test.
diff --git a/gcc/testsuite/g++.dg/pr112822.C b/gcc/testsuite/g++.dg/pr112822.C
new file mode 100644
index 000..3921d5c1bbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr112822.C
@@ -0,0 +1,369 @@
+/* PR target/112822 */
+/* { dg-options "-w -O2" } */
+
+/* Verify we do not ICE on the following noisy creduced test case. */
+
+namespace b {
+typedef int c;
+template struct d;
+template struct d { using f = e; };
+template struct aa;
+template struct aa { using f = h; };
+template using ab = typename d::f;
+template using n = typename aa::f;
+template class af {
+public:
+ typedef __complex__ ah;
+ template af operator+=(e) {
+ah o;
+x = o;
+return *this;
+ }
+ ah x;
+};
+} // namespace b
+namespace {
+enum { p };
+enum { ac, ad };
+struct ae;
+struct al;
+struct ag;
+typedef b::c an;
+namespace ai {
+template struct ak { typedef aj f; };
+template using ar = typename ak::f;
+template struct am {
+ enum { at };
+};
+template struct ao {
+ enum { at };
+};
+template struct ap;
+template struct aq {
+ enum { at };
+};
+} // namespace ai
+template struct ay;
+template class as;
+template class ba;
+template class aw;
+template class be;
+template class az;
+namespace ai {
+template struct bg;
+template ::bd>
+struct bk;
+template struct bf;
+template struct bm;
+template struct bh;
+template ::bj>::at> struct bp {
+ typedef bi f;
+};
+template struct br {
+ typedef typename bp::f>::f f;
+};
+template struct bn;
+template struct bn {
+ typedef aw f;
+};
+template struct bx {
+ typedef typename bn::bs, aj ::bo>::f f;
+};
+template struct bt { typedef b::n<0, aj, aj> f; };
+template ::f> struct cb {
+ enum { bw };
+ typedef b::n::f> f;
+};
+template ::bs> struct by {
+ typedef be f;
+};
+template struct bz {
+ typedef typename by::f f;
+};
+template struct ch;
+template struct ch { typedef ci bd; };
+} // namespace ai
+template > struct cg;
+template struct cg { typedef aj cn; };
+namespace ai {
+template cj cp;
+template void cl(bu *cr, cj cs) { ct(cr, cs); }
+typedef __attribute__((altivec(vector__))) double co;
+void ct(double *cr, co cs) { *(co *)cr = cs; }
+struct cq {
+ co q;
+};
+template <> struct bm> { typedef cq f; };
+template <> struct bh { typedef cq bj; };
+void ct(b::af *cr, cq cs) { ct((double *)cr, cs.q); }
+template struct cx {
+ template void cu(cw *a, cj) {
+cl(a, cp);
+ }
+};
+} // namespace ai
+template class ba : public ay {
+public:
+ typedef ai::ap bu;
+ typedef b::n::bo, bu, b::n::at, bu, bu>> cv;
+ typedef ay db;
+ db::dc;
+ cv coeff(an dd, an col) const { return dc().coeff(dd, col); }
+};
+template class cz : public ba::at> {
+public:
+ ai::ap b;
+ enum { da, dg, dh, bv, bq, di = dg, bo };
+};
+template class be : public cz {
+public:
+ typedef typename ai::ap::bu bu;
+ typedef cz db;
+ db::dc;
+ template cd +=(const be &);
+ template az df(de);
+};
+template struct ay {
+ cd () { return *static_cast(this); }
+ cd dc() const;
+};
+template class dl;
+namespace ai {
+template struct ap> {
+ typedef bb dj;
+ typedef bc r;
+ typedef ap s;
+ typedef ap t;
+ typedef typename cg::cn bu;
+ typedef typename ch::bd>::bd cf;
+ enum { bo };
+};
+} // namespace ai
+template
+class az : public dl, ai::ap, ai::bg::bd>> {
+public:
+ typedef dk bb;
+ typedef Rhs_ bc;
+ typedef typename ai::bt::f LhsNested;
+ typedef typename ai::bt::f dn;
+ typedef ai::ar u;
+ typedef ai::ar RhsNestedCleaned;
+ u lhs();
+ RhsNestedCleaned rhs();
+};
+template
+class dl : public ai::bz, al>::f {};
+namespace ai {
+template struct v { typedef ag w; };
+template struct evaluator_traits_base {
+ typedef typename v::cf>::w w;
+};
+template struct ax : evaluator_traits_base {};
+template struct y { static const bool at = false; };
+template class plainobjectbase_evaluator_data {
+public:
+ plainobjectbase_evaluator_data(bu *ptr, an) : data(ptr) {}
+ an outerStride() { return z; }
+ bu *data;
+};
+template struct evaluator {
+ typedef cd PlainObjectType;
+ typedef typename PlainObjectType::bu bu;
+ enum { IsVectorAtCompileTime };
+ enum {
Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
> Am 12.12.2023 um 17:50 schrieb Martin Jambor :
>
> Hi,
>
> PR 112822 revealed a corner case in load_assign_lhs_subreplacements
> where it creates invalid gimple: an assignment where on the LHS there
> is a complex variable which however is not a gimple register because
> it has partial defs and on the right hand side there is a
> VIEW_CONVERT_EXPR. This patch invokes force_gimple_operand_gsi on
> such statements (like it already does when both sides of a generated
> assignment have partial definitions.
>
> I've made sure the patch passes bootstrap and testsuite on x86_64-linux,
> the bug reporter was kind enough to also check the same on an
> powerpc64le-linux (see bugzilla comment #8).
>
> The testcase has reasonable size but it is specific to ppc64le and its
> altivec vectors. My plan is to ask the bug reporter to massage it into
> a target specific testcase in bugzilla. Alternatively I can try to
> craft a testcase from scratch but that will take time.
>
> Despite the above, is the patch OK for master?
Ok
Richard
>
> Thanks,
>
> Martin
>
>
>
> gcc/ChangeLog:
>
> 2023-12-12 Martin Jambor
>
>PR tree-optimization/112822
>* tree-sra.cc (load_assign_lhs_subreplacements): Invoke
>force_gimple_operand_gsi also when LHS has partial stores and RHS is a
>VIEW_CONVERT_EXPR.
> ---
> gcc/tree-sra.cc | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
> index 3bd0c7a9af0..99a1b0a6d17 100644
> --- a/gcc/tree-sra.cc
> +++ b/gcc/tree-sra.cc
> @@ -4219,11 +4219,15 @@ load_assign_lhs_subreplacements (struct access *lacc,
> if (racc && racc->grp_to_be_replaced)
>{
> rhs = get_access_replacement (racc);
> + bool vce = false;
> if (!useless_type_conversion_p (lacc->type, racc->type))
> -rhs = fold_build1_loc (sad->loc, VIEW_CONVERT_EXPR,
> - lacc->type, rhs);
> +{
> + rhs = fold_build1_loc (sad->loc, VIEW_CONVERT_EXPR,
> + lacc->type, rhs);
> + vce = true;
> +}
>
> - if (racc->grp_partial_lhs && lacc->grp_partial_lhs)
> + if (lacc->grp_partial_lhs && (vce || racc->grp_partial_lhs))
>rhs = force_gimple_operand_gsi (>old_gsi, rhs, true,
>NULL_TREE, true, GSI_SAME_STMT);
>}
> --
> 2.43.0
>
