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: 627557 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rHyMn3pMBz9t43 for ; Mon, 30 May 2016 10:35:49 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=XyL6F8W6; dkim-atps=neutral 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=qDqAQjxxwy98IZ9NZ fa/Nbt8R4ISqijT7AmXG4fxAJ7ZwK5aj8yixgBUV6ndeGf0fvHsaciVzxhmZTX4n ouPVQ9Cn05m8EVgku6FWLHGHtj1/oj9F/pjqYCmxnzLBd8M5sqmtJXYi3fTx9myT /7GV7lvbj18W5FLBspQ0XzqEb0= 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=XyL6F8W6PhFk1Re7SS1mU6RRpxWxzZynAbnR4zAVcOj6Mp86LvLhToB qoaArfipHsKLYAvGFuwh2uj0pyocvD/Zu+tAt2s1hj36RD+fRkZee1wx707ercat oP0HvjvDFZl9DZSCBMtA5CF3mnm6Kq8ONOWkDB90xg8Qnw2gSyb8= 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)) {