RFA (gimplify): PATCH to implement C++ order of evaluation paper

RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

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?

For the moment the patch turns on full ordering by default so we can see 
what effect it has in routine benchmarking.  This will probably back off 
by the time of the GCC 7 release.

In my own SPEC runs with an earlier version of the patch, most of the 
C++ tests did not change significantly, but xalancbmk slowed down about 
1%.  I'm running it again now with the current patch.

Tested x86_64-pc-linux-gnu, applying second patch to trunk, is the first 
patch OK as well?

Jason

[-- Attachment #2: assign-order.diff --]
[-- Type: text/x-patch, Size: 2197 bytes --]

commit ec26b9bc7d98fb127d0e38b1b935278e54bd62c4
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Jun 10 14:55:54 2016 -0400

    	Gimplify RHS of assignment first.
    
    	* gimplify.c (rhs_predicate_for): Check DECL_HAS_VALUE_EXPR_P.
    	(gimplify_modify_expr): Gimplify RHS first.

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index ae8b4fc..8608569 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3807,7 +3807,9 @@ gimplify_init_ctor_eval (tree object, vec<constructor_elt, va_gc> *elts,
 gimple_predicate
 rhs_predicate_for (tree lhs)
 {
-  if (is_gimple_reg (lhs))
+  if (is_gimple_reg (lhs)
+      && (!DECL_P (lhs)
+	  || !DECL_HAS_VALUE_EXPR_P (lhs)))
     return is_gimple_reg_rhs_or_call;
   else
     return is_gimple_mem_rhs_or_call;
@@ -4778,10 +4780,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 +4797,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;
 

[-- Attachment #3: args-order.diff --]
[-- Type: text/x-patch, Size: 20476 bytes --]

commit 0516df95f13b7d30b17d8412739afc3c8d18df6e
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Jun 6 16:50:48 2016 -0400

    	P0145R2: Refining Expression Order for C++.
    
    gcc/c-family/
    	* c.opt (fargs-in-order): New.
    	* c-opts.c (c_common_post_options): Adjust flag_args_in_order.
    gcc/cp/
    	* cp-tree.h (CALL_EXPR_OPERATOR_SYNTAX, CALL_EXPR_ORDERED_ARGS)
    	(CALL_EXPR_REVERSE_ARGS): New.
    	* call.c (build_new_op_1): Set them.
    	(extract_call_expr, op_is_ordered): New.
    	(build_over_call): Set CALL_EXPR_ORDERED_ARGS.
    	* cp-gimplify.c (cp_gimplify_expr) [CALL_EXPR]: Handle new flags.
    	* pt.c (tsubst_copy_and_build): Copy new flags.
    	* semantics.c (simplify_aggr_init_expr): Likewise.
    	* tree.c (build_aggr_init_expr): Likewise.
    	(build_min_non_dep_op_overload): Likewise.

diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index fec58bc..ff6339c 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -910,6 +910,12 @@ c_common_post_options (const char **pfilename)
   else if (warn_narrowing == -1)
     warn_narrowing = 0;
 
+  /* C++17 requires that function arguments be evaluated left-to-right even on
+     PUSH_ARGS_REVERSED targets.  */
+  if (c_dialect_cxx ()
+      && flag_args_in_order == -1)
+    flag_args_in_order = 2 /*(cxx_dialect >= cxx1z) ? 2 : 0*/;
+
   /* Global sized deallocation is new in C++14.  */
   if (flag_sized_deallocation == -1)
     flag_sized_deallocation = (cxx_dialect >= cxx14);
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 761a83b..83fd84c 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1043,6 +1043,14 @@ falt-external-templates
 C++ ObjC++ Ignore Warn(switch %qs is no longer supported)
 No longer supported.
 
+fargs-in-order
+C++ ObjC++ Alias(fargs-in-order=, 2, 0)
+Always evaluate function arguments in left-to-right order.
+
+fargs-in-order=
+C++ ObjC++ Var(flag_args_in_order) Joined UInteger Init(-1)
+Always evaluate function arguments in left-to-right order.
+
 fasm
 C ObjC C++ ObjC++ Var(flag_no_asm, 0)
 Recognize the \"asm\" keyword.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 729b7eb..e2b89b8 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5372,6 +5372,40 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
     }
 }
 
+/* Returns 1 if P0145R2 says that the LHS of operator CODE is evaluated first,
+   -1 if the RHS is evaluated first, or 0 if the order is unspecified.  */
+
+static int
+op_is_ordered (tree_code code)
+{
+  if (!flag_args_in_order)
+    return 0;
+
+  switch (code)
+    {
+      // 5. b @= a
+    case MODIFY_EXPR:
+      return -1;
+
+      // 1. a.b
+      // Not overloadable (yet).
+      // 2. a->b
+      // Only one argument.
+      // 3. a->*b
+    case MEMBER_REF:
+      // 6. a[b]
+    case ARRAY_REF:
+      // 7. a << b
+    case LSHIFT_EXPR:
+      // 8. a >> b
+    case RSHIFT_EXPR:
+      return 1;
+
+    default:
+      return 0;
+    }
+}
+
 static tree
 build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
 		tree arg2, tree arg3, tree *overload, tsubst_flags_t complain)
@@ -5660,17 +5694,33 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
 	  else
 	    result = build_over_call (cand, LOOKUP_NORMAL, complain);
 
-	  if (processing_template_decl
-	      && result != NULL_TREE
-	      && result != error_mark_node
-	      && DECL_HIDDEN_FRIEND_P (cand->fn))
+	  if (trivial_fn_p (cand->fn))
+	    /* There won't be a CALL_EXPR.  */;
+	  else if (result && result != error_mark_node)
 	    {
-	      tree call = result;
-	      if (REFERENCE_REF_P (call))
-		call = TREE_OPERAND (call, 0);
-	      /* This prevents build_new_function_call from discarding this
-		 function during instantiation of the enclosing template.  */
-	      KOENIG_LOOKUP_P (call) = 1;
+	      tree call = extract_call_expr (result);
+	      CALL_EXPR_OPERATOR_SYNTAX (call) = true;
+
+	      if (processing_template_decl && DECL_HIDDEN_FRIEND_P (cand->fn))
+		/* This prevents build_new_function_call from discarding this
+		   function during instantiation of the enclosing template.  */
+		KOENIG_LOOKUP_P (call) = 1;
+
+	      /* Specify evaluation order as per P0145R2.  */
+	      CALL_EXPR_ORDERED_ARGS (call) = false;
+	      switch (op_is_ordered (code))
+		{
+		case -1:
+		  CALL_EXPR_REVERSE_ARGS (call) = true;
+		  break;
+
+		case 1:
+		  CALL_EXPR_ORDERED_ARGS (call) = true;
+		  break;
+
+		default:
+		  break;
+		}
 	    }
 	}
       else
