The C++ sequence point rules for compound assignment (+= and such) are slightly different from those for C: the value of the LHS is not loaded until after the RHS value (and LHS location) are evaluated. Since we don't have tree codes for compound assignment, I need to handle this in C++ by preevaluating the RHS into a temporary. Doing this breaks various gomp tests because the logic for recognizing compound assignment gets confused by the COMPOUND_EXPR, so I've adjusted the appropriate omp code appropriately.

Tested x86_64-pc-linux-gnu, OK for trunk?

commit cec1fed71c8cf44b26fbb80546b1c2dd3780ebe1
Author: Jason Merrill <ja...@redhat.com>
Date:   Fri Jul 8 14:07:23 2011 -0400

    	PR c++/45437
    gcc/
    	* gimplify.c (goa_stabilize_expr): Handle RHS preevaluation in
    	compound assignment.
    gcc/c-family/
    	* c-omp.c (check_omp_for_incr_expr): Handle preevaluation.
    gcc/cp/
    	* typeck.c (cp_build_modify_expr): Preevaluate RHS.

diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index 1ee0bf0..1c5415a 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -213,6 +213,29 @@ check_omp_for_incr_expr (location_t loc, tree exp, tree decl)
         return fold_build2_loc (loc, PLUS_EXPR,
 			    TREE_TYPE (exp), TREE_OPERAND (exp, 0), t);
       break;
+    case COMPOUND_EXPR:
+      {
+	/* cp_build_modify_expr forces preevaluation of the RHS to make
+	   sure that it is evaluated before the lvalue-rvalue conversion
+	   is applied to the LHS.  Reconstruct the original expression.  */
+	tree op0 = TREE_OPERAND (exp, 0);
+	tree op1 = TREE_OPERAND (exp, 1);
+	if (TREE_CODE (op0) == CONVERT_EXPR)
+	  op0 = TREE_OPERAND (op0, 0);
+	if (TREE_CODE (op0) == TARGET_EXPR
+	    && !VOID_TYPE_P (TREE_TYPE (op0)))
+	  {
+	    tree temp = TARGET_EXPR_SLOT (op0);
+	    if (TREE_CODE_CLASS (TREE_CODE (op1)) == tcc_binary
+		&& TREE_OPERAND (op1, 1) == temp)
+	      {
+		op1 = copy_node (op1);
+		TREE_OPERAND (op1, 1) = TARGET_EXPR_INITIAL (op0);
+		return check_omp_for_incr_expr (loc, op1, decl);
+	      }
+	  }
+	break;
+      }
     default:
       break;
     }
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5febff5..1479092 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6663,6 +6663,8 @@ cp_build_modify_expr (tree lhs, enum tree_code modifycode, tree rhs,
 	}
       else
 	{
+	  tree init = NULL_TREE;
+
 	  /* A binary op has been requested.  Combine the old LHS
 	     value with the RHS producing the value we should actually
 	     store into the LHS.  */
@@ -6670,7 +6672,17 @@ cp_build_modify_expr (tree lhs, enum tree_code modifycode, tree rhs,
 			 && MAYBE_CLASS_TYPE_P (TREE_TYPE (lhstype)))
 			|| MAYBE_CLASS_TYPE_P (lhstype)));
 
+	  /* Preevaluate the RHS to make sure its evaluation is complete
+	     before the lvalue-to-rvalue conversion of the LHS:
+
+	     [expr.ass] With respect to an indeterminately-sequenced
+	     function call, the operation of a compound assignment is a
+	     single evaluation. [ Note: Therefore, a function call shall
+	     not intervene between the lvalue-to-rvalue conversion and the
+	     side effect associated with any single compound assignment
+	     operator. -- end note ]  */
 	  lhs = stabilize_reference (lhs);
+	  rhs = stabilize_expr (rhs, &init);
 	  newrhs = cp_build_binary_op (input_location,
 				       modifycode, lhs, rhs,
 				       complain);
@@ -6682,6 +6694,9 @@ cp_build_modify_expr (tree lhs, enum tree_code modifycode, tree rhs,
 	      return error_mark_node;
 	    }
 
+	  if (init)
+	    newrhs = cp_build_compound_expr (init, newrhs, complain);
+
 	  /* Now it looks like a plain assignment.  */
 	  modifycode = NOP_EXPR;
 	  if (c_dialect_objc ())
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 4ff7e93..5a928be 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -6451,6 +6451,13 @@ goa_stabilize_expr (tree *expr_p, gimple_seq *pre_p, tree lhs_addr,
 	  saw_lhs |= goa_stabilize_expr (&TREE_OPERAND (expr, 0), pre_p,
 					 lhs_addr, lhs_var);
 	  break;
+	case COMPOUND_EXPR:
+	  /* Break out any preevaluations from cp_build_modify_expr.  */
+	  for (; TREE_CODE (expr) == COMPOUND_EXPR;
+	       expr = TREE_OPERAND (expr, 1))
+	    gimplify_stmt (&TREE_OPERAND (expr, 0), pre_p);
+	  *expr_p = expr;
+	  return goa_stabilize_expr (expr_p, pre_p, lhs_addr, lhs_var);
 	default:
 	  break;
 	}
diff --git a/gcc/testsuite/g++.dg/expr/compound-asn1.C b/gcc/testsuite/g++.dg/expr/compound-asn1.C
new file mode 100644
index 0000000..194235c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/compound-asn1.C
@@ -0,0 +1,15 @@
+// PR c++/45437
+// { dg-options -Wsequence-point }
+// { dg-do run }
+
+bool f(bool& b) {
+  b = true;
+  return false;
+}
+
+int main() {
+  bool b = false;
+  b |= f(b);
+  if (!b)
+    return 1;
+}
diff --git a/gcc/testsuite/g++.dg/warn/sequence-pt-1.C b/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
index 05eee82..6a98fd7 100644
--- a/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
+++ b/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
@@ -62,7 +62,7 @@ foo (int a, int b, int n, int p, int *ptr, struct s *sptr,
   (a = a++) && b; /* { dg-warning "undefined" "sequence point warning" } */
   b, (a = a++); /* { dg-warning "undefined" "sequence point warning" } */
   (a = a++), b; /* { dg-warning "undefined" "sequence point warning" } */
-  a ^= b ^= a ^= b; /* { dg-warning "undefined" "sequence point warning" } */
+  a ^= b ^= a ^= b; /* { dg-bogus "undefined" "sequence point warning" } */
 
   a = a; /* { dg-bogus "undefined" "bogus sequence point warning" } */
   a = (a++ && 4); /* { dg-bogus "undefined" "bogus sequence point warning" } */

Reply via email to