From patchwork Sat Mar 10 07:55:06 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Pinski X-Patchwork-Id: 145843 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]) by ozlabs.org (Postfix) with SMTP id 8A256B6EEE for ; Sat, 10 Mar 2012 18:55:29 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1331970930; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=tgi7anXGERONLCsgvdSIsyP6FJ8=; b=Y2FYBsONKEGP5vWC4NV+Zf92vtBCWFZ9qp8yIp2uaSoP0AZF+oWrsxbt/Jw7og yOJNEWTxFu633qJxRDFqZWztosKecGNQW4renjEUj2JWvDbHBLkWYdiJ6sy6G5/C VO6mfsgLM2H9nahqqhJMr7gb9rPtyR4qDdRohRYmvsdu0= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=BMY/ubDKgO/NyvbFnoKXt4jU5WQar07/VWBluXo3akgZGBddWIBEPDDin3w+03 D4LXCiP5HTt/Ho3Jh/HYMNBPlFdJdGbyaG6t400fQLp3fJY/ZXJ3YdWOlTIPonLE gDrA42jXmN5hQKapEFrmSexvaZpXeS7yt8Sq26IH+U2y8=; Received: (qmail 11146 invoked by alias); 10 Mar 2012 07:55:25 -0000 Received: (qmail 10873 invoked by uid 22791); 10 Mar 2012 07:55:21 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, TW_TM X-Spam-Check-By: sourceware.org Received: from mail-we0-f175.google.com (HELO mail-we0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 10 Mar 2012 07:55:08 +0000 Received: by wera1 with SMTP id a1so1966870wer.20 for ; Fri, 09 Mar 2012 23:55:06 -0800 (PST) MIME-Version: 1.0 Received: by 10.180.96.168 with SMTP id dt8mr1125207wib.18.1331366106500; Fri, 09 Mar 2012 23:55:06 -0800 (PST) Received: by 10.216.10.13 with HTTP; Fri, 9 Mar 2012 23:55:06 -0800 (PST) In-Reply-To: References: Date: Fri, 9 Mar 2012 23:55:06 -0800 Message-ID: Subject: Re: [PATCH] Improve PHI-OPT when there are multiple phis From: Andrew Pinski To: Richard Guenther Cc: GCC Patches X-IsSubscribed: yes 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 Woops I forgot the patch. Thanks, Andrew Pinski On Fri, Mar 9, 2012 at 11:45 AM, Andrew Pinski wrote: > On Tue, Jan 24, 2012 at 2:50 AM, Richard Guenther > wrote: >> On Tue, Jan 24, 2012 at 7:34 AM, Andrew Pinski >> wrote: >>> Hi, >>>  Right now PHI-OPT does try to handle the case where we have multiple >>> PHIs but the other PHIs have the same value for the edges we care >>> about. >>> This fixes the issue and allows PHI-OPT to handle a few more cases and >>> it removes the TODO in the comments. >>> >>> OK For 4.8? Bootstrapped and tested on x86_64-linux-gnu with no regressions. >>> >>> Thanks, >>> Andrew Pinski >>> >>> ChangeLog: >>> * tree-ssa-phiopt.c (gimple_phi_singleton_for_edges): New function. >> >> The name is confusing I think, because it returns the single non-singleton >> PHI for the edge pair ... you can avoid choosing a better name by >> inlining it to its single call site.  I'd maybe name it >> single_non_singleton_phi_for_edges. >> >> Otherwise ok. > > Here is the updated patch with one small change, value_replacement > uses single_non_singleton_phi_for_edges now too. > > OK still?  Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > > Thanks, > Andrew Pinski > > > ChangeLog: > * tree-ssa-phiopt.c (single_non_singleton_phi_for_edges): New function. > (tree_ssa_phiopt_worker): Use single_non_singleton_phi_for_edges. > (value_replacement): Likewise. > (empty_block_p): Check also if the PHIs for the block are empty. > > testsuite/ChangeLog: > * gcc.dg/tree-ssa/phi-opt-7.c: New testcase. > > >> >> Thanks, >> Richard. >> >>> (tree_ssa_phiopt_worker): Use gimple_phi_singleton_for_edges. >>> (empty_block_p): Check also if the PHIs for the block are empty. >>> >>> testsuite/ChangeLog: >>> * gcc.dg/tree-ssa/phi-opt-7.c: New testcase. Index: testsuite/gcc.dg/tree-ssa/phi-opt-7.c =================================================================== --- testsuite/gcc.dg/tree-ssa/phi-opt-7.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/phi-opt-7.c (revision 0) @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O1 -fdump-tree-optimized" } */ + +int g(int,int); +int f(int t, int c) +{ + int d = 0; + int e = 0; + if (t) + { + d = t; + if (c) e = 1; + } + else d = 0, e = 0; + return g(d,e); +} + +/* There should be one ifs as one of them should be changed into + a conditional and the other should be there still. */ +/* { dg-final { scan-tree-dump-times "if" 1 "optimized" } }*/ +/* { dg-final { scan-tree-dump-times "D.\[0-9\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */ + Index: testsuite/gcc.dg/tree-ssa/phi-opt-8.c =================================================================== --- testsuite/gcc.dg/tree-ssa/phi-opt-8.c (revision 185132) +++ testsuite/gcc.dg/tree-ssa/phi-opt-8.c (working copy) @@ -19,7 +19,7 @@ int f(int t, int c) but currently is not as PHI-OPT does not reduce the t PHI as we have two phis. Note this is fixed with http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01195.html . */ -/* { dg-final { scan-tree-dump-not "if" "phiopt1" { xfail *-*-* } } } */ +/* { dg-final { scan-tree-dump-not "if" "phiopt1" } } */ /* { dg-final { scan-tree-dump "g .t_\[0-9\]*.D.," "optimized" } } */ /* { dg-final { scan-tree-dump-not "PHI" "optimized" } } */ /* { dg-final { cleanup-tree-dump "phiopt1" } } */ Index: tree-ssa-phiopt.c =================================================================== --- tree-ssa-phiopt.c (revision 185132) +++ tree-ssa-phiopt.c (working copy) @@ -193,6 +193,33 @@ tree_ssa_cs_elim (void) return tree_ssa_phiopt_worker (true); } +/* Return the singleton PHI in the SEQ of PHIs for edges E0 and E1. */ + +static gimple +single_non_singleton_phi_for_edges (gimple_seq seq, edge e0, edge e1) +{ + gimple_stmt_iterator i; + gimple phi = NULL; + if (gimple_seq_singleton_p (seq)) + return gsi_stmt (gsi_start (seq)); + for (i = gsi_start (seq); !gsi_end_p (i); gsi_next (&i)) + { + gimple p = gsi_stmt (i); + /* If the PHI arguments are equal then we can skip this PHI. */ + if (operand_equal_for_phi_arg_p (gimple_phi_arg_def (p, e0->dest_idx), + gimple_phi_arg_def (p, e1->dest_idx))) + continue; + + /* If we already have a PHI that has the two edge arguments are + different, then return it is not a singleton for these PHIs. */ + if (phi) + return NULL; + + phi = p; + } + return phi; +} + /* For conditional store replacement we need a temporary to put the old contents of the memory in. */ static tree condstoretemp; @@ -316,6 +343,7 @@ tree_ssa_phiopt_worker (bool do_store_el gimple_seq phis = phi_nodes (bb2); gimple_stmt_iterator gsi; bool candorest = true; + /* Value replacement can work with more than one PHI so try that first. */ for (gsi = gsi_start (phis); !gsi_end_p (gsi); gsi_next (&gsi)) @@ -333,21 +361,8 @@ tree_ssa_phiopt_worker (bool do_store_el if (!candorest) continue; - /* Check to make sure that there is only one non-virtual PHI node. - TODO: we could do it with more than one iff the other PHI nodes - have the same elements for these two edges. */ - phi = NULL; - for (gsi = gsi_start (phis); !gsi_end_p (gsi); gsi_next (&gsi)) - { - if (!is_gimple_reg (gimple_phi_result (gsi_stmt (gsi)))) - continue; - if (phi) - { - phi = NULL; - break; - } - phi = gsi_stmt (gsi); - } + + phi = single_non_singleton_phi_for_edges (phis, e1, e2); if (!phi) continue; @@ -447,6 +462,8 @@ empty_block_p (basic_block bb) { /* BB must have no executable statements. */ gimple_stmt_iterator gsi = gsi_after_labels (bb); + if (phi_nodes (bb)) + return false; if (gsi_end_p (gsi)) return true; if (is_gimple_debug (gsi_stmt (gsi))) @@ -736,10 +753,11 @@ value_replacement (basic_block cond_bb, arg = arg1; /* If the middle basic block was empty or is defining the - PHI arguments and this is a singleton phi then we can remove - the middle basic block. */ + PHI arguments and this is a single phi where the args are different + for the edges e0 and e1 then we can remove the middle basic block. */ if (emtpy_or_with_defined_p - && gimple_seq_singleton_p (phi_nodes (gimple_bb (phi)))) + && single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)), + e0, e1)) { replace_phi_edge_with_variable (cond_bb, e1, phi, arg); /* Note that we optimized this PHI. */ @@ -754,7 +772,8 @@ value_replacement (basic_block cond_bb, { fprintf (dump_file, "PHI "); print_generic_expr (dump_file, gimple_phi_result (phi), 0); - fprintf (dump_file, " reduced for COND_EXPR in block %d to ", cond_bb->index); + fprintf (dump_file, " reduced for COND_EXPR in block %d to ", + cond_bb->index); print_generic_expr (dump_file, arg, 0); fprintf (dump_file, ".\n"); }