@@ -5846,6 +5896,25 @@ build_new_op (location_t loc, enum tree_code code, int flags,
   return ret;
 }
 
+/* CALL was returned by some call-building function; extract the actual
+   CALL_EXPR from any bits that have been tacked on, e.g. by
+   convert_from_reference.  */
+
+tree
+extract_call_expr (tree call)
+{
+  while (TREE_CODE (call) == COMPOUND_EXPR)
+    call = TREE_OPERAND (call, 1);
+  if (REFERENCE_REF_P (call))
+    call = TREE_OPERAND (call, 0);
+  if (TREE_CODE (call) == TARGET_EXPR)
+    call = TARGET_EXPR_INITIAL (call);
+  gcc_assert (TREE_CODE (call) == CALL_EXPR
+	      || TREE_CODE (call) == AGGR_INIT_EXPR
+	      || call == error_mark_node);
+  return call;
+}
+
 /* Returns true if FN has two parameters, of which the second has type
    size_t.  */
 
@@ -7533,10 +7602,10 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
     }
 
   /* Ellipsis */
+  int magic = magic_varargs_p (fn);
   for (; arg_index < vec_safe_length (args); ++arg_index)
     {
       tree a = (*args)[arg_index];
-      int magic = magic_varargs_p (fn);
       if (magic == 2)
 	{
 	  /* Do no conversions for certain magic varargs.  */
@@ -7666,9 +7735,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
       if (is_really_empty_class (type))
 	{
 	  /* Avoid copying empty classes.  */
-	  val = build2 (COMPOUND_EXPR, void_type_node, to, arg);
-	  TREE_NO_WARNING (val) = 1;
-	  val = build2 (COMPOUND_EXPR, type, val, to);
+	  val = build2 (COMPOUND_EXPR, type, arg, to);
 	  TREE_NO_WARNING (val) = 1;
 	}
       else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base)))
@@ -7756,9 +7823,15 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
     }
 
   tree call = build_cxx_call (fn, nargs, argarray, complain|decltype_flag);
-  if (TREE_CODE (call) == CALL_EXPR
-      && (cand->flags & LOOKUP_LIST_INIT_CTOR))
-    CALL_EXPR_LIST_INIT_P (call) = true;
+  if (call != error_mark_node
+      && !magic
+      && (flag_args_in_order > 1
+	  || (cand->flags & LOOKUP_LIST_INIT_CTOR)))
+    {
+      tree c = extract_call_expr (call);
+      /* build_new_op_1 will clear this when appropriate.  */
+      CALL_EXPR_ORDERED_ARGS (c) = true;
+    }
   return call;
 }
 
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index dcb0fa6..97b043a 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -565,6 +565,7 @@ int
 cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 {
   int saved_stmts_are_full_exprs_p = 0;
+  location_t loc = EXPR_LOC_OR_LOC (*expr_p, input_location);
   enum tree_code code = TREE_CODE (*expr_p);
   enum gimplify_status ret;
 
@@ -752,18 +753,26 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	  cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
 	  return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
 	}
-      /* DR 1030 says that we need to evaluate the elements of an
-	 initializer-list in forward order even when it's used as arguments to
-	 a constructor.  So if the target wants to evaluate them in reverse
-	 order and there's more than one argument other than 'this', gimplify
-	 them in order.  */
       ret = GS_OK;
-      if (PUSH_ARGS_REVERSED && CALL_EXPR_LIST_INIT_P (*expr_p)
-	  && call_expr_nargs (*expr_p) > 2)
+      if (!CALL_EXPR_FN (*expr_p))
+	/* Internal function call.  */;
+      else if (CALL_EXPR_REVERSE_ARGS (*expr_p))
 	{
-	  int nargs = call_expr_nargs (*expr_p);
-	  location_t loc = EXPR_LOC_OR_LOC (*expr_p, input_location);
-	  for (int i = 1; i < nargs; ++i)
+	  /* This is a call to a (compound) assignment operator that used
+	     the operator syntax; gimplify the RHS first.  */
+	  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);
+	  if (t == GS_ERROR)
+	    ret = GS_ERROR;
+	}
+      else if (CALL_EXPR_ORDERED_ARGS (*expr_p))
+	{
+	  /* Leave the last argument for gimplify_call_expr, to avoid problems
+	     with __builtin_va_arg_pack().  */
+	  int nargs = call_expr_nargs (*expr_p) - 1;
+	  for (int i = 0; i < nargs; ++i)
 	    {
 	      enum gimplify_status t
 		= gimplify_arg (&CALL_EXPR_ARG (*expr_p, i), pre_p, loc);
@@ -771,6 +780,22 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 		ret = GS_ERROR;
 	    }
 	}
