On Wed, Jun 15, 2016 at 6:30 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Tue, Jun 14, 2016 at 10:15 PM, Jason Merrill <ja...@redhat.com> wrote:
>> As discussed in bug 71104, the C++ P0145 proposal specifies the evaluation
>> order of certain operations:
>>
>> 1. a.b
>> 2. a->b
>> 3. a->*b
>> 4. a(b1, b2, b3)
>> 5. b @= a
>> 6. a[b]
>> 7. a << b
>> 8. a >> b
>>
>> The second patch introduces a flag -fargs-in-order to control whether these
>> orders are enforced on calls. -fargs-in-order=1 enforces all but the
>> ordering between function arguments in #4.
>>
>> The first patch implements #5 for the built-in assignment operator by
>> changing the order of gimplification of MODIFY_EXPR in the back end, as
>> richi was also thinking about doing to fix 71104. This runs into problems
>> with DECL_VALUE_EXPR variables, where is_gimple_reg can be true before
>> gimplification and false afterward, so he checks for this situation in
>> rhs_predicate_for. richi, you said you were still working on 71104; is this
>> patch OK to put in for now, or should I wait for something better?
> I wasn't too happy about the rhs_predicate_for change and I was also worried
> about generating a lot less optimal GIMPLE due to evaluating the predicate
> on un-gimplified *to_p.
We can try to be more clever about recognizing things that will
gimplify to a reg. How does this patch look?
> I wondered if we should simply gimplify *from_p
> with is_gimple_mem_rhs_or_call unconditionally, then gimplify *to_p
> and after that if (unmodified) rhs_predicate_for (*to_p) is !=
> is_gimple_mem_rhs_or_call re-gimplify *from_p to avoid this. That should
> also avoid changing
> rhs_predicate_for.
The problem with this approach is that gimplification is destructive;
you can't just throw away the first sequence and gimplify again. For
instance, SAVE_EXPRs are clobbered the first time they are seen in
gimplification.
> Not sure if that solves whatever you were running into with OpenMP.
>
> I simply didn't have found the time to experiment with the above or even
> validate my fear by say comparing .gimple dumps of cc1 files with/without
> the gimplification order change.
Looking through the gimple dumps for optabs.c and c-common.c with this
patch I don't see any increase in temporaries, but I do see some
improved locality such that we initialize a pointer temporary just
before assigning to one of its fields rather than initializing it
before doing all the value computation, e.g.
before:
- _14 = *node;
- _15 = contains_struct_check (_14, 1, "../../../gcc/gcc/c-family/c-common.
c", 7672, &__FUNCTION__);
...lots...
- _15->typed.type = _56;
after:
+ _55 = *node;
+ _56 = contains_struct_check (_55, 1,
"../../../gcc/gcc/c-family/c-common.c", 7672, &__FUNCTION__);
+ _56->typed.type = _54;
Is this version of the patch OK?
Jason
commit 50495a102be99950002b0cc9f824fcb90cdf65fb
Author: Jason Merrill <ja...@redhat.com>
Date: Thu Jun 16 01:25:02 2016 -0400
P0145R2: Refining Expression Order for C++ (assignment)
* gimplify.c (will_be_gimple_reg): New.
(rhs_predicate_for): Use it.
(gimplify_modify_expr): Gimplify RHS first.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index ae8b4fc..5d51d64 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3802,12 +3802,45 @@ gimplify_init_ctor_eval (tree object,
vec<constructor_elt, va_gc> *elts,
}
}
+/* Return true if LHS will satisfy is_gimple_reg after gimplification. */
+
+static bool
+will_be_gimple_reg (tree lhs)
+{
+ while (true)
+ switch (TREE_CODE (lhs))
+ {
+ case COMPOUND_EXPR:
+ lhs = TREE_OPERAND (lhs, 1);
+ break;
+
+ case INIT_EXPR:
+ case MODIFY_EXPR:
+ case PREINCREMENT_EXPR:
+ case PREDECREMENT_EXPR:
+ lhs = TREE_OPERAND (lhs, 0);
+ break;
+
+ case VAR_DECL:
+ case PARM_DECL:
+ case RESULT_DECL:
+ if (DECL_HAS_VALUE_EXPR_P (lhs))
+ {
+ lhs = DECL_VALUE_EXPR (lhs);
+ break;
+ }
+ /* else fall through. */
+ default:
+ return is_gimple_reg (lhs);
+ }
+}
+
/* Return the appropriate RHS predicate for this LHS. */
gimple_predicate
rhs_predicate_for (tree lhs)
{
- if (is_gimple_reg (lhs))
+ if (will_be_gimple_reg (lhs))
return is_gimple_reg_rhs_or_call;
else
return is_gimple_mem_rhs_or_call;
@@ -4778,10 +4811,6 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p,
that is what we must do here. */
maybe_with_size_expr (from_p);
- ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue);
- if (ret == GS_ERROR)
- return ret;
-
/* As a special case, we have to temporarily allow for assignments
with a CALL_EXPR on the RHS. Since in GIMPLE a function call is
a toplevel statement, when gimplifying the GENERIC expression
@@ -4799,6 +4828,10 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p,
if (ret == GS_ERROR)
return ret;
+ ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue);
+ if (ret == GS_ERROR)
+ return ret;
+
/* In case of va_arg internal fn wrappped in a WITH_SIZE_EXPR, add the type
size as argument to the call. */
if (TREE_CODE (*from_p) == WITH_SIZE_EXPR)
diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
index 15df903..d351219 100644
--- a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
+++ b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
@@ -84,7 +84,7 @@ template <class T> void f()
// b @= a
aref(19)=A(18);
- //iref(21)=f(20);
+ iref(21)=f(20);
aref(23)+=f(22);
last = 0;
@@ -123,7 +123,7 @@ void g()
// b @= a
aref(19)=A(18);
- //iref(21)=f(20);
+ iref(21)=f(20);
aref(23)+=f(22);
last = 0;
