From patchwork Mon May 30 00:35: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: 68811 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp1151821qge; Sun, 29 May 2016 17:35:54 -0700 (PDT) X-Received: by 10.98.13.212 with SMTP id 81mr37328093pfn.18.1464568554232; Sun, 29 May 2016 17:35:54 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id ti6si46568818pac.128.2016.05.29.17.35.53 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 29 May 2016 17:35:54 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-428561-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-428561-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-428561-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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=gTkVsO1fUq5Bc//yA zwEw7tntkSqKv3yV7D+soABF6fTFSg47zPgisKZGP+GtOSWZBS7SQgBu/ftCs+zO 2GAbkrJnhlcu/0KXD3QbPXJ5m0btkZLxLhhGmDYR5DVvXhZq+APtnP+TqTZF0YqO verkX8M5SqAb3wiqNmuMbPkkEA= 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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=fiyRWpFtvyR7aqP0GayhvE+ sLr4=; b=wPMyiIeBbPCj9FC/QLsETsvYNshkW4mLfAE/J0/pRZ/diBzfjM4Q+ud kSuVsg+ot7fDONcvPYA+f9DsbGvm5Mwmljnyd5d/WZ9ZueyUOL8ybNg/BSKNptoP 8Nl7AMowWMCa6YLhgRQDAyj0COnJ44VgNVEgi6LaKomzs24tJ8u4= Received: (qmail 34775 invoked by alias); 30 May 2016 00:35:43 -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 34764 invoked by uid 89); 30 May 2016 00:35:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=dump_file, dump_flags, afterwards, opindex X-HELO: mail-pa0-f42.google.com Received: from mail-pa0-f42.google.com (HELO mail-pa0-f42.google.com) (209.85.220.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 30 May 2016 00:35:31 +0000 Received: by mail-pa0-f42.google.com with SMTP id bz2so26869394pad.1 for ; Sun, 29 May 2016 17:35:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to; bh=LfiHKpwc9s6xlyjMmdOV5U2Q69XBnRrH9Ae8/Z4GR4k=; b=fjJKVdiU6VNokzXB9eGl0zcqym1i/qpuM6JpMqjCtiT+2oayrrMDDQ3BsJhN7NtHyv W3BTzjiIXXpyUUq2VF88c7USOO1yE+G+IXBVgYtK0Jx5GMcG3fb1SsWlHcJeca3lb6Db WHSNzDsT6EHPLJOhLRt8z3Hv5WEoC/k/jQLFeZ5mS9/KLGLbaw/trOuu+BcyqL7e1/SV +52S+EfnruHUjT0yBNosuYFPdyqo8PlQNk5SP9fJy+0xfjQdG4xdkT4IijllEaXxtjtS Nneb7QCoWVwh8ykvYz2VU5/42cNhzvtr0CdgcnHs2of0dBnZV/aYEoXy0w1VBISZiW8k zOow== X-Gm-Message-State: ALyK8tLQ1XTSEfayGgPBoEZCCpIbsFG/dE2dwKL6rIVwYFQShgEqDLaRjNfDbYTf4Ls1HS8E X-Received: by 10.66.119.177 with SMTP id kv17mr41978564pab.57.1464568529878; Sun, 29 May 2016 17:35:29 -0700 (PDT) Received: from [10.1.1.13] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by smtp.gmail.com with ESMTPSA id ff10sm9321306pac.13.2016.05.29.17.35.26 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 29 May 2016 17:35:29 -0700 (PDT) Subject: Re: [PATCH2][PR71252] Fix insertion point of stmt_to_insert To: "gcc-patches@gcc.gnu.org" , Richard Biener , Jakub Jelinek References: From: kugan Message-ID: <574B8AC2.8090602@linaro.org> Date: Mon, 30 May 2016 10:35:14 +1000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes On 28/05/16 01:28, Kugan Vivekanandarajah wrote: > Hi Richard, > > This fix insertion point of stmt_to_insert based on your comments. In > insert_stmt_before_use , I now use find_insert_point such that we > insert the stmt_to_insert after its operands are defined. This means > that we now have to insert before and insert after in other cases. > > I also factored out uses of insert_stmt_before_use. > > I tested this with: > ./build/gcc/f951 cp2k_single_file.f90 -O3 -ffast-math -march=westmere > > I am running bootstrap and regression testing on x86-64-linux gnu. Is > this OK for trunk if the testing is fine ? I will also test with other > test cases from relevant PRs > > Hi, Here is the tested patch. I made a slight change to reflect the paten used everywhere else in reassoc. i.e., in insert_stmt_before_use, after finding the insertion point I now do: + if (stmt == insert_point) + gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT); + else + insert_stmt_after (stmt_to_insert, insert_point); This is consistent with what is done in other places. I tested the patch with CPU2006 and bootstrapped and regression tested for x86-64-none-linux-gnu with no new regressions. Is this OK for trunk? Thanks, Kugan gcc/testsuite/ChangeLog: 2016-05-28 Kugan Vivekanandarajah PR middle-end/71269 PR middle-end/71292 * gcc.dg/tree-ssa/pr71269.c: New test. * gcc.dg/tree-ssa/pr71292.c: New test. gcc/ChangeLog: 2016-05-28 Kugan Vivekanandarajah PR middle-end/71269 PR middle-end/71252 * tree-ssa-reassoc.c (insert_stmt_before_use): Use find_insert_point so that inserted stmt will not dominate stmts that defines its operand. (rewrite_expr_tree): Add stmt_to_insert before adding the use stmt. (rewrite_expr_tree_parallel): Likewise. 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/testsuite/gcc.dg/tree-ssa/pr71292.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71292.c index e69de29..1a25d93 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr71292.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71292.c @@ -0,0 +1,12 @@ +/* PR middle-end/71292 */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +unsigned long a; +long b, d; +int c; +void fn1 () +{ + unsigned long e = a + c; + b = d + e + a + 8; +} diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c index c9ed679..bc4b55a 100644 --- a/gcc/tree-ssa-reassoc.c +++ b/gcc/tree-ssa-reassoc.c @@ -1777,16 +1777,6 @@ eliminate_redundant_comparison (enum tree_code opcode, return false; } -/* If the stmt that defines operand has to be inserted, insert it - before the use. */ -static void -insert_stmt_before_use (gimple *stmt, gimple *stmt_to_insert) -{ - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); - gimple_set_uid (stmt_to_insert, gimple_uid (stmt)); - gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT); -} - /* Transform repeated addition of same values into multiply with constant. */ @@ -3799,6 +3789,29 @@ find_insert_point (gimple *stmt, tree rhs1, tree rhs2) return stmt; } +/* If the stmt that defines operand has to be inserted, insert it + before the use. */ +static void +insert_stmt_before_use (gimple *stmt, gimple *stmt_to_insert) +{ + gcc_assert (is_gimple_assign (stmt_to_insert)); + tree rhs1 = gimple_assign_rhs1 (stmt_to_insert); + tree rhs2 = gimple_assign_rhs2 (stmt_to_insert); + gimple *insert_point = find_insert_point (stmt, rhs1, rhs2); + gimple_stmt_iterator gsi = gsi_for_stmt (insert_point); + gimple_set_uid (stmt_to_insert, gimple_uid (insert_point)); + + /* If the insert point is not stmt, then insert_point would be + the point where operand rhs1 or rhs2 is defined. In this case, + stmt_to_insert has to be inserted afterwards. This would + only happen when the stmt insertion point is flexible. */ + if (stmt == insert_point) + gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT); + else + insert_stmt_after (stmt_to_insert, insert_point); +} + + /* Recursively rewrite our linearized statements so that the operators match those in OPS[OPINDEX], putting the computation in rank order. Return new lhs. */ @@ -3835,6 +3848,12 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex, print_gimple_stmt (dump_file, stmt, 0, 0); } + /* 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); /* Even when changed is false, reassociation could have e.g. removed some redundant operations, so unless we are just swapping the arguments or unless there is no change at all (then we just @@ -3843,12 +3862,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,12 +3877,6 @@ 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); @@ -4109,16 +4116,18 @@ rewrite_expr_tree_parallel (gassign *stmt, int width, print_gimple_stmt (dump_file, stmts[i], 0, 0); } + /* 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); + stmt1 = stmt2 = NULL; + /* We keep original statement only for the last one. All others are recreated. */ if (i == stmt_num - 1) { - /* 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]); @@ -4126,12 +4135,6 @@ rewrite_expr_tree_parallel (gassign *stmt, int width, else { 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 (dump_file && (dump_flags & TDF_DETAILS)) {