+      else if (flag_args_in_order == 1
+	       && !CALL_EXPR_OPERATOR_SYNTAX (*expr_p))
+	{
+	  /* If flag_args_in_order == 1, we don't force an order on all
+	     function arguments, but do evaluate the object argument first.  */
+	  tree fntype = TREE_TYPE (CALL_EXPR_FN (*expr_p));
+	  if (POINTER_TYPE_P (fntype))
+	    fntype = TREE_TYPE (fntype);
+	  if (TREE_CODE (fntype) == METHOD_TYPE)
+	    {
+	      enum gimplify_status t
+		= gimplify_arg (&CALL_EXPR_ARG (*expr_p, 0), pre_p, loc);
+	      if (t == GS_ERROR)
+		ret = GS_ERROR;
+	    }
+	}
       break;
 
     case RETURN_EXPR:
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 14ba120..6c6ad10 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -179,19 +179,21 @@ operator == (const cp_expr &lhs, tree rhs)
       IDENTIFIER_CTOR_OR_DTOR_P (in IDENTIFIER_NODE)
       BIND_EXPR_BODY_BLOCK (in BIND_EXPR)
       DECL_NON_TRIVIALLY_INITIALIZED_P (in VAR_DECL)
-      CALL_EXPR_LIST_INIT_P (in CALL_EXPR, AGGR_INIT_EXPR)
+      CALL_EXPR_ORDERED_ARGS (in CALL_EXPR, AGGR_INIT_EXPR)
    4: TREE_HAS_CONSTRUCTOR (in INDIRECT_REF, SAVE_EXPR, CONSTRUCTOR,
-	  or FIELD_DECL).
+	  CALL_EXPR, or FIELD_DECL).
       IDENTIFIER_TYPENAME_P (in IDENTIFIER_NODE)
       DECL_TINFO_P (in VAR_DECL)
       FUNCTION_REF_QUALIFIED (in FUNCTION_TYPE, METHOD_TYPE)
    5: C_IS_RESERVED_WORD (in IDENTIFIER_NODE)
       DECL_VTABLE_OR_VTT_P (in VAR_DECL)
       FUNCTION_RVALUE_QUALIFIED (in FUNCTION_TYPE, METHOD_TYPE)
+      CALL_EXPR_REVERSE_ARGS (in CALL_EXPR, AGGR_INIT_EXPR)
    6: IDENTIFIER_REPO_CHOSEN (in IDENTIFIER_NODE)
       DECL_CONSTRUCTION_VTABLE_P (in VAR_DECL)
       TYPE_MARKED_P (in _TYPE)
       RANGE_FOR_IVDEP (in RANGE_FOR_STMT)
+      CALL_EXPR_OPERATOR_SYNTAX (in CALL_EXPR, AGGR_INIT_EXPR)
 
    Usage of TYPE_LANG_FLAG_?:
    0: TYPE_DEPENDENT_P
@@ -3379,6 +3381,9 @@ extern void decl_shadowed_for_var_insert (tree, tree);
 #define DELETE_EXPR_USE_VEC(NODE) \
   TREE_LANG_FLAG_1 (DELETE_EXPR_CHECK (NODE))
 
+#define CALL_OR_AGGR_INIT_CHECK(NODE) \
+  TREE_CHECK2 ((NODE), CALL_EXPR, AGGR_INIT_EXPR)
+
 /* Indicates that this is a non-dependent COMPOUND_EXPR which will
    resolve to a function call.  */
 #define COMPOUND_EXPR_OVERLOADED(NODE) \
@@ -3388,9 +3393,20 @@ extern void decl_shadowed_for_var_insert (tree, tree);
    should be performed at instantiation time.  */
 #define KOENIG_LOOKUP_P(NODE) TREE_LANG_FLAG_0 (CALL_EXPR_CHECK (NODE))
 
-/* True if CALL_EXPR expresses list-initialization of an object.  */
-#define CALL_EXPR_LIST_INIT_P(NODE) \
-  TREE_LANG_FLAG_3 (TREE_CHECK2 ((NODE),CALL_EXPR,AGGR_INIT_EXPR))
+/* True if the arguments to NODE should be evaluated in left-to-right
+   order regardless of PUSH_ARGS_REVERSED.  */
+#define CALL_EXPR_ORDERED_ARGS(NODE) \
+  TREE_LANG_FLAG_3 (CALL_OR_AGGR_INIT_CHECK (NODE))
+
+/* True if the arguments to NODE should be evaluated in right-to-left
+   order regardless of PUSH_ARGS_REVERSED.  */
+#define CALL_EXPR_REVERSE_ARGS(NODE) \
+  TREE_LANG_FLAG_5 (CALL_OR_AGGR_INIT_CHECK (NODE))
+
+/* True if CALL_EXPR was written as an operator expression, not a function
+   call.  */
+#define CALL_EXPR_OPERATOR_SYNTAX(NODE) \
+  TREE_LANG_FLAG_6 (CALL_OR_AGGR_INIT_CHECK (NODE))
 
 /* Indicates whether a string literal has been parenthesized. Such
    usages are disallowed in certain circumstances.  */
@@ -5542,6 +5558,7 @@ extern bool null_ptr_cst_p			(tree);
 extern bool null_member_pointer_value_p		(tree);
 extern bool sufficient_parms_p			(const_tree);
 extern tree type_decays_to			(tree);
+extern tree extract_call_expr			(tree);
 extern tree build_user_type_conversion		(tree, tree, int,
 						 tsubst_flags_t);
 extern tree build_new_function_call		(tree, vec<tree, va_gc> **, bool, 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3a3d9b8..11b5d82 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -16652,6 +16652,20 @@ tsubst_copy_and_build (tree t,
 
 	release_tree_vector (call_args);
 
+	if (ret != error_mark_node)
+	  {
+	    bool op = CALL_EXPR_OPERATOR_SYNTAX (t);
+	    bool ord = CALL_EXPR_ORDERED_ARGS (t);
+	    bool rev = CALL_EXPR_REVERSE_ARGS (t);
+	    if (op || ord || rev)
+	      {
+		function = extract_call_expr (ret);
+		CALL_EXPR_OPERATOR_SYNTAX (function) = op;
+		CALL_EXPR_ORDERED_ARGS (function) = ord;
+		CALL_EXPR_REVERSE_ARGS (function) = rev;
+	      }
+	  }
+
 	RETURN (ret);
       }
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 5365091..9b0cff8 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -4057,8 +4057,11 @@ simplify_aggr_init_expr (tree *tp)
 				    aggr_init_expr_nargs (aggr_init_expr),
 				    AGGR_INIT_EXPR_ARGP (aggr_init_expr));
   TREE_NOTHROW (call_expr) = TREE_NOTHROW (aggr_init_expr);
