* [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
@ 2023-12-12 16:50 Martin Jambor
2023-12-12 17:17 ` Richard Biener
2023-12-12 18:45 ` Peter Bergner
0 siblings, 2 replies; 14+ messages in thread
From: Martin Jambor @ 2023-12-12 16:50 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener
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?
Thanks,
Martin
gcc/ChangeLog:
2023-12-12 Martin Jambor <mjambor@suse.cz>
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 (&sad->old_gsi, rhs, true,
NULL_TREE, true, GSI_SAME_STMT);
}
--
2.43.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-12 16:50 [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822) Martin Jambor
@ 2023-12-12 17:17 ` Richard Biener
2023-12-12 18:45 ` Peter Bergner
1 sibling, 0 replies; 14+ messages in thread
From: Richard Biener @ 2023-12-12 17:17 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches
> Am 12.12.2023 um 17:50 schrieb Martin Jambor <mjambor@suse.cz>:
>
> 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 <mjambor@suse.cz>
>
> 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 (&sad->old_gsi, rhs, true,
> NULL_TREE, true, GSI_SAME_STMT);
> }
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-12 16:50 [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822) Martin Jambor
2023-12-12 17:17 ` Richard Biener
@ 2023-12-12 18:45 ` Peter Bergner
2023-12-12 18:51 ` Peter Bergner
1 sibling, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2023-12-12 18:45 UTC (permalink / raw)
To: Martin Jambor, GCC Patches; +Cc: Richard Biener
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 00000000000..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 <bool, typename> struct d;
+template <typename e> struct d<true, e> { using f = e; };
+template <bool, typename, typename> struct aa;
+template <typename g, typename h> struct aa<false, g, h> { using f = h; };
+template <bool l, typename e> using ab = typename d<l, e>::f;
+template <bool l, typename g, typename h> using n = typename aa<l, g, h>::f;
+template <typename> class af {
+public:
+ typedef __complex__ ah;
+ template <typename e> 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 <typename aj> struct ak { typedef aj f; };
+template <typename aj> using ar = typename ak<aj>::f;
+template <typename> struct am {
+ enum { at };
+};
+template <typename, typename> struct ao {
+ enum { at };
+};
+template <typename> struct ap;
+template <typename> struct aq {
+ enum { at };
+};
+} // namespace ai
+template <typename> struct ay;
+template <typename> class as;
+template <typename, int> class ba;
+template <typename, int au, int av, int = 0, int = au, int = av> class aw;
+template <typename> class be;
+template <typename, typename, int> class az;
+namespace ai {
+template <typename, typename> struct bg;
+template <typename aj, int = bg<typename aj::bb, typename aj::bc>::bd>
+struct bk;
+template <typename, typename> struct bf;
+template <typename> struct bm;
+template <typename> struct bh;
+template <int, typename bi, bool = ao<bi, typename bh<bi>::bj>::at> struct bp {
+ typedef bi f;
+};
+template <typename aj, int bl> struct br {
+ typedef typename bp<bl, typename bm<aj>::f>::f f;
+};
+template <typename, typename, int> struct bn;
+template <typename aj, int bo> struct bn<aj, al, bo> {
+ typedef aw<typename aj ::bu, aj ::bv, aj ::bq> f;
+};
+template <typename aj> struct bx {
+ typedef typename bn<aj, typename ap<aj>::bs, aj ::bo>::f f;
+};
+template <typename aj> struct bt { typedef b::n<0, aj, aj> f; };
+template <typename aj, int, typename ca = typename bx<aj>::f> struct cb {
+ enum { bw };
+ typedef b::n<bw, ca, typename bt<aj>::f> f;
+};
+template <typename cd, typename = typename ap<cd>::bs> struct by {
+ typedef be<cd> f;
+};
+template <typename cd, typename bs> struct bz {
+ typedef typename by<cd, bs>::f f;
+};
+template <typename, typename, int> struct ch;
+template <typename ci, int cc> struct ch<ci, ci, cc> { typedef ci bd; };
+} // namespace ai
+template <typename ck, typename ce, typename = ai::bf<ck, ce>> struct cg;
+template <typename aj, typename cm> struct cg<aj, cm> { typedef aj cn; };
+namespace ai {
+template <typename cj, int> cj cp;
+template <typename bu, typename cj, int> 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<b::af<double>> { typedef cq f; };
+template <> struct bh<cq> { typedef cq bj; };
+void ct(b::af<double> *cr, cq cs) { ct((double *)cr, cs.q); }
+template <typename cw, typename> struct cx {
+ template <int cy, typename cj> void cu(cw *a, cj) {
+ cl<cw, cj, cy>(a, cp<cj, cy>);
+ }
+};
+} // namespace ai
+template <typename cd> class ba<cd, ac> : public ay<cd> {
+public:
+ typedef ai::ap<cd> bu;
+ typedef b::n<ai::ap<cd>::bo, bu, b::n<ai::am<bu>::at, bu, bu>> cv;
+ typedef ay<cd> db;
+ db::dc;
+ cv coeff(an dd, an col) const { return dc().coeff(dd, col); }
+};
+template <typename cd> class cz : public ba<cd, ai::aq<cd>::at> {
+public:
+ ai::ap<cd> b;
+ enum { da, dg, dh, bv, bq, di = dg, bo };
+};
+template <typename cd> class be : public cz<cd> {
+public:
+ typedef typename ai::ap<cd>::bu bu;
+ typedef cz<cd> db;
+ db::dc;
+ template <typename de> cd &operator+=(const be<de> &);
+ template <typename de> az<cd, de, ad> df(de);
+};
+template <typename cd> struct ay {
+ cd &dc() { return *static_cast<cd *>(this); }
+ cd dc() const;
+};
+template <typename, typename, int, typename> class dl;
+namespace ai {
+template <typename bb, typename bc, int dm> struct ap<az<bb, bc, dm>> {
+ typedef bb dj;
+ typedef bc r;
+ typedef ap<dj> s;
+ typedef ap<r> t;
+ typedef typename cg<typename dj ::bu, typename r ::bu>::cn bu;
+ typedef typename ch<typename s::cf, typename t::cf, bg<bb, bc>::bd>::bd cf;
+ enum { bo };
+};
+} // namespace ai
+template <typename dk, typename Rhs_, int dm>
+class az : public dl<dk, Rhs_, dm,
+ ai::ch<ai::ap<dk>, ai::ap<Rhs_>, ai::bg<dk, Rhs_>::bd>> {
+public:
+ typedef dk bb;
+ typedef Rhs_ bc;
+ typedef typename ai::bt<bb>::f LhsNested;
+ typedef typename ai::bt<bc>::f dn;
+ typedef ai::ar<LhsNested> u;
+ typedef ai::ar<dn> RhsNestedCleaned;
+ u lhs();
+ RhsNestedCleaned rhs();
+};
+template <typename bb, typename bc, int dm, typename>
+class dl : public ai::bz<az<bb, bc, dm>, al>::f {};
+namespace ai {
+template <typename> struct v { typedef ag w; };
+template <typename aj> struct evaluator_traits_base {
+ typedef typename v<typename ap<aj>::cf>::w w;
+};
+template <typename aj> struct ax : evaluator_traits_base<aj> {};
+template <typename> struct y { static const bool at = false; };
+template <typename bu, int z> class plainobjectbase_evaluator_data {
+public:
+ plainobjectbase_evaluator_data(bu *ptr, an) : data(ptr) {}
+ an outerStride() { return z; }
+ bu *data;
+};
+template <typename cd> struct evaluator {
+ typedef cd PlainObjectType;
+ typedef typename PlainObjectType::bu bu;
+ enum { IsVectorAtCompileTime };
+ enum { OuterStrideAtCompileTime };
+ evaluator(PlainObjectType &m) : m_d(m.data(), IsVectorAtCompileTime) {}
+ bu &coeffRef(an, an) { return m_d.data[m_d.outerStride()]; }
+ plainobjectbase_evaluator_data<bu, OuterStrideAtCompileTime> m_d;
+};
+template <typename bu, int Rows, int Cols, int Options, int MaxRows,
+ int MaxCols>
+struct evaluator<aw<bu, Rows, Cols, Options, MaxRows, MaxCols>>
+ : evaluator<as<aw<bu, Rows, Cols>>> {
+ typedef aw<bu, Rows, Cols> XprType;
+ evaluator(XprType &m) : evaluator<as<XprType>>(m) {}
+};
+template <typename DstEvaluator, typename, typename>
+struct copy_using_evaluator_traits {
+ typedef typename DstEvaluator::bu cw;
+ enum { RestrictedInnerSize };
+ typedef typename br<cw, RestrictedInnerSize>::f bi;
+};
+template <typename Kernel, an, int>
+struct copy_using_evaluator_innervec_CompleteUnrolling {
+ typedef typename Kernel::bi bi;
+ enum { outer, inner, SrcAlignment, DstAlignment };
+ static void run(Kernel kernel) {
+ kernel.template assignPacketByOuterInner<DstAlignment, SrcAlignment, bi>(
+ outer, inner);
+ }
+};
+template <typename Kernel> struct dense_assignment_loop {
+ static void run(Kernel kernel) {
+ typedef typename Kernel::DstEvaluatorType::XprType DstXprType;
+ copy_using_evaluator_innervec_CompleteUnrolling<
+ Kernel, 0, DstXprType::dh>::run(kernel);
+ }
+};
+template <typename DstEvaluatorTypeT, typename SrcEvaluatorTypeT,
+ typename Functor>
+class generic_dense_assignment_kernel {
+ typedef typename DstEvaluatorTypeT::XprType DstXprType;
+
+public:
+ typedef DstEvaluatorTypeT DstEvaluatorType;
+ typedef SrcEvaluatorTypeT SrcEvaluatorType;
+ typedef copy_using_evaluator_traits<DstEvaluatorTypeT, SrcEvaluatorTypeT,
+ Functor>
+ AssignmentTraits;
+ typedef typename AssignmentTraits::bi bi;
+ generic_dense_assignment_kernel(DstEvaluatorType dst, SrcEvaluatorType src,
+ Functor, DstXprType dstExpr)
+ : m_dst(dst), m_src(src), m_dstExpr(dstExpr) {}
+ template <int StoreMode, int LoadMode, typename cj> void cu(an dd, an col) {
+ m_functor.template cu<StoreMode>(
+ &m_dst.coeffRef(dd, col), m_src.template packet<LoadMode, cj>(dd, col));
+ }
+ template <int StoreMode, int LoadMode, typename cj>
+ void assignPacketByOuterInner(an, an) {
+ an dd;
+ an col;
+ cu<StoreMode, LoadMode, cj>(dd, col);
+ }
+ DstEvaluatorType m_dst;
+ SrcEvaluatorType &m_src;
+ Functor m_functor;
+ DstXprType m_dstExpr;
+};
+template <typename DstXprType, typename SrcXprType, typename Functor>
+void call_dense_assignment_loop(DstXprType &dst, SrcXprType src, Functor func) {
+ typedef evaluator<DstXprType> DstEvaluatorType;
+ typedef evaluator<SrcXprType> SrcEvaluatorType;
+ SrcEvaluatorType srcEvaluator(src);
+ DstEvaluatorType dstEvaluator(dst);
+ typedef generic_dense_assignment_kernel<DstEvaluatorType, SrcEvaluatorType,
+ Functor>
+ Kernel;
+ Kernel kernel(dstEvaluator, srcEvaluator, func, dst);
+ dense_assignment_loop<Kernel>::run(kernel);
+}
+template <typename, typename> struct AssignmentKind;
+struct Dense2Dense;
+template <> struct AssignmentKind<ag, ag> { typedef Dense2Dense Kind; };
+template <typename DstXprType, typename SrcXprType, typename,
+ typename = typename AssignmentKind<typename ax<DstXprType>::w,
+ typename ax<SrcXprType>::w>::Kind,
+ typename = void>
+struct Assignment;
+template <typename Dst, typename Src, typename Func>
+void call_assignment(Dst &dst, Src src, Func func,
+ b::ab<!y<Src>::at, void *> = 0) {
+ enum { NeedToTranspose };
+ typedef b::n<NeedToTranspose, Dst, Dst> ActualDstTypeCleaned;
+ typedef b::n<NeedToTranspose, Dst, Dst &> ActualDstType;
+ ActualDstType actualDst(dst);
+ Assignment<ActualDstTypeCleaned, Src, Func>::run(actualDst, src, func);
+}
+template <typename DstXprType, typename SrcXprType, typename Functor,
+ typename Weak>
+struct Assignment<DstXprType, SrcXprType, Functor, Weak> {
+ static void run(DstXprType &dst, SrcXprType src, Functor func) {
+ call_dense_assignment_loop(dst, src, func);
+ }
+};
+template <typename aj, int bl> struct plain_array { aj array[bl]; };
+} // namespace ai
+template <typename aj, int bl, int, int av, int> class DenseStorage {
+ ai::plain_array<aj, bl> m_data;
+
+public:
+ an cols() { return av; }
+ aj *data() { return m_data.array; }
+};
+template <typename cd> class as : public ai::by<cd>::f {
+public:
+ enum { Options };
+ typedef typename ai::by<cd>::f db;
+ typedef typename ai::ap<cd>::bu bu;
+ DenseStorage<bu, db::di, db::da, db::dg, Options> m_storage;
+ an cols() { return m_storage.cols(); }
+ bu &coeffRef(an, an colId) { return data()[colId]; }
+ bu *data() { return m_storage.data(); }
+};
+namespace ai {
+template <typename Scalar_, int au, int av, int Options_, int MaxRows_,
+ int MaxCols_>
+struct ap<aw<Scalar_, au, av, Options_, MaxRows_, MaxCols_>> {
+ typedef Scalar_ bu;
+ typedef ae cf;
+ typedef al bs;
+ enum { bo };
+};
+} // namespace ai
+template <typename Scalar_, int au, int, int Options_, int, int>
+class aw : public as<aw<Scalar_, au, Options_>> {
+public:
+ template <typename T0, typename T1> aw(T0, T1) {}
+};
+template <typename cd>
+template <typename de>
+cd &be<cd>::operator+=(const be<de> &other) {
+ call_assignment(dc(), other.dc(), ai::cx<bu, bu>());
+ return dc();
+}
+namespace ai {
+template <typename, typename> struct bg {
+ enum { bd };
+};
+template <typename bb, typename bc, int Options>
+struct evaluator<az<bb, bc, Options>> : bk<az<bb, bc, Options>> {
+ typedef az<bb, bc, Options> XprType;
+ typedef bk<XprType> db;
+ evaluator(XprType xpr) : db(xpr) {}
+};
+template <typename bb, typename bc, int cc> struct bk<az<bb, bc, ad>, cc> {
+ typedef az<bb, bc, ad> XprType;
+ bk(XprType xpr)
+ : m_lhs(xpr.lhs()), m_rhs(xpr.rhs()), m_lhsImpl(m_lhs), m_rhsImpl(m_rhs) {
+ }
+ typedef typename cb<bb, bc::dg>::f LhsNested;
+ typedef typename cb<bc, bb::da>::f dn;
+ typedef LhsNested u;
+ typedef dn RhsNestedCleaned;
+ typedef u LhsEtorType;
+ typedef RhsNestedCleaned RhsEtorType;
+ template <int, typename bi> bi packet(an, an);
+ LhsNested m_lhs;
+ dn m_rhs;
+ LhsEtorType m_lhsImpl;
+ RhsEtorType m_rhsImpl;
+};
+} // namespace ai
+} // namespace
+namespace Eigen {
+template <typename Type1, typename Type2> bool verifyIsApprox(Type1, Type2);
+}
+using namespace Eigen;
+template <typename TC, typename TA, typename TB> TC ref_prod(TC C, TA, TB B) {
+ for (an i; i;)
+ for (an j = 0; j < C.cols(); ++j)
+ for (an k; k;)
+ C.coeffRef(i, j) += B.coeff(k, j);
+ return C;
+}
+template <typename aj, int Rows, int Cols, int Depth, int OC, int OA, int OB>
+b::ab<!0, void> test_lazy_single(int rows, int cols, int depth) {
+ aw<aj, Depth, OA> ci(rows, depth);
+ aw<aj, Cols, OB> B(depth, cols);
+ aw<aj, Rows, OC> C(rows, cols);
+ aw D(C);
+ verifyIsApprox(C += ci.df(B), ref_prod(D, ci, B));
+}
+template <typename aj, int Rows, int Cols, int Depth>
+void test_lazy_all_layout(int rows = Rows, int cols = Cols, int depth = Depth) {
+ test_lazy_single<aj, Rows, Cols, Depth, p, p, p>(rows, cols, depth);
+}
+template <typename aj> void test_lazy_l2() {
+ test_lazy_all_layout<aj, 1, 4, 2>();
+}
+void fn1() { test_lazy_l2<b::af<double>>(); }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-12 18:45 ` Peter Bergner
@ 2023-12-12 18:51 ` Peter Bergner
2023-12-12 19:26 ` Richard Biener
0 siblings, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2023-12-12 18:51 UTC (permalink / raw)
To: Martin Jambor, GCC Patches; +Cc: Richard Biener
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-12 18:51 ` Peter Bergner
@ 2023-12-12 19:26 ` Richard Biener
2023-12-12 22:50 ` Peter Bergner
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2023-12-12 19:26 UTC (permalink / raw)
To: Peter Bergner; +Cc: Martin Jambor, GCC Patches
> Am 12.12.2023 um 19:51 schrieb Peter Bergner <bergner@linux.ibm.com>:
>
> 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
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-12 19:26 ` Richard Biener
@ 2023-12-12 22:50 ` Peter Bergner
2023-12-13 2:36 ` Jason Merrill
0 siblings, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2023-12-12 22:50 UTC (permalink / raw)
To: Richard Biener; +Cc: Martin Jambor, GCC Patches
On 12/12/23 1:26 PM, Richard Biener wrote:
>> Am 12.12.2023 um 19:51 schrieb Peter Bergner <bergner@linux.ibm.com>:
>>
>> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-12 22:50 ` Peter Bergner
@ 2023-12-13 2:36 ` Jason Merrill
2023-12-13 5:26 ` Peter Bergner
2023-12-13 16:24 ` Jason Merrill
0 siblings, 2 replies; 14+ messages in thread
From: Jason Merrill @ 2023-12-13 2:36 UTC (permalink / raw)
To: Peter Bergner, Richard Biener; +Cc: Martin Jambor, GCC Patches
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 <bergner@linux.ibm.com>:
>>>
>>> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-13 2:36 ` Jason Merrill
@ 2023-12-13 5:26 ` Peter Bergner
2023-12-13 7:51 ` Richard Biener
2023-12-13 16:24 ` Jason Merrill
1 sibling, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2023-12-13 5:26 UTC (permalink / raw)
To: Jason Merrill, Richard Biener; +Cc: Martin Jambor, GCC Patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-13 5:26 ` Peter Bergner
@ 2023-12-13 7:51 ` Richard Biener
2023-12-13 8:05 ` Jakub Jelinek
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2023-12-13 7:51 UTC (permalink / raw)
To: Peter Bergner; +Cc: Jason Merrill, Martin Jambor, GCC Patches
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-13 7:51 ` Richard Biener
@ 2023-12-13 8:05 ` Jakub Jelinek
2023-12-13 14:21 ` Peter Bergner
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Jelinek @ 2023-12-13 8:05 UTC (permalink / raw)
To: Richard Biener; +Cc: Peter Bergner, Jason Merrill, Martin Jambor, GCC Patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-13 8:05 ` Jakub Jelinek
@ 2023-12-13 14:21 ` Peter Bergner
0 siblings, 0 replies; 14+ messages in thread
From: Peter Bergner @ 2023-12-13 14:21 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: Jason Merrill, Martin Jambor, GCC Patches
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 <bergner@linux.ibm.com>
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. */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-13 2:36 ` Jason Merrill
2023-12-13 5:26 ` Peter Bergner
@ 2023-12-13 16:24 ` Jason Merrill
2023-12-13 16:26 ` Jakub Jelinek
1 sibling, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2023-12-13 16:24 UTC (permalink / raw)
To: Peter Bergner; +Cc: Martin Jambor, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 636 bytes --]
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 <bergner@linux.ibm.com>:
>>>>
>>>> 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.
[-- Attachment #2: 0001-testsuite-fix-g-.dg-pr112822.C.patch --]
[-- Type: text/x-patch, Size: 810 bytes --]
From d2b269ce30d77dbfc6c28c75887c330d4698b132 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-13 16:24 ` Jason Merrill
@ 2023-12-13 16:26 ` Jakub Jelinek
2023-12-13 19:23 ` Jason Merrill
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Jelinek @ 2023-12-13 16:26 UTC (permalink / raw)
To: Jason Merrill; +Cc: Peter Bergner, Martin Jambor, GCC Patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822)
2023-12-13 16:26 ` Jakub Jelinek
@ 2023-12-13 19:23 ` Jason Merrill
0 siblings, 0 replies; 14+ messages in thread
From: Jason Merrill @ 2023-12-13 19:23 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Peter Bergner, Martin Jambor, GCC Patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-12-13 19:23 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-12 16:50 [PATCH] SRA: Force gimple operand in an additional corner case (PR 112822) Martin Jambor
2023-12-12 17:17 ` Richard Biener
2023-12-12 18:45 ` Peter Bergner
2023-12-12 18:51 ` Peter Bergner
2023-12-12 19:26 ` Richard Biener
2023-12-12 22:50 ` Peter Bergner
2023-12-13 2:36 ` Jason Merrill
2023-12-13 5:26 ` Peter Bergner
2023-12-13 7:51 ` Richard Biener
2023-12-13 8:05 ` Jakub Jelinek
2023-12-13 14:21 ` Peter Bergner
2023-12-13 16:24 ` Jason Merrill
2023-12-13 16:26 ` Jakub Jelinek
2023-12-13 19:23 ` Jason Merrill
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).