From patchwork Fri May 27 15:28:53 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 68766 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp153658qge; Fri, 27 May 2016 08:29:18 -0700 (PDT) X-Received: by 10.66.87.198 with SMTP id ba6mr23763682pab.151.1464362958853; Fri, 27 May 2016 08:29:18 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id e189si14967967pfa.193.2016.05.27.08.29.18 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 May 2016 08:29:18 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-428467-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-428467-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-428467-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:date:message-id:subject:from:to:content-type; q= dns; s=default; b=jewR2DBAQpqrqq4Eo+nJ+ds2t5/ICPdBWaUDtpUlgqFTqf FJu9/BPEwLxkZ6iUQjtZIQYpYqbJ8o0kqvdShfHAw/zlqESUABwVriF13UY1d35y /gOsAGXPgdqZjRJ6k+RrauCKsqMc02WUgBX5NYRR9L1mTcSHI5DtT/c+65GFg= 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:date:message-id:subject:from:to:content-type; s= default; bh=SH8YZKgBY8Q3THAovPT5aPOgqOM=; b=b3IDDZj/AKtu9gIUvu5S bFBusWTDu7Ik/TNr4diDJaszJ1p8W3PQcNDMC4U7zsKlwdfIE4X9PZqhS1eIit0o UW7uDEcpkuFPIezuTIPxRz22ngV2EMP0N/TDV7p5xtMDBE5EyyXDEjGCX8V3g2tk FzkEgKfbX13Ep38uc31ef2E= Received: (qmail 80034 invoked by alias); 27 May 2016 15:29:07 -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 80017 invoked by uid 89); 27 May 2016 15:29:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=our X-HELO: mail-qk0-f178.google.com Received: from mail-qk0-f178.google.com (HELO mail-qk0-f178.google.com) (209.85.220.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 27 May 2016 15:28:55 +0000 Received: by mail-qk0-f178.google.com with SMTP id h185so48508712qke.2 for ; Fri, 27 May 2016 08:28:55 -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:date:message-id:subject:from:to; bh=3Ir2puGoPi5/ZPbFL/EflzeVtdGtZS7KSDDYfhQouP4=; b=euV+OQybpe7v0Ux912tEUvQb4YwLtf0DgWUK3cPP43XuNY8esHWZwUMuGEIBx2JnCX 8mrxuf2pp/XHSZn0UZYVKUJ8n90LkkbonEpOZbJwCycmcPiNNmyAl3BhRz/tuhJEHuPx W6rK6RETIpLLeMCej+e3sDTMVhlmbhudaIBHf8xiFpoIy9AAdfwtrt5RgRSCOLrg89kM OBFInlWjjwEyFd3Gs/ilUk7Nwjzi0yD0f1P9Sjeujk/TCIZ3DoH4c2UJfXAnlGH+Auul kHW2t1/MxatmWRVkkzlcV1UZTx+5Glt9SnBEAxuqsnwW9Mqb/r9TpJnOgyBgzCIyQJ5s lbbw== X-Gm-Message-State: ALyK8tISXGlPelRCy12JoRVfXF7jFOqrXkpJcm4LzLB9ELNbBCuVdFOvZzOi2tboY3j92V406DtiHVjn2fUHN4v4 MIME-Version: 1.0 X-Received: by 10.55.172.6 with SMTP id v6mr13307270qke.98.1464362933614; Fri, 27 May 2016 08:28:53 -0700 (PDT) Received: by 10.200.42.218 with HTTP; Fri, 27 May 2016 08:28:53 -0700 (PDT) Date: Sat, 28 May 2016 01:28:53 +1000 Message-ID: Subject: [PATCH2][PR71252] Fix insertion point of stmt_to_insert From: Kugan Vivekanandarajah To: "gcc-patches@gcc.gnu.org" , Richard Biener , Jakub Jelinek X-IsSubscribed: yes 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 Thanks, Kugan gcc/testsuite/ChangeLog: 2016-05-28 Kugan Vivekanandarajah * gcc.dg/tree-ssa/pr71269.c: New test. gcc/ChangeLog: 2016-05-28 Kugan Vivekanandarajah * 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/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c index c9ed679..8a2154f 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 + gsi_insert_after (&gsi, stmt_to_insert, GSI_NEW_STMT); +} + + /* 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)) {