-  CALL_EXPR_LIST_INIT_P (call_expr) = CALL_EXPR_LIST_INIT_P (aggr_init_expr);
   CALL_FROM_THUNK_P (call_expr) = AGGR_INIT_FROM_THUNK_P (aggr_init_expr);
+  CALL_EXPR_OPERATOR_SYNTAX (call_expr)
+    = CALL_EXPR_OPERATOR_SYNTAX (aggr_init_expr);
+  CALL_EXPR_ORDERED_ARGS (call_expr) = CALL_EXPR_ORDERED_ARGS (aggr_init_expr);
+  CALL_EXPR_REVERSE_ARGS (call_expr) = CALL_EXPR_REVERSE_ARGS (aggr_init_expr);
 
   if (style == ctor)
     {
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index abda6e4..9ab964d 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -524,7 +524,9 @@ build_aggr_init_expr (tree type, tree init)
       TREE_SIDE_EFFECTS (rval) = 1;
       AGGR_INIT_VIA_CTOR_P (rval) = is_ctor;
       TREE_NOTHROW (rval) = TREE_NOTHROW (init);
-      CALL_EXPR_LIST_INIT_P (rval) = CALL_EXPR_LIST_INIT_P (init);
+      CALL_EXPR_OPERATOR_SYNTAX (rval) = CALL_EXPR_OPERATOR_SYNTAX (init);
+      CALL_EXPR_ORDERED_ARGS (rval) = CALL_EXPR_ORDERED_ARGS (init);
+      CALL_EXPR_REVERSE_ARGS (rval) = CALL_EXPR_REVERSE_ARGS (init);
     }
   else
     rval = init;
