From patchwork Thu May 26 09:32:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 68677 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp301874qge; Thu, 26 May 2016 02:33:20 -0700 (PDT) X-Received: by 10.98.99.66 with SMTP id x63mr12646464pfb.132.1464255200576; Thu, 26 May 2016 02:33:20 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id z67si5486392pfj.116.2016.05.26.02.33.20 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 May 2016 02:33:20 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-428339-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-428339-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-428339-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=j9EzE/PSsS/GkY0Sxs 9U5tlrBEv8Idk9IdKMrQUJHi/yohUWkj+Ei6+sIn8j+dzARot9QpU7iGZiSObGqy trimWugaK4+jzXEEh3ahVl2nZGeRqbBThk5A8HfGR10tz0u7SVhku+k1hhjcdBin Ve+Md3Mg9fWgPlSM/Bx3P7PpE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=ei3NoytvgizduwfTrwRJi4R2 OmU=; b=gbf8n9DG9d9Bd4GMIRGp6VPTMUabVKnMamM9hruzx2MBab9jT0uOzHzi zTjPE3B7gmvOvnEm2jAdT3lbpsFjK54w8rEQWC+UcVq3/U5ljNYHKPZHwLMvFD+3 Mje7SaB9PWRlys9kvSboGSgFaw94cgYfyT81o+7DnRAH4X2Atp8= Received: (qmail 61743 invoked by alias); 26 May 2016 09:32:28 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 61681 invoked by uid 89); 26 May 2016 09:32:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qg0-f47.google.com Received: from mail-qg0-f47.google.com (HELO mail-qg0-f47.google.com) (209.85.192.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 26 May 2016 09:32:16 +0000 Received: by mail-qg0-f47.google.com with SMTP id 90so34697901qgz.1 for ; Thu, 26 May 2016 02:32:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=R/+aksCIY/4K543MozbY3r4WhOyulshO5h2Xel2LNSY=; b=ZvzGh8kFT6c3UGUOepTvFZlce/UPvZaq+o8bun3gLEKSnTbKpzYcb2dSPk3Az6msF+ iuSL9fL95BztZlsrAKPIBzJV8kBBfbqh5uiYaC1CnwgqAMOqK/SXJSUmORUqmDr6KCql fbHLF12iWzjbWaY9P8FvToMmNwAjo88Kpf7CkVt5WfwCMucAeJgX20Ht20Rv6AxizZfj 3aevzCW3LELezM2hYtOm0V2eGfDC3UJ6yKMcmPbOIc9LFkV+FK0TRGhapCpWJGKirqfA ypontKUTG0tWjiXuGiG5CjPlJjhtIh/cpbZR0YQd3KS50SArbic0MEx+AxMey+YHIRnH TDsw== X-Gm-Message-State: ALyK8tLoc0MxfnvtUGz2b5ezJ7eV8HJIPWZSxZszJ6hWGd08qTuUPkRXgK6LrHjTVZUF910E/qHYgYTGR0ptwZDi MIME-Version: 1.0 X-Received: by 10.140.98.38 with SMTP id n35mr7596287qge.22.1464255134649; Thu, 26 May 2016 02:32:14 -0700 (PDT) Received: by 10.200.42.218 with HTTP; Thu, 26 May 2016 02:32:14 -0700 (PDT) In-Reply-To: <20160526081843.GW28550@tucnak.redhat.com> References: <20160526081843.GW28550@tucnak.redhat.com> Date: Thu, 26 May 2016 19:32:14 +1000 Message-ID: Subject: Re: [PR71252][PR71269] Fix trunk errors due to stmt_to_insert From: Kugan Vivekanandarajah To: Jakub Jelinek Cc: "gcc-patches@gcc.gnu.org" , Richard Biener X-IsSubscribed: yes Hi Jakub, On 26 May 2016 at 18:18, Jakub Jelinek wrote: > On Thu, May 26, 2016 at 02:17:56PM +1000, Kugan Vivekanandarajah wrote: >> --- a/gcc/tree-ssa-reassoc.c >> +++ b/gcc/tree-ssa-reassoc.c >> @@ -3767,8 +3767,10 @@ swap_ops_for_binary_stmt (vec ops, >> operand_entry temp = *oe3; >> oe3->op = oe1->op; >> oe3->rank = oe1->rank; >> + oe3->stmt_to_insert = oe1->stmt_to_insert; >> oe1->op = temp.op; >> oe1->rank= temp.rank; >> + oe1->stmt_to_insert = temp.stmt_to_insert; > > If you want to swap those 3 fields (what about the others?), can't you write > std::swap (oe1->op, oe3->op); > std::swap (oe1->rank, oe3->rank); > std::swap (oe1->stmt_to_insert, oe3->stmt_to_insert); > instead and drop operand_entry temp = *oe3; ? > >> } >> else if ((oe1->rank == oe3->rank >> && oe2->rank != oe3->rank) >> @@ -3779,8 +3781,10 @@ swap_ops_for_binary_stmt (vec ops, >> operand_entry temp = *oe2; >> oe2->op = oe1->op; >> oe2->rank = oe1->rank; >> + oe2->stmt_to_insert = oe1->stmt_to_insert; >> oe1->op = temp.op; >> oe1->rank = temp.rank; >> + oe1->stmt_to_insert = temp.stmt_to_insert; >> } > > Similarly. Done. Revised patch attached. Thanks, Kugan diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c index e69de29..4dceaaa 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c @@ -0,0 +1,10 @@ +/* PR middle-end/71269 */ +/* { dg-do compile } */ +/* { dg-options "-O1" } */ + +int a, b, c; +void fn2 (int); +void fn1 () +{ + fn2 (sizeof 0 + c + a + b + b); +} diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c index c9ed679..db6ac6b 100644 --- a/gcc/tree-ssa-reassoc.c +++ b/gcc/tree-ssa-reassoc.c @@ -3764,11 +3764,9 @@ swap_ops_for_binary_stmt (vec ops, && !is_phi_for_stmt (stmt, oe1->op) && !is_phi_for_stmt (stmt, oe2->op))) { - operand_entry temp = *oe3; - oe3->op = oe1->op; - oe3->rank = oe1->rank; - oe1->op = temp.op; - oe1->rank= temp.rank; + std::swap (oe1->op, oe3->op); + std::swap (oe1->rank, oe3->rank); + std::swap (oe1->stmt_to_insert, oe3->stmt_to_insert); } else if ((oe1->rank == oe3->rank && oe2->rank != oe3->rank) @@ -3776,11 +3774,9 @@ swap_ops_for_binary_stmt (vec ops, && !is_phi_for_stmt (stmt, oe1->op) && !is_phi_for_stmt (stmt, oe3->op))) { - operand_entry temp = *oe2; - oe2->op = oe1->op; - oe2->rank = oe1->rank; - oe1->op = temp.op; - oe1->rank = temp.rank; + std::swap (oe1->op, oe2->op); + std::swap (oe1->rank, oe2->rank); + std::swap (oe1->stmt_to_insert, oe2->stmt_to_insert); } } @@ -3790,6 +3786,42 @@ swap_ops_for_binary_stmt (vec ops, static inline gimple * find_insert_point (gimple *stmt, tree rhs1, tree rhs2) { + /* If rhs1 is defined by stmt_to_insert, insert after its argument + definion stmt. */ + if (TREE_CODE (rhs1) == SSA_NAME + && !gimple_nop_p (SSA_NAME_DEF_STMT (rhs1)) + && !gimple_bb (SSA_NAME_DEF_STMT (rhs1))) + { + gimple *stmt1 = SSA_NAME_DEF_STMT (rhs1); + gcc_assert (is_gimple_assign (stmt1)); + tree rhs11 = gimple_assign_rhs1 (stmt1); + tree rhs12 = gimple_assign_rhs2 (stmt1); + if (TREE_CODE (rhs11) == SSA_NAME + && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs11))) + stmt = SSA_NAME_DEF_STMT (rhs11); + if (TREE_CODE (rhs12) == SSA_NAME + && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs12))) + stmt = SSA_NAME_DEF_STMT (rhs12); + } + + /* If rhs2 is defined by stmt_to_insert, insert after its argument + definion stmt. */ + if (TREE_CODE (rhs2) == SSA_NAME + && !gimple_nop_p (SSA_NAME_DEF_STMT (rhs2)) + && !gimple_bb (SSA_NAME_DEF_STMT (rhs2))) + { + gimple *stmt1 = SSA_NAME_DEF_STMT (rhs2); + gcc_assert (is_gimple_assign (stmt1)); + tree rhs11 = gimple_assign_rhs1 (stmt1); + tree rhs12 = gimple_assign_rhs2 (stmt1); + if (TREE_CODE (rhs11) == SSA_NAME + && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs11))) + stmt = SSA_NAME_DEF_STMT (rhs11); + if (TREE_CODE (rhs12) == SSA_NAME + && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs12))) + stmt = SSA_NAME_DEF_STMT (rhs12); + } + if (TREE_CODE (rhs1) == SSA_NAME && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs1))) stmt = SSA_NAME_DEF_STMT (rhs1); @@ -3843,12 +3875,6 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex, { gimple *insert_point = find_insert_point (stmt, oe1->op, oe2->op); - /* If the stmt that defines operand has to be inserted, insert it - before the use. */ - if (oe1->stmt_to_insert) - insert_stmt_before_use (stmt, oe1->stmt_to_insert); - if (oe2->stmt_to_insert) - insert_stmt_before_use (stmt, oe2->stmt_to_insert); lhs = make_ssa_name (TREE_TYPE (lhs)); stmt = gimple_build_assign (lhs, gimple_assign_rhs_code (stmt), @@ -3864,17 +3890,17 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex, { gcc_checking_assert (find_insert_point (stmt, oe1->op, oe2->op) == stmt); - /* If the stmt that defines operand has to be inserted, insert it - before the use. */ - if (oe1->stmt_to_insert) - insert_stmt_before_use (stmt, oe1->stmt_to_insert); - if (oe2->stmt_to_insert) - insert_stmt_before_use (stmt, oe2->stmt_to_insert); gimple_assign_set_rhs1 (stmt, oe1->op); gimple_assign_set_rhs2 (stmt, oe2->op); update_stmt (stmt); } + /* If the stmt that defines operand has to be inserted, insert it + before the use after the stmt is inserted. */ + if (oe1->stmt_to_insert) + insert_stmt_before_use (stmt, oe1->stmt_to_insert); + if (oe2->stmt_to_insert) + insert_stmt_before_use (stmt, oe2->stmt_to_insert); if (rhs1 != oe1->op && rhs1 != oe2->op) remove_visited_stmt_chain (rhs1); @@ -3893,11 +3919,6 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex, /* Rewrite the next operator. */ oe = ops[opindex]; - /* If the stmt that defines operand has to be inserted, insert it - before the use. */ - if (oe->stmt_to_insert) - insert_stmt_before_use (stmt, oe->stmt_to_insert); - /* Recurse on the LHS of the binary operator, which is guaranteed to be the non-leaf side. */ tree new_rhs1 @@ -3944,6 +3965,10 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex, update_stmt (stmt); } + /* If the stmt that defines operand has to be inserted, insert it + before the use after the use_stmt is inserted. */ + if (oe->stmt_to_insert) + insert_stmt_before_use (stmt, oe->stmt_to_insert); if (dump_file && (dump_flags & TDF_DETAILS)) { fprintf (dump_file, " into "); @@ -4115,24 +4140,41 @@ rewrite_expr_tree_parallel (gassign *stmt, int width, { /* If the stmt that defines operand has to be inserted, insert it before the use. */ - if (stmt1) - insert_stmt_before_use (stmts[i], stmt1); - if (stmt2) - insert_stmt_before_use (stmts[i], stmt2); gimple_assign_set_rhs1 (stmts[i], op1); gimple_assign_set_rhs2 (stmts[i], op2); update_stmt (stmts[i]); } else { + /* PR71252: stmt_to_insert has to be inserted after use stmt created + by build_and_add_sum. However if the other operand doesnt have define-stmt + or is defined by GIMPLE_NOP, we have to insert it first. */ + if (stmt1 + && (TREE_CODE (op2) != SSA_NAME + || !gimple_bb (SSA_NAME_DEF_STMT (op2)) + || gimple_nop_p (SSA_NAME_DEF_STMT (op2)))) + { + insert_stmt_before_use (stmts[i], stmt1); + stmt1 = NULL; + } + if (stmt2 + && (TREE_CODE (op1) != SSA_NAME + || !gimple_bb (SSA_NAME_DEF_STMT (op1)) + || gimple_nop_p (SSA_NAME_DEF_STMT (op1)))) + { + insert_stmt_before_use (stmts[i], stmt2); + stmt2 = NULL; + } stmts[i] = build_and_add_sum (TREE_TYPE (last_rhs1), op1, op2, opcode); - /* If the stmt that defines operand has to be inserted, insert it - before new build_and_add stmt after it is created. */ - if (stmt1) - insert_stmt_before_use (stmts[i], stmt1); - if (stmt2) - insert_stmt_before_use (stmts[i], stmt2); } + + /* If the stmt that defines operand has to be inserted, insert it + before new use stmt after it is created. */ + if (stmt1) + insert_stmt_before_use (stmts[i], stmt1); + if (stmt2) + insert_stmt_before_use (stmts[i], stmt2); + stmt1 = stmt2 = NULL; if (dump_file && (dump_flags & TDF_DETAILS)) { fprintf (dump_file, " into "); @@ -5312,15 +5354,15 @@ reassociate_bb (basic_block bb) { tree last_op = ops.last ()->op; - /* If the stmt that defines operand has to be inserted, insert it - before the use. */ - if (ops.last ()->stmt_to_insert) - insert_stmt_before_use (stmt, ops.last ()->stmt_to_insert); if (powi_result) transform_stmt_to_multiply (&gsi, stmt, last_op, powi_result); else transform_stmt_to_copy (&gsi, stmt, last_op); + /* If the stmt that defines operand has to be inserted, insert it + before the use. */ + if (ops.last ()->stmt_to_insert) + insert_stmt_before_use (stmt, ops.last ()->stmt_to_insert); } else {