From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27844 invoked by alias); 13 Jan 2014 19:11:48 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 27828 invoked by uid 89); 13 Jan 2014 19:11:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,T_FRT_FREE autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 13 Jan 2014 19:11:46 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s0DJBhAN001583 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 13 Jan 2014 14:11:43 -0500 Received: from tucnak.zalov.cz (vpn1-6-246.ams2.redhat.com [10.36.6.246]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s0DJBf3w004382 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 13 Jan 2014 14:11:43 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.7/8.14.7) with ESMTP id s0DJBfET001052; Mon, 13 Jan 2014 20:11:41 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.7/8.14.7/Submit) id s0DJBXQf001051; Mon, 13 Jan 2014 20:11:33 +0100 Date: Mon, 13 Jan 2014 19:11:00 -0000 From: Jakub Jelinek To: "Joseph S. Myers" , Richard Biener , "Balaji V. Iyer" Cc: gcc-patches@gcc.gnu.org Subject: [C PATCH] Preevaluate rhs for lhs op= rhs in C (PR c/58943) Message-ID: <20140113191133.GC892@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2014-01/txt/msg00746.txt.bz2 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 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