@@ -2854,8 +2856,7 @@ build_min_non_dep_op_overload (enum tree_code op,
   tree fn, call;
   vec<tree, va_gc> *args;
 
-  if (REFERENCE_REF_P (non_dep))
-    non_dep = TREE_OPERAND (non_dep, 0);
+  non_dep = extract_call_expr (non_dep);
 
   nargs = call_expr_nargs (non_dep);
 
@@ -2897,10 +2898,11 @@ build_min_non_dep_op_overload (enum tree_code op,
   call = build_min_non_dep_call_vec (non_dep, fn, args);
   release_tree_vector (args);
 
-  tree call_expr = call;
-  if (REFERENCE_REF_P (call_expr))
-    call_expr = TREE_OPERAND (call_expr, 0);
+  tree call_expr = extract_call_expr (call);
   KOENIG_LOOKUP_P (call_expr) = KOENIG_LOOKUP_P (non_dep);
+  CALL_EXPR_OPERATOR_SYNTAX (call_expr) = true;
+  CALL_EXPR_ORDERED_ARGS (call_expr) = CALL_EXPR_ORDERED_ARGS (non_dep);
+  CALL_EXPR_REVERSE_ARGS (call_expr) = CALL_EXPR_REVERSE_ARGS (non_dep);
 
   return call;
 }
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 223fd86..2105351 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -189,7 +189,8 @@ in the following sections.
 
 @item C++ Language Options
 @xref{C++ Dialect Options,,Options Controlling C++ Dialect}.
-@gccoptlist{-fabi-version=@var{n}  -fno-access-control  -fcheck-new @gol
+@gccoptlist{-fabi-version=@var{n}  -fno-access-control @gol
+-fargs-in-order=@var{n} -fcheck-new @gol
 -fconstexpr-depth=@var{n}  -ffriend-injection @gol
 -fno-elide-constructors @gol
 -fno-enforce-eh-specs @gol
@@ -2233,6 +2234,14 @@ option is used for the warning.
 Turn off all access checking.  This switch is mainly useful for working
 around bugs in the access control code.
 
+@item -fargs-in-order
+@opindex fargs-in-order
+Evaluate function arguments and operands of some binary expressions in
+left-to-right order, and evaluate the right side of an assignment
+before the left side, as proposed in P0145R2.  Enabled by default with
+@option{-std=c++1z}.  @option{-fargs-in-order=1} implements all of the
+ordering requirements except function arguments.
+
 @item -fcheck-new
 @opindex fcheck-new
 Check that the pointer returned by @code{operator new} is non-null
diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order1.C b/gcc/testsuite/g++.dg/cpp1z/eval-order1.C
new file mode 100644
index 0000000..278990d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/eval-order1.C
@@ -0,0 +1,21 @@
+// P0145R2: Refining Expression Order for C++
+// { dg-do run }
+// { dg-options "-std=c++1z" }
+
+extern "C" int printf (const char *, ...);
+void sink(...) { }
+
+int last = 0;
+int f(int i)
+{
+  if (i < last)
+    __builtin_abort ();
+  last = i;
+  return i;
+}
+
+int main()
+{
+  sink(f(1), f(2));
+  sink(f(3), f(4), f(5));
+}
diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order2.C b/gcc/testsuite/g++.dg/cpp1z/eval-order2.C
new file mode 100644
index 0000000..2a741d6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/eval-order2.C
@@ -0,0 +1,15 @@
+// P0145R2: Refining Expression Order for C++
+// { dg-do run }
+// { dg-options "-std=c++1z" }
+
+#include <string>
+#define assert(X) if (!(X)) __builtin_abort();
+
+int main()
+{
+  std::string s = "but I have heard it works even if you don't believe in it" ;
+  s.replace(0, 4, "" ).replace( s.find( "even" ), 4, "only" )
+    .replace( s.find( " don't" ), 6, "" );
+
+  assert( s == "I have heard it works only if you believe in it" ) ;
+}
diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
new file mode 100644
index 0000000..15df903
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
@@ -0,0 +1,150 @@
+// P0145R2: Refining Expression Order for C++
+// { dg-do run }
+// { dg-options "-std=c++1z -fargs-in-order=1" }
+
+extern "C" int printf (const char *, ...);
+void sink(...) { }
+
+int last = 0;
+int f(int i)
+{
+  if (i < last)
+    __builtin_abort ();
+  last = i;
+  return i;
+}
+
+int& g(int i)
+{
+  static int dummy;
+  f(i);
+  return dummy;
+}
+
+struct A
+{
+  int _i;
+  A(int i): _i(f(i)) { }
+  A& memfn(int i, int j) { f(j); return *this; }
+  int operator<<(int i) { }
+  A& operator=(const A&) { return *this; }
+  A& operator+=(int i) { return *this; }
+};
+
+int operator>>(A&, int i) { }
+
+A a(0);
+A* afn(int i)
+{
+  f(i);
+  return &a;
+}
+
+A& aref(int i)
+{
+  f(i);
+  return a;
+}
+
+static int si;
+int* ip (int i)
+{
+  f(i);
+  return &si;
+}
+
+int& iref(int i)
+{
+  f(i);
+  return si;
+}
+
+auto pmff(int i) {
+  f(i);
+  return &A::memfn;
+}
+
+template <class T> void f()
+{
+  // a.b
+  A(1).memfn(f(2),3).memfn(f(4),5);
+  aref(6).memfn(f(7),8);
+  (aref(9).*pmff(10))(f(11),12);
+  last = 0;
+
+  // a->b
+  afn(12)->memfn(f(13),14);
+
+  // a->*b
+  (afn(15)->*pmff(16))(f(17),18);
+  last = 0;
+
+  // a(b)
+  // covered in eval-order1.C
+
+  // b @= a
+  aref(19)=A(18);
+  //iref(21)=f(20);
+  aref(23)+=f(22);
+  last = 0;
+
+  // a[b]
+  afn(20)[f(21)-21].memfn(f(22),23);
+  ip(24)[f(25)-25] = 0;
+  last=0;
+
+  // a << b
+  aref(24) << f(25);
+  iref(26) << f(27);
+  last=0;
+
+  // a >> b
+  aref(26) >> f(27);
+  iref(28) >> f(29);
+}
+
+void g()
+{
+  // a.b
+  A(1).memfn(f(2),3).memfn(f(4),5);
+  aref(6).memfn(f(7),8);
+  (aref(9).*pmff(10))(f(11),12);
+  last = 0;
+
+  // a->b
+  afn(12)->memfn(f(13),14);
+
+  // a->*b
+  (afn(15)->*pmff(16))(f(17),18);
+  last = 0;
+
+  // a(b)
+  // covered in eval-order1.C
+
+  // b @= a
+  aref(19)=A(18);
+  //iref(21)=f(20);
+  aref(23)+=f(22);
+  last = 0;
+
+  // a[b]
+  afn(20)[f(21)-21].memfn(f(22),23);
+  ip(24)[f(25)-25] = 0;
+  last=0;
+
+  // a << b
+  aref(24) << f(25);
+  iref(26) << f(27);
+  last=0;
+
+  // a >> b
+  aref(26) >> f(27);
+  iref(28) >> f(29);
+}
+
+int main()
+{
+  g();
+  last = 0;
+  f<int>();
+}

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Richard Biener]

On Tue, Jun 14, 2016 at 10:15 PM, Jason Merrill <jason@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.  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.

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.

Richard.

> For the moment the patch turns on full ordering by default so we can see
> what effect it has in routine benchmarking.  This will probably back off by
> the time of the GCC 7 release.
>
> In my own SPEC runs with an earlier version of the patch, most of the C++
> tests did not change significantly, but xalancbmk slowed down about 1%.  I'm
> running it again now with the current patch.
>
> Tested x86_64-pc-linux-gnu, applying second patch to trunk, is the first
> patch OK as well?
>
> Jason

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 2881 bytes --]

On Wed, Jun 15, 2016 at 6:30 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Jun 14, 2016 at 10:15 PM, Jason Merrill <jason@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

[-- Attachment #2: assign-order.diff --]
[-- Type: text/plain, Size: 2902 bytes --]

commit 50495a102be99950002b0cc9f824fcb90cdf65fb
Author: Jason Merrill <jason@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;
 

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jakub Jelinek]

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:

static bool
is_whatever (tree t)
{
  /* For calls, as there are side-effects, assume lhs might not be
     is_gimple_reg.  */
  if (TREE_CODE (t) == CALL_EXPR && is_gimple_reg_type (TREE_TYPE (t)))
    return is_gimple_val (t);
  /* For other side effects, also make sure those are evaluated before
     side-effects in lhs.  */
  if (TREE_THIS_VOLATILE (t))
    return is_gimple_mem_rhs_or_call (t);
  /* Otherwise, optimistically assume lhs will be is_gimple_reg.  */
  return is_gimple_reg_rhs_or_call (t);
}

and then do in gimplify_modify_expr:
  ret = gimplify_expr (from_p, pre_p, post_p,
		       is_gimple_reg (*to_p)
		       ? is_gimple_reg_rhs_or_call : is_whatever,
                       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;

  if (!is_gimple_reg (*to_p) && !is_gimple_mem_rhs_or_call (*from_p))
    {
      ret = gimplify_expr (from_p, pre_p, post_p, is_gimple_mem_rhs_or_call,
			   fb_rvalue);
      if (ret == GS_ERROR)
	return ret;
    }

Or if you want to guess if *to_p will be is_gimple_reg or not after
gimplification, do it just very conservatively and let the more difficult
to predict cases handled worst case by forcing something into a temporary
with the above code.

	Jakub

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jakub Jelinek]

On Thu, Jun 16, 2016 at 06:15:34PM +0200, Jakub Jelinek wrote:
> and then do in gimplify_modify_expr:
>   ret = gimplify_expr (from_p, pre_p, post_p,
> 		       is_gimple_reg (*to_p)

^^^ of course even this is a prediction and wrong one for
DECL_HAS_VALUE_EXPR_Ps.  Conservative would be is_whatever always.

> 		       ? is_gimple_reg_rhs_or_call : is_whatever,
>                        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;
> 
>   if (!is_gimple_reg (*to_p) && !is_gimple_mem_rhs_or_call (*from_p))
>     {
>       ret = gimplify_expr (from_p, pre_p, post_p, is_gimple_mem_rhs_or_call,
> 			   fb_rvalue);
>       if (ret == GS_ERROR)
> 	return ret;
>     }
> 
> Or if you want to guess if *to_p will be is_gimple_reg or not after
> gimplification, do it just very conservatively and let the more difficult
> to predict cases handled worst case by forcing something into a temporary
> with the above code.

	Jakub

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Richard Biener]

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.

Richard.

> static bool
> is_whatever (tree t)
> {
>   /* For calls, as there are side-effects, assume lhs might not be
>      is_gimple_reg.  */
>   if (TREE_CODE (t) == CALL_EXPR && is_gimple_reg_type (TREE_TYPE (t)))
>     return is_gimple_val (t);
>   /* For other side effects, also make sure those are evaluated before
>      side-effects in lhs.  */
>   if (TREE_THIS_VOLATILE (t))
>     return is_gimple_mem_rhs_or_call (t);
>   /* Otherwise, optimistically assume lhs will be is_gimple_reg.  */
>   return is_gimple_reg_rhs_or_call (t);
> }
>
> and then do in gimplify_modify_expr:
>   ret = gimplify_expr (from_p, pre_p, post_p,
>                        is_gimple_reg (*to_p)
>                        ? is_gimple_reg_rhs_or_call : is_whatever,
>                        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;
>
>   if (!is_gimple_reg (*to_p) && !is_gimple_mem_rhs_or_call (*from_p))
>     {
>       ret = gimplify_expr (from_p, pre_p, post_p, is_gimple_mem_rhs_or_call,
>                            fb_rvalue);
>       if (ret == GS_ERROR)
>         return ret;
>     }
>
> Or if you want to guess if *to_p will be is_gimple_reg or not after
> gimplification, do it just very conservatively and let the more difficult
> to predict cases handled worst case by forcing something into a temporary
> with the above code.
>
>         Jakub

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

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.

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

[-- Attachment #2: gimplify-modify.diff --]
[-- Type: text/plain, Size: 2262 bytes --]

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)

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Richard Biener]

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

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jakub Jelinek]

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

Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 2075 bytes --]

