* [PATCH] c++: Fix up -fstrong-eval-order handling of call arguments [PR70796]
@ 2021-11-04 14:07 Jakub Jelinek
2021-11-05 10:07 ` [PATCH] c++, v2: " Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2021-11-04 14:07 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
For -fstrong-eval-order (default for C++17 and later) we make sure to
gimplify arguments in the right order, but as the following testcase
shows that is not enough.
The problem is that some lvalues can satisfy the is_gimple_val / fb_rvalue
predicate used by gimplify_arg for is_gimple_reg_type typed expressions,
or is_gimple_lvalue / fb_either used for other types.
E.g. in foo we have:
C::C (&p, ++i, ++i)
before gimplification where i is an automatic int variable and without this
patch gimplify that as:
i = i + 1;
i = i + 1;
C::C (&p, i, i);
which means that the ctor is called with the original i value incremented
by 2 in both arguments, while because the call is CALL_EXPR_ORDERED_ARGS
the first argument should be different. Similarly in qux we have:
B::B (&p, TARGET_EXPR <D.2274, *(const struct A &) A::operator++ (&i)>,
TARGET_EXPR <D.2275, *(const struct A &) A::operator++ (&i)>)
and gimplify it as:
_1 = A::operator++ (&i);
_2 = A::operator++ (&i);
B::B (&p, MEM[(const struct A &)_1], MEM[(const struct A &)_2]);
but because A::operator++ returns the passed in argument, again we have
the same value in both cases due to gimplify_arg doing:
/* Also strip a TARGET_EXPR that would force an extra copy. */
if (TREE_CODE (*arg_p) == TARGET_EXPR)
{
tree init = TARGET_EXPR_INITIAL (*arg_p);
if (init
&& !VOID_TYPE_P (TREE_TYPE (init)))
*arg_p = init;
}
which is perfectly fine optimization for calls with unordered arguments,
but breaks the ordered ones.
Lastly, in corge, we have before gimplification:
D::foo (NON_LVALUE_EXPR <p>, 3, ++p)
and gimplify it as
p = p + 4;
D::foo (p, 3, p);
which is again wrong, because the this argument isn't before the
side-effects but after it.
The following patch adds cp_gimplify_arg wrapper, which if ordered
and is_gimple_reg_type forces non-SSA_NAME is_gimple_variable
result into a temporary, and if ordered, not is_gimple_reg_type
and argument is TARGET_EXPR bypasses the gimplify_arg optimization.
So, in foo with this patch we gimplify it as:
i = i + 1;
i.0_1 = i;
i = i + 1;
C::C (&p, i.0_1, i);
in qux as:
_1 = A::operator++ (&i);
D.2312 = MEM[(const struct A &)_1];
_2 = A::operator++ (&i);
B::B (&p, D.2312, MEM[(const struct A &)_2]);
where D.2312 is a temporary and in corge as:
p.9_1 = p;
p = p + 4;
D::foo (p.9_1, 3, p);
The is_gimple_reg_type forcing into a temporary should be really cheap
(I think even at -O0 it should be optimized if there is no modification in
between), the aggregate copies might be more expensive but I think e.g. SRA
or FRE should be able to deal with those if there are no intervening
changes. But still, the patch tries to avoid those when it is cheaply
provable that nothing bad happens (if no argument following it in the
strong evaluation order doesn't have TREE_SIDE_EFFECTS, then even VAR_DECLs
etc. shouldn't be modified after it). For the METHOD_TYPE first argument
I use a temporary always though, that should be always is_gimple_reg_type...
I've tried if e.g.
int i = 1;
return i << ++i;
doesn't suffer from this problem as well, but it doesn't, the FE uses
SAVE_EXPR <i>, SAVE_EXPR <i> << ++i;
in that case which gimplifies the way we want (temporary in the first
operand).
Ok for trunk if it passes bootstrap/regtest?
2021-11-04 Jakub Jelinek <jakub@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-04 14:55:46.473306970 +0100
@@ -398,6 +398,42 @@ 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)
+ *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 +649,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 +659,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;
}
@@ -640,7 +685,8 @@ cp_gimplify_expr (tree *expr_p, gimple_s
if (TREE_CODE (fntype) == METHOD_TYPE)
{
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,
+ true);
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] c++, v2: Fix up -fstrong-eval-order handling of call arguments [PR70796]
2021-11-04 14:07 [PATCH] c++: Fix up -fstrong-eval-order handling of call arguments [PR70796] Jakub Jelinek
@ 2021-11-05 10:07 ` Jakub Jelinek
2021-11-18 22:34 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2021-11-05 10:07 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
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 <jakub@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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++, v2: Fix up -fstrong-eval-order handling of call arguments [PR70796]
2021-11-05 10:07 ` [PATCH] c++, v2: " Jakub Jelinek
@ 2021-11-18 22:34 ` Jason Merrill
0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2021-11-18 22:34 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 11/5/21 06:07, Jakub Jelinek wrote:
> 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?
OK.
> 2021-11-05 Jakub Jelinek <jakub@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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-18 22:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 14:07 [PATCH] c++: Fix up -fstrong-eval-order handling of call arguments [PR70796] Jakub Jelinek
2021-11-05 10:07 ` [PATCH] c++, v2: " Jakub Jelinek
2021-11-18 22:34 ` 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).