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: 627271 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 3rGVL36LpHz9t0t for ; Sat, 28 May 2016 01:29:14 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=yHD0D4VU; 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 :mime-version:date:message-id:subject:from:to:content-type; q= dns; s=default; b=UPijUGu1M1MKjF007lzLzItkguUE95Oh/sRTLWjc3r29IQ akQxZmFSRHvffAE4WBCVQyDebgmEsasFOxFXMJ5P2YlGOzs3c75yJFJNT4D4Id7O XMufZaeTZx4facTfWezV83x1hivgknu20QY4p9BP0tJmGuIcmBGoRF8eZMViE= 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=yHD0D4VUiqTcf9H3y221 tx/Arhh71ArxOaH0mfXWeNMXYNLVeUoB0zIFn9PVDdS2XdV3JnlYCGclMRRCuzpH 5EoQcVnsqUleqjeyUFdisxi2zJlnyzot7dVZzfRIRgOL/otn7dFRGrqnMT5wBtLW xIxXaOVUwNAZGzAcY6kLo50= 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)) {