| Message ID | CADzB+2=gp37wxt+BRrGDChvsrYzJYWffxY54YvkmKZyH6cmuRw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jul 7, 2016 at 9:18 PM, Jason Merrill <jason@redhat.com> wrote: > On Tue, Jun 28, 2016 at 10:00 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Thu, Jun 16, 2016 at 6:15 PM, Jakub Jelinek <jakub@redhat.com> wrote: >>> On Thu, Jun 16, 2016 at 11:28:48AM -0400, Jason Merrill wrote: >>>> 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) >>> >>> I wonder if instead of trying to guess early what we'll gimplify into it >>> wouldn't be better to gimplify *from_p twice, first time with a predicate >>> that would assume *to_p could be gimplified into is_gimple_ref, but >>> guarantee there are no side-effects (so that those aren't evaluated >>> after lhs side-effects), and second time if needed (if *to_p didn't end up >>> being is_gimple_reg). So something like a new predicate like: >> >> Yes, that is what I was suggesting. > > How about this? I also have a patch to handle assignment order > entirely in the front end, but my impression has been that you wanted > to make this change for other reasons as well. Yes. Looks good to me. > In other news, I convinced the committee to drop function arguments > from the order of evaluation paper, so we don't have to worry about > that hit on PUSH_ARGS_REVERSED targets. Good. Thanks, Richard. > Jason
On Thu, Jul 07, 2016 at 03:18:13PM -0400, Jason Merrill wrote: > How about this? I also have a patch to handle assignment order > entirely in the front end, but my impression has been that you wanted > to make this change for other reasons as well. So what exactly is supposed to be the evaluation order for function calls with lhs in C++17? Reading http://en.cppreference.com/w/cpp/language/eval_order I'm confused. struct S { S (); ~S (); ... }; S s[1024]; typedef S (*fn) (int, int); fn a[1024]; void foo (int *i, int *j, int *k, int *l) { s[i[0]++] = (a[j[0]++]) (k[0]++, l[0]++); } So, j[0]++ needs to happen first, then k[0]++ and l[0]++ (indeterminately sequenced), but what about the function call vs. i[0]++? There is the rule that for E1 = E2 all side-effects of E2 happen before all side-effects of E1. I mean, if the function return type is a gimple reg type, then I see no problem in honoring that, the function call returns a temporary, then the side-effects of the lhs are evaluated and then it is stored to that lvalue. But, if the return type is non-POD, then we need to pass the address of the lhs as invisible reference to the function call, how can we do it if we can't yet evaluate the side-effects of the lhs? Perhaps better testcase is: int bar (int); void baz () { s[bar (0)] = (a[bar (1)]) (bar (2), 0); } In which order all the 4 calls are made? What the patch you've posted does is that it gimplifies from_p first, and gimplify_call_expr will first evaluate bar (1), then bar (2), but then it is a CALL_EXPR; then it gimplifies the lhs, i.e. bar (0) call, and finally the indirect call. > > In other news, I convinced the committee to drop function arguments > from the order of evaluation paper, so we don't have to worry about > that hit on PUSH_ARGS_REVERSED targets. > > Jason > commit 8dac319f5647d31568ad9278edeff3607aa1b3cc > Author: Jason Merrill <jason@redhat.com> > Date: Sat Jun 25 19:12:42 2016 +0300 > > P0145: Refining Expression Order for C++ (assignment) > > * gimplify.c (initial_rhs_predicate_for): New. > (gimplfy_modify_expr): Gimplify RHS before LHS. > > diff --git a/gcc/gimplify.c b/gcc/gimplify.c > index 47c4d25..0276588 100644 > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -3813,6 +3813,18 @@ rhs_predicate_for (tree lhs) > return is_gimple_mem_rhs_or_call; > } > > +/* Return the initial guess for an appropriate RHS predicate for this LHS, > + before the LHS has been gimplified. */ > + > +static gimple_predicate > +initial_rhs_predicate_for (tree lhs) > +{ > + if (is_gimple_reg_type (TREE_TYPE (lhs))) > + return is_gimple_reg_rhs_or_call; > + else > + return is_gimple_mem_rhs_or_call; > +} > + > /* Gimplify a C99 compound literal expression. This just means adding > the DECL_EXPR before the current statement and using its anonymous > decl instead. */ > @@ -4778,10 +4790,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 > @@ -4794,6 +4802,16 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, > reaches the CALL_EXPR. On return from gimplify_expr, the newly > created GIMPLE_CALL <foo> will be the last statement in *PRE_P > and all we need to do here is set 'a' to be its LHS. */ > + ret = gimplify_expr (from_p, pre_p, post_p, > + initial_rhs_predicate_for (*to_p), fb_rvalue); > + 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; > + > + /* Now that the LHS is gimplified, we know what to use for the RHS. */ > ret = gimplify_expr (from_p, pre_p, post_p, rhs_predicate_for (*to_p), > fb_rvalue); > if (ret == GS_ERROR) Jakub
diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 47c4d25..0276588 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -3813,6 +3813,18 @@ rhs_predicate_for (tree lhs) return is_gimple_mem_rhs_or_call; } +/* Return the initial guess for an appropriate RHS predicate for this LHS, + before the LHS has been gimplified. */ + +static gimple_predicate +initial_rhs_predicate_for (tree lhs) +{ + if (is_gimple_reg_type (TREE_TYPE (lhs))) + return is_gimple_reg_rhs_or_call; + else + return is_gimple_mem_rhs_or_call; +} + /* Gimplify a C99 compound literal expression. This just means adding the DECL_EXPR before the current statement and using its anonymous decl instead. */ @@ -4778,10 +4790,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 @@ -4794,6 +4802,16 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, reaches the CALL_EXPR. On return from gimplify_expr, the newly created GIMPLE_CALL <foo> will be the last statement in *PRE_P and all we need to do here is set 'a' to be its LHS. */ + ret = gimplify_expr (from_p, pre_p, post_p, + initial_rhs_predicate_for (*to_p), fb_rvalue); + 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; + + /* Now that the LHS is gimplified, we know what to use for the RHS. */ ret = gimplify_expr (from_p, pre_p, post_p, rhs_predicate_for (*to_p), fb_rvalue); if (ret == GS_ERROR)