Hi!
This patch fixes the following testcase by preevaluating rhs if it has
(can have) side-effects in lhs op= rhs expressions. Bootstrapped/regtested
on x86_64-linux and i686-linux, ok for trunk?
C++ already does a similar thing (though in that case with TARGET_EXPRs).
Note1: had to tweak ssa-fre-33.c testcase a little bit (but it still fails
without the fix which went together with it and succeeds with the fix
and from that point onwards), because before fre1 there isn't enough forward
propagation that would make it constant (the addition result becomes
constant during fre1).
Note2: c-c++-common/cilk-plus/AN/rank_mismatch2.c ICEs now, supposedly
array notation handling doesn't handle SAVE_EXPRs properly, Balaji, do you
think you can debug it and fix up afterwards?
2014-01-13 Jakub Jelinek <ja...@redhat.com>
PR c/58943
* c-typeck.c (build_modify_expr): For lhs op= rhs, if rhs has side
effects, preevaluate rhs using SAVE_EXPR first.
* c-omp.c (c_finish_omp_atomic): Set in_late_binary_op around
build_modify_expr with non-NOP_EXPR opcode. Handle return from it
being COMPOUND_EXPR.
(c_finish_omp_for): Handle incr being COMPOUND_EXPR with first
operand a SAVE_EXPR and second MODIFY_EXPR.
* gcc.c-torture/execute/pr58943.c: New test.
* gcc.dg/tree-ssa/ssa-fre-33.c (main): Avoid using += in the test.
--- gcc/c/c-typeck.c.jj 2014-01-04 09:48:20.845147744 +0100
+++ gcc/c/c-typeck.c 2014-01-13 14:57:27.133743740 +0100
@@ -5193,6 +5193,7 @@ build_modify_expr (location_t location,
{
tree result;
tree newrhs;
+ tree rhseval = NULL_TREE;
tree rhs_semantic_type = NULL_TREE;
tree lhstype = TREE_TYPE (lhs);
tree olhstype = lhstype;
@@ -5254,8 +5255,17 @@ build_modify_expr (location_t location,
/* Construct the RHS for any non-atomic compound assignemnt. */
if (!is_atomic_op)
{
+ /* If in LHS op= RHS the RHS has side-effects, ensure they
+ are preevaluated before the rest of the assignment expression's
+ side-effects, because RHS could contain e.g. function calls
+ that modify LHS. */
+ if (TREE_SIDE_EFFECTS (rhs))
+ {
+ newrhs = in_late_binary_op ? save_expr (rhs) : c_save_expr (rhs);
+ rhseval = newrhs;
+ }
newrhs = build_binary_op (location,
- modifycode, lhs, rhs, 1);
+ modifycode, lhs, newrhs, 1);
/* The original type of the right hand side is no longer
meaningful. */
@@ -5269,7 +5279,7 @@ build_modify_expr (location_t location,
if so, we need to generate setter calls. */
result = objc_maybe_build_modify_expr (lhs, newrhs);
if (result)
- return result;
+ goto return_result;
/* Else, do the check that we postponed for Objective-C. */
if (!lvalue_or_else (location, lhs, lv_assign))
@@ -5363,7 +5373,7 @@ build_modify_expr (location_t location,
if (result)
{
protected_set_expr_location (result, location);
- return result;
+ goto return_result;
}
}
@@ -5384,11 +5394,15 @@ build_modify_expr (location_t location,
as the LHS argument. */
if (olhstype == TREE_TYPE (result))
- return result;
+ goto return_result;
result = convert_for_assignment (location, olhstype, result, rhs_origtype,
ic_assign, false, NULL_TREE, NULL_TREE, 0);
protected_set_expr_location (result, location);
+
+return_result:
+ if (rhseval)
+ result = build2 (COMPOUND_EXPR, TREE_TYPE (result), rhseval, result);
return result;
}
--- gcc/c-family/c-omp.c.jj 2014-01-04 09:48:20.000000000 +0100
+++ gcc/c-family/c-omp.c 2014-01-13 15:23:51.653591098 +0100
@@ -136,7 +136,7 @@ c_finish_omp_atomic (location_t loc, enu
enum tree_code opcode, tree lhs, tree rhs,
tree v, tree lhs1, tree rhs1, bool swapped, bool seq_cst)
{
- tree x, type, addr;
+ tree x, type, addr, pre = NULL_TREE;
if (lhs == error_mark_node || rhs == error_mark_node
|| v == error_mark_node || lhs1 == error_mark_node
@@ -194,9 +194,18 @@ c_finish_omp_atomic (location_t loc, enu
rhs = build2_loc (loc, opcode, TREE_TYPE (lhs), rhs, lhs);
opcode = NOP_EXPR;
}
+ bool save = in_late_binary_op;
+ in_late_binary_op = true;
x = build_modify_expr (loc, lhs, NULL_TREE, opcode, loc, rhs, NULL_TREE);
+ in_late_binary_op = save;
if (x == error_mark_node)
return error_mark_node;
+ if (TREE_CODE (x) == COMPOUND_EXPR)
+ {
+ pre = TREE_OPERAND (x, 0);
+ gcc_assert (TREE_CODE (pre) == SAVE_EXPR);
+ x = TREE_OPERAND (x, 1);
+ }
gcc_assert (TREE_CODE (x) == MODIFY_EXPR);
rhs = TREE_OPERAND (x, 1);
@@ -264,6 +273,8 @@ c_finish_omp_atomic (location_t loc, enu
x = omit_one_operand_loc (loc, type, x, rhs1addr);
}
+ if (pre)
+ x = omit_one_operand_loc (loc, type, x, pre);
return x;
}
@@ -555,6 +566,12 @@ c_finish_omp_for (location_t locus, enum
incr = c_omp_for_incr_canonicalize_ptr (elocus, decl, incr);
break;
+ case COMPOUND_EXPR:
+ if (TREE_CODE (TREE_OPERAND (incr, 0)) != SAVE_EXPR
+ || TREE_CODE (TREE_OPERAND (incr, 1)) != MODIFY_EXPR)
+ break;
+ incr = TREE_OPERAND (incr, 1);
+ /* FALLTHRU */
case MODIFY_EXPR:
if (TREE_OPERAND (incr, 0) != decl)
break;
--- gcc/testsuite/gcc.c-torture/execute/pr58943.c.jj1 2014-01-13
15:43:39.391463496 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr58943.c 2014-01-13
15:43:32.591490400 +0100
@@ -0,0 +1,19 @@
+/* PR c/58943 */
+
+unsigned int x[1] = { 2 };
+
+unsigned int
+foo (void)
+{
+ x[0] |= 128;
+ return 1;
+}
+
+int
+main ()
+{
+ x[0] |= foo ();
+ if (x[0] != 131)
+ __builtin_abort ();
+ return 0;
+}
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-33.c.jj 2012-02-09
12:11:45.000000000 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-33.c 2014-01-13 19:13:20.000000000
+0100
@@ -11,8 +11,8 @@ struct {
float x;
int main(int argc)
{
- vector float res = (vector float){0.0f,0.0f,0.0f,1.0f};
- res += (vector float){1.0f,2.0f,3.0f,4.0f};
+ vector float res;
+ res = (vector float){1.0f,2.0f,3.0f,5.0f};
s.global_res = res;
x = *((float*)&s.global_res + 1);
return 0;
Jakub