On Fri, Jul 8, 2016 at 9:42 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 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.

As we discussed in IRC, to get the required semantics the front-end
needs to prevent this gimplifier optimization, as in this patch.  The
second patch changes -fargs-in-order to -fstrong-eval-order and
removes the ordering of function arguments.

Tested x86_64-pc-linux-gnu, applying to trunk.

[-- Attachment #2: fe-gimplify-modify.diff --]
[-- Type: text/plain, Size: 5427 bytes --]

commit 6f19d771aa9d1afde0e56aca5b69235fb19d1daa
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jul 7 14:30:43 2016 -0400

    	P0145R2: Refining Expression Order for C++ (assignment 2).
    
    	* cp-gimplify.c (lvalue_has_side_effects): New.
    	(cp_gimplify_expr): Implement assignment ordering.

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index c04368f..8496d7c 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -559,6 +559,33 @@ simple_empty_class_p (tree type, tree op)
     && is_really_empty_class (type);
 }
 
+/* Returns true if evaluating E as an lvalue has side-effects;
+   specifically, a volatile lvalue has TREE_SIDE_EFFECTS, but it doesn't really
+   have side-effects until there is a read or write through it.  */
+
+static bool
+lvalue_has_side_effects (tree e)
+{
+  if (!TREE_SIDE_EFFECTS (e))
+    return false;
+  while (handled_component_p (e))
+    {
+      if (TREE_CODE (e) == ARRAY_REF
+	  && TREE_SIDE_EFFECTS (TREE_OPERAND (e, 1)))
+	return true;
+      e = TREE_OPERAND (e, 0);
+    }
+  if (DECL_P (e))
+    /* Just naming a variable has no side-effects.  */
+    return false;
+  else if (INDIRECT_REF_P (e))
+    /* Similarly, indirection has no side-effects.  */
+    return TREE_SIDE_EFFECTS (TREE_OPERAND (e, 0));
+  else
+    /* For anything else, trust TREE_SIDE_EFFECTS.  */
+    return TREE_SIDE_EFFECTS (e);
+}
+
 /* Do C++-specific gimplification.  Args are as for gimplify_expr.  */
 
 int
@@ -659,8 +686,6 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	    /* Remove any copies of empty classes.  Also drop volatile
 	       variables on the RHS to avoid infinite recursion from
 	       gimplify_expr trying to load the value.  */
-	    gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
-			   is_gimple_lvalue, fb_lvalue);
 	    if (TREE_SIDE_EFFECTS (op1))
 	      {
 		if (TREE_THIS_VOLATILE (op1)
@@ -669,8 +694,29 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 
 		gimplify_and_add (op1, pre_p);
 	      }
+	    gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
+			   is_gimple_lvalue, fb_lvalue);
 	    *expr_p = TREE_OPERAND (*expr_p, 0);
 	  }
