On Thu, Nov 04, 2021 at 03:07:57PM +0100, Jakub Jelinek via Gcc-patches wrote:
> For the METHOD_TYPE first argument
> I use a temporary always though, that should be always is_gimple_reg_type...
Doing so regressed
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++11 scan-tree-dump gimple "V::V
.this, _1.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++11 scan-tree-dump gimple "Y::Y
._2, _3.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++14 scan-tree-dump gimple "V::V
.this, _1.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++14 scan-tree-dump gimple "Y::Y
._2, _3.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++17 scan-tree-dump gimple "V::V
.this, _1.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++17 scan-tree-dump gimple "Y::Y
._2, _3.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++20 scan-tree-dump gimple "V::V
.this, _1.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++20 scan-tree-dump gimple "Y::Y
._2, _3.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++2b scan-tree-dump gimple "V::V
.this, _1.;"
+FAIL: g++.dg/cpp1z/inh-ctor23.C -std=gnu++2b scan-tree-dump gimple "Y::Y
._2, _3.;"
because the testcase relies on this being passed directly in gimple dump,
rather than some SSA_NAME based on this.
Instead of changing the testcase, I've figured out that it is actually quite
easy to restore previous behavior here, for 2 reasons even.
One is that there are no side-effects in the ctor call arguments, so
the forcing of this into a temporary wasn't really needed, we can like in
the other cases quite cheaply see if the call has any side-effect arguments.
And the other reason is that in C++ this can't be modified, and similarly
vars with reference type can't be modified, so for those we don't need to
force them into a temporary either even if there are side-effects.
This means e.g. on
struct S
{
void foo (S &, int);
void bar (int);
};
void S::foo (S &p, int x)
{
this->bar (++x);
p.bar (++x);
}
we can keep what we were emitting before even for -std=c++17.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
and after a while for 7.3 too?
2021-11-05 Jakub Jelinek <ja...@redhat.com>
PR c++/70796
* cp-gimplify.c (cp_gimplify_arg): New function.
(cp_gimplify_expr): Use cp_gimplify_arg instead of gimplify_arg,
pass true as last argument to it if there are any following
arguments in strong evaluation order with side-effects.
* g++.dg/cpp1z/eval-order11.C: New test.
--- gcc/cp/cp-gimplify.c.jj 2021-10-29 19:33:10.542344939 +0200
+++ gcc/cp/cp-gimplify.c 2021-11-05 00:41:29.124227336 +0100
@@ -398,6 +398,47 @@ gimplify_to_rvalue (tree *expr_p, gimple
return t;
}
+/* Like gimplify_arg, but if ORDERED is set (which should be set if
+ any of the arguments this argument is sequenced before has
+ TREE_SIDE_EFFECTS set, make sure expressions with is_gimple_reg_type type
+ are gimplified into SSA_NAME or a fresh temporary and for
+ non-is_gimple_reg_type we don't optimize away TARGET_EXPRs. */
+
+static enum gimplify_status
+cp_gimplify_arg (tree *arg_p, gimple_seq *pre_p, location_t call_location,
+ bool ordered)
+{
+ enum gimplify_status t;
+ if (ordered
+ && !is_gimple_reg_type (TREE_TYPE (*arg_p))
+ && TREE_CODE (*arg_p) == TARGET_EXPR)
+ {
+ /* gimplify_arg would strip away the TARGET_EXPR, but
+ that can mean we don't copy the argument and some following
+ argument with side-effect could modify it. */
+ protected_set_expr_location (*arg_p, call_location);
+ return gimplify_expr (arg_p, pre_p, NULL, is_gimple_lvalue, fb_either);
+ }
+ else
+ {
+ t = gimplify_arg (arg_p, pre_p, call_location);
+ if (t == GS_ERROR)
+ return GS_ERROR;
+ else if (ordered
+ && is_gimple_reg_type (TREE_TYPE (*arg_p))
+ && is_gimple_variable (*arg_p)
+ && TREE_CODE (*arg_p) != SSA_NAME
+ /* No need to force references into register, references
+ can't be modified. */
+ && !TYPE_REF_P (TREE_TYPE (*arg_p))
+ /* And this can't be modified either. */
+ && *arg_p != current_class_ptr)
+ *arg_p = get_initialized_tmp_var (*arg_p, pre_p);
+ return t;
+ }
+
+}
+
/* Do C++-specific gimplification. Args are as for gimplify_expr. */
int
@@ -613,7 +654,8 @@ cp_gimplify_expr (tree *expr_p, gimple_s
gcc_assert (call_expr_nargs (*expr_p) == 2);
gcc_assert (!CALL_EXPR_ORDERED_ARGS (*expr_p));
enum gimplify_status t
- = gimplify_arg (&CALL_EXPR_ARG (*expr_p, 1), pre_p, loc);
+ = cp_gimplify_arg (&CALL_EXPR_ARG (*expr_p, 1), pre_p, loc,
+ TREE_SIDE_EFFECTS (CALL_EXPR_ARG (*expr_p, 0)));
if (t == GS_ERROR)
ret = GS_ERROR;
}
@@ -622,10 +664,18 @@ cp_gimplify_expr (tree *expr_p, gimple_s
/* Leave the last argument for gimplify_call_expr, to avoid problems
with __builtin_va_arg_pack(). */
int nargs = call_expr_nargs (*expr_p) - 1;
+ int last_side_effects_arg = -1;
+ for (int i = nargs; i > 0; --i)
+ if (TREE_SIDE_EFFECTS (CALL_EXPR_ARG (*expr_p, i)))
+ {
+ last_side_effects_arg = i;
+ break;
+ }
for (int i = 0; i < nargs; ++i)
{
enum gimplify_status t
- = gimplify_arg (&CALL_EXPR_ARG (*expr_p, i), pre_p, loc);
+ = cp_gimplify_arg (&CALL_EXPR_ARG (*expr_p, i), pre_p, loc,
+ i < last_side_effects_arg);
if (t == GS_ERROR)
ret = GS_ERROR;
}
@@ -639,8 +689,17 @@ cp_gimplify_expr (tree *expr_p, gimple_s
fntype = TREE_TYPE (fntype);
if (TREE_CODE (fntype) == METHOD_TYPE)
{
+ int nargs = call_expr_nargs (*expr_p);
+ bool side_effects = false;
+ for (int i = 1; i < nargs; ++i)
+ if (TREE_SIDE_EFFECTS (CALL_EXPR_ARG (*expr_p, i)))
+ {
+ side_effects = true;
+ break;
+ }
enum gimplify_status t
- = gimplify_arg (&CALL_EXPR_ARG (*expr_p, 0), pre_p, loc);
+ = cp_gimplify_arg (&CALL_EXPR_ARG (*expr_p, 0), pre_p, loc,
+ side_effects);
if (t == GS_ERROR)
ret = GS_ERROR;
}
--- gcc/testsuite/g++.dg/cpp1z/eval-order11.C.jj 2021-11-04
14:02:50.439482571 +0100
+++ gcc/testsuite/g++.dg/cpp1z/eval-order11.C 2021-11-04 14:15:43.850479169
+0100
@@ -0,0 +1,89 @@
+// PR c++/70796
+// { dg-do run { target c++11 } }
+// { dg-options "-fstrong-eval-order" { target c++14_down } }
+
+struct A
+{
+ int x = 0;
+ A & operator ++ () { ++x; return *this; }
+};
+struct B
+{
+ A first, second;
+ B (A x, A y) : first{x}, second{y} {}
+};
+struct C
+{
+ int first, second;
+ C (int x, int y) : first{x}, second{y} {}
+};
+struct D
+{
+ int d;
+ void foo (int x, D *y)
+ {
+ if (y != this + 1)
+ __builtin_abort ();
+ d = x;
+ }
+};
+D d[2] = { { 1 }, { 2 } };
+
+void
+foo ()
+{
+ int i = 0;
+ C p{++i, ++i};
+ if (p.first != 1 || p.second != 2)
+ __builtin_abort ();
+}
+
+void
+bar ()
+{
+ int i = 0;
+ C p{++i, ++i};
+ if (p.first != 1 || p.second != 2)
+ __builtin_abort ();
+ int &j = i;
+ C q{++j, ++j};
+ if (q.first != 3 || q.second != 4)
+ __builtin_abort ();
+}
+
+void
+baz ()
+{
+ int i = 0;
+ C p{(int &) ++i, (int &) ++i};
+ if (p.first != 1 || p.second != 2)
+ __builtin_abort ();
+}
+
+void
+qux ()
+{
+ A i;
+ B p{++i, ++i};
+ if (p.first.x != 1 || p.second.x != 2)
+ __builtin_abort ();
+}
+
+void
+corge ()
+{
+ D *p = &d[0];
+ p->foo (3, ++p);
+ if (d[0].d != 3 || d[1].d != 2)
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ bar ();
+ baz ();
+ foo ();
+ qux ();
+ corge ();
+}
Jakub