+	/* P0145 says that the RHS is sequenced before the LHS.
+	   gimplify_modify_expr gimplifies the RHS before the LHS, but that
+	   isn't quite strong enough in two cases:
+
+	   1) gimplify.c wants to leave a CALL_EXPR on the RHS, which would
+	   mean it's evaluated after the LHS.
+
+	   2) the value calculation of the RHS is also sequenced before the
+	   LHS, so for scalar assignment we need to preevaluate if the
+	   RHS could be affected by LHS side-effects even if it has no
+	   side-effects of its own.  We don't need this for classes because
+	   class assignment takes its RHS by reference.  */
+       else if (flag_strong_eval_order > 1
+                && TREE_CODE (*expr_p) == MODIFY_EXPR
+                && lvalue_has_side_effects (op0)
+		&& (TREE_CODE (op1) == CALL_EXPR
+		    || (SCALAR_TYPE_P (TREE_TYPE (op1))
+			&& !TREE_CONSTANT (op1))))
+	 TREE_OPERAND (*expr_p, 1) = get_formal_tmp_var (op1, pre_p);
       }
       ret = GS_OK;
       break;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 65a6885..9544853 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4080,6 +4080,13 @@ diagnosed by this option, and it may give an occasional false positive
 result, but in general it has been found fairly effective at detecting
 this sort of problem in programs.
 
+The C++17 standard will define the order of evaluation of operands in
+more cases: in particular it requires that the right-hand side of an
+assignment be evaluated before the left-hand side, so the above
+examples are no longer undefined.  But this warning will still warn
+about them, to help people avoid writing code that is undefined in C
+and earlier revisions of C++.
+
 The standard is worded confusingly, therefore there is some debate
 over the precise meaning of the sequence point rules in subtle cases.
 Links to discussions of the problem, including proposed formal
diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
index d382d07..e87dce4 100644
--- a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
+++ b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
@@ -31,6 +31,13 @@ struct A
   A& operator+=(int i) { return *this; }
 };
 
+struct B
+{
+  int _i;
+  B(): _i(0) {}
+  B(int i): _i(f(i)) { }
+};
+
 int operator>>(A&, int i) { }
 
 A a(0);
@@ -46,6 +53,18 @@ A& aref(int i)
   return a;
 }
 
+B b;
+B bval(int i)
+{
+  return B(i);
+}
+
+B& bref(int i)
+{
+  f(i);
+  return b;
+}
+
 static int si;
 int* ip (int i)
 {
@@ -84,10 +103,18 @@ template <class T> void f()
 
   // b @= a
   aref(19)=A(18);
-  //iref(21)=f(20);
+  iref(21)=f(20);
   aref(23)+=f(22);
+  bref(25)=bval(24);
+  (f(27), b) = bval(26);
   last = 0;
 
+  int ar[4] = {};
+  int i = 0;
+  ar[i++] = i;
+  if (ar[0] != 0)
+    __builtin_abort();
+
   // a[b]
   afn(20)[f(21)-21].memfn(f(22),23);
   ip(24)[f(25)-25] = 0;
@@ -123,10 +150,18 @@ void g()
 
   // b @= a
   aref(19)=A(18);
-  //iref(21)=f(20);
+  iref(21)=f(20);
   aref(23)+=f(22);
+  bref(25)=bval(24);
+  (f(27), b) = bval(26);
   last = 0;
 
+  int ar[4] = {};
+  int i = 0;
+  ar[i++] = i;
+  if (ar[0] != 0)
+    __builtin_abort();
+
   // a[b]
   afn(20)[f(21)-21].memfn(f(22),23);
   ip(24)[f(25)-25] = 0;

[-- Attachment #3: strong-eval-order.diff --]
[-- Type: text/plain, Size: 7634 bytes --]

commit 6f4916044fa7c0f94a68b8895ff20c5cee41796f
Author: Jason Merrill <jason@redhat.com>
Date:   Sat Jun 25 19:09:42 2016 +0300

    	P0145: Refining Expression Order for C++ (-fstrong-eval-order).
    
    gcc/c-family/
    	* c.opts (-fargs-in-order): Rename to -fstrong-eval-order.
    	* c-opts.c: Adjust.
    gcc/cp/
    	* call.c (op_is_ordered, build_over_call): Adjust for
    	-fargs-in-order renaming to -fstrong-eval-order.
    	* cp-gimplify.c (cp_gimplify_expr): Likewise.

diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index ff6339c..d945825 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -910,11 +910,11 @@ c_common_post_options (const char **pfilename)
   else if (warn_narrowing == -1)
     warn_narrowing = 0;
 
-  /* C++17 requires that function arguments be evaluated left-to-right even on
-     PUSH_ARGS_REVERSED targets.  */
+  /* C++17 has stricter evaluation order requirements; let's use some of them
+     for earlier C++ as well, so chaining works as expected.  */
   if (c_dialect_cxx ()
-      && flag_args_in_order == -1)
-    flag_args_in_order = 2 /*(cxx_dialect >= cxx1z) ? 2 : 0*/;
+      && flag_strong_eval_order == -1)
+    flag_strong_eval_order = (cxx_dialect >= cxx1z ? 2 : 1);
 
   /* Global sized deallocation is new in C++14.  */
   if (flag_sized_deallocation == -1)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 83fd84c..8c70152 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1043,14 +1043,6 @@ falt-external-templates
 C++ ObjC++ Ignore Warn(switch %qs is no longer supported)
 No longer supported.
 
-fargs-in-order
-C++ ObjC++ Alias(fargs-in-order=, 2, 0)
-Always evaluate function arguments in left-to-right order.
-
-fargs-in-order=
-C++ ObjC++ Var(flag_args_in_order) Joined UInteger Init(-1)
-Always evaluate function arguments in left-to-right order.
-
 fasm
 C ObjC C++ ObjC++ Var(flag_no_asm, 0)
 Recognize the \"asm\" keyword.
@@ -1518,6 +1510,28 @@ Assume that values of enumeration type are always within the minimum range of th
 fstrict-prototype
 C++ ObjC++ Ignore Warn(switch %qs is no longer supported)
 
+fstrong-eval-order
+C++ ObjC++ Common Alias(fstrong-eval-order=, all, none)
+Follow the C++17 evaluation order requirements for assignment expressions,
+shift, member function calls, etc.
+
+fstrong-eval-order=
+C++ ObjC++ Common Var(flag_strong_eval_order) Joined Enum(strong_eval_order) Init(-1)
+Follow the C++17 evaluation order requirements for assignment expressions,
+shift, member function calls, etc.
+
+Enum
+Name(strong_eval_order) Type(int)
+
+EnumValue
+Enum(strong_eval_order) String(none) Value(0)
+
+EnumValue
+Enum(strong_eval_order) String(some) Value(1)
+
+EnumValue
+Enum(strong_eval_order) String(all) Value(2)
+
 ftabstop=
 C ObjC C++ ObjC++ Joined RejectNegative UInteger
 -ftabstop=<number>	Distance between tab stops for column reporting.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index d77092b..8b93c61 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5378,14 +5378,15 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 static int
 op_is_ordered (tree_code code)
 {
-  if (!flag_args_in_order)
-    return 0;
-
   switch (code)
     {
       // 5. b @= a
     case MODIFY_EXPR:
-      return -1;
+      return (flag_strong_eval_order > 1 ? -1 : 0);
+
+      // 6. a[b]
+    case ARRAY_REF:
+      return (flag_strong_eval_order > 1 ? 1 : 0);
 
       // 1. a.b
       // Not overloadable (yet).
@@ -5393,13 +5394,11 @@ op_is_ordered (tree_code code)
       // Only one argument.
       // 3. a->*b
     case MEMBER_REF:
-      // 6. a[b]
-    case ARRAY_REF:
       // 7. a << b
     case LSHIFT_EXPR:
       // 8. a >> b
     case RSHIFT_EXPR:
-      return 1;
+      return (flag_strong_eval_order ? 1 : 0);
 
     default:
       return 0;
@@ -7830,9 +7829,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 
   tree call = build_cxx_call (fn, nargs, argarray, complain|decltype_flag);
   if (call != error_mark_node
-      && !magic
-      && (flag_args_in_order > 1
-	  || (cand->flags & LOOKUP_LIST_INIT_CTOR)))
+      && cand->flags & LOOKUP_LIST_INIT_CTOR)
     {
       tree c = extract_call_expr (call);
       /* build_new_op_1 will clear this when appropriate.  */
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 1d81fb1..c04368f 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -780,11 +780,10 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 		ret = GS_ERROR;
 	    }
 	}
-      else if (flag_args_in_order == 1
+      else if (flag_strong_eval_order
 	       && !CALL_EXPR_OPERATOR_SYNTAX (*expr_p))
 	{
-	  /* If flag_args_in_order == 1, we don't force an order on all
-	     function arguments, but do evaluate the object argument first.  */
+	  /* If flag_strong_eval_order, evaluate the object argument first.  */
 	  tree fntype = TREE_TYPE (CALL_EXPR_FN (*expr_p));
 	  if (POINTER_TYPE_P (fntype))
 	    fntype = TREE_TYPE (fntype);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b6398ff..65a6885 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2237,14 +2237,6 @@ option is used for the warning.
 Turn off all access checking.  This switch is mainly useful for working
 around bugs in the access control code.
 
-@item -fargs-in-order
-@opindex fargs-in-order
-Evaluate function arguments and operands of some binary expressions in
-left-to-right order, and evaluate the right side of an assignment
-before the left side, as proposed in P0145R2.  Enabled by default with
-@option{-std=c++1z}.  @option{-fargs-in-order=1} implements all of the
-ordering requirements except function arguments.
-
 @item -fcheck-new
 @opindex fcheck-new
 Check that the pointer returned by @code{operator new} is non-null
@@ -2483,6 +2475,15 @@ represented in the minimum number of bits needed to represent all the
 enumerators).  This assumption may not be valid if the program uses a
 cast to convert an arbitrary integer value to the enumerated type.
 
+@item -fstrong-eval-order
+@opindex fstrong-eval-order
+Evaluate member access, array subscripting, and shift expressions in
+left-to-right order, and evaluate assignment in right-to-left order,
+as adopted for C++17.  Enabled by default with @option{-std=c++1z}.
+@option{-fstrong-eval-order=some} enables just the ordering of member
+access and shift expressions, and is the default without
+@option{-std=c++1z}.
+
 @item -ftemplate-backtrace-limit=@var{n}
 @opindex ftemplate-backtrace-limit
 Set the maximum number of template instantiation notes for a single
diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order1.C b/gcc/testsuite/g++.dg/cpp1z/eval-order1.C
deleted file mode 100644
index 278990d..0000000
--- a/gcc/testsuite/g++.dg/cpp1z/eval-order1.C
+++ /dev/null
@@ -1,21 +0,0 @@
-// P0145R2: Refining Expression Order for C++
-// { dg-do run }
-// { dg-options "-std=c++1z" }
-
-extern "C" int printf (const char *, ...);
-void sink(...) { }
-
-int last = 0;
-int f(int i)
-{
-  if (i < last)
-    __builtin_abort ();
-  last = i;
-  return i;
-}
-
-int main()
-{
-  sink(f(1), f(2));
-  sink(f(3), f(4), f(5));
-}
diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
index 15df903..d382d07 100644
--- a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
+++ b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
@@ -1,6 +1,6 @@
 // P0145R2: Refining Expression Order for C++
 // { dg-do run }
-// { dg-options "-std=c++1z -fargs-in-order=1" }
+// { dg-options "-std=c++1z" }
 
 extern "C" int printf (const char *, ...);
 void sink(...) { }