From patchwork Thu Jan 5 11:27:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 711352 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 3tvQRB1RQlz9t17 for ; Thu, 5 Jan 2017 22:28:21 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="ShWj62yA"; 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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=xwFteoFYdDP7h6vYe yDF2dJj46ER7K+fDfbTLBQT9ZHQE84RalPQID4UsqK8xeaLQjkVffmIH2SCKQZng P0+JGTdKC4BfULIGLvX5rGZWKRK0DWR8SbzhbbtTsh78V7oN6GREB1duX0P1QkFL +6/0Ug931kKbhCbfyKgjAOfTy8= 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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; s=default; bh=KnpyL9PmKm5rGVgJ/5MtlGQ YveA=; b=ShWj62yAWveiJHl0n2Z6ZmdlEuW4sYs4axk9puLlOeJ2eq0uN4NQK3d t/WjOQ9gEepTjvA3lWsGMIzj6x4gdfVylhSXouv13Wg6e5uwPjZDwD/xKZ1XAGTV pZOgH3VhaLHvGPSXRqqj7ISGv9ONiFSeVXpo1gYUAiOaZgizagg0= Received: (qmail 8131 invoked by alias); 5 Jan 2017 11:28:11 -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 5953 invoked by uid 89); 5 Jan 2017 11:28:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=06am, 06AM X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Jan 2017 11:28:09 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0C73080F98; Thu, 5 Jan 2017 11:28:09 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-54.ams2.redhat.com [10.36.116.54]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v05BS6Uk030219 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 5 Jan 2017 06:28:07 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v05BS2ot026072; Thu, 5 Jan 2017 12:28:03 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v05BRx89026071; Thu, 5 Jan 2017 12:27:59 +0100 Date: Thu, 5 Jan 2017 12:27:59 +0100 From: Jakub Jelinek To: Richard Biener Cc: Jeff Law , gcc-patches@gcc.gnu.org Subject: [PATCH] Tweak factor_out_conditional_conversion (PR tree-optimization/71016, take 2) Message-ID: <20170105112759.GZ21933@tucnak> Reply-To: Jakub Jelinek References: <20170102201643.GN21933@tucnak> <20170104101506.GH21933@tucnak> <20170104180124.GO21933@tucnak> <862e732b-f1c1-1f1c-bf19-4d0a0428c768@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes Hi! On Thu, Jan 05, 2017 at 09:09:06AM +0100, Richard Biener wrote: > I think that would be premature. Consider a modified > gcc.dg/tree-ssa/pr66726.c: > > extern unsigned short mode_size[]; > > int > oof (int mode) > { > int tem; > if (64 < mode_size[mode]) > tem = 64; > else > tem = mode_size[mode]; > return tem; > } You're right, that testcase still needs factor_out_*. > That we now fold the GENERIC cond-expr during gimplification > is of course good but doesn't make the phiopt code useless. > As far as I remember I objected adding handling of conversions > in the minmax replacement code and instead suggested to add this > "enablement" phiopt instead. So what I suggest is to tame that down > to the cases where it actually enables something (empty BB afterwards, > a PHI with a use that's also used on the controlling conditional). So like this (if it passes bootstrap/regtest)? 2017-01-05 Jakub Jelinek PR tree-optimization/71016 * tree-ssa-phiopt.c (tree_ssa_phiopt_worker): Pass cond_stmt to factor_out_conditional_conversion. Formatting fix. (factor_out_conditional_conversion): Add cond_stmt argument. If arg1 is INTEGER_CST, punt if new_arg0 is not any operand of cond_stmt and if arg0_def_stmt is not the only stmt in its bb. * gcc.target/i386/pr71016.c: New test. * gcc.target/aarch64/pr71016.c: New test. * gcc.dg/tree-ssa/pr66726-3.c: New test. Jakub --- gcc/tree-ssa-phiopt.c.jj 2017-01-03 08:12:27.000000000 +0100 +++ gcc/tree-ssa-phiopt.c 2017-01-05 12:12:09.571017488 +0100 @@ -49,7 +49,8 @@ along with GCC; see the file COPYING3. static unsigned int tree_ssa_phiopt_worker (bool, bool); static bool conditional_replacement (basic_block, basic_block, edge, edge, gphi *, tree, tree); -static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree); +static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree, + gimple *); static int value_replacement (basic_block, basic_block, edge, edge, gimple *, tree, tree); static bool minmax_replacement (basic_block, basic_block, @@ -233,7 +234,7 @@ tree_ssa_phiopt_worker (bool do_store_el continue; } else if (do_hoist_loads - && EDGE_SUCC (bb1, 0)->dest == EDGE_SUCC (bb2, 0)->dest) + && EDGE_SUCC (bb1, 0)->dest == EDGE_SUCC (bb2, 0)->dest) { basic_block bb3 = EDGE_SUCC (bb1, 0)->dest; @@ -313,7 +314,8 @@ tree_ssa_phiopt_worker (bool do_store_el gcc_assert (arg0 != NULL_TREE && arg1 != NULL_TREE); gphi *newphi = factor_out_conditional_conversion (e1, e2, phi, - arg0, arg1); + arg0, arg1, + cond_stmt); if (newphi != NULL) { phi = newphi; @@ -402,11 +404,12 @@ replace_phi_edge_with_variable (basic_bl /* PR66726: Factor conversion out of COND_EXPR. If the arguments of the PHI stmt are CONVERT_STMT, factor out the conversion and perform the conversion - to the result of PHI stmt. Return the newly-created PHI, if any. */ + to the result of PHI stmt. COND_STMT is the controlling predicate. + Return the newly-created PHI, if any. */ static gphi * factor_out_conditional_conversion (edge e0, edge e1, gphi *phi, - tree arg0, tree arg1) + tree arg0, tree arg1, gimple *cond_stmt) { gimple *arg0_def_stmt = NULL, *arg1_def_stmt = NULL, *new_stmt; tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE; @@ -472,7 +475,31 @@ factor_out_conditional_conversion (edge && int_fits_type_p (arg1, TREE_TYPE (new_arg0))) { if (gimple_assign_cast_p (arg0_def_stmt)) - new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1); + { + /* For the INTEGER_CST case, we are just moving the + conversion from one place to another, which can often + hurt as the conversion moves further away from the + statement that computes the value. So, perform this + only if new_arg0 is an operand of COND_STMT, or + if arg0_def_stmt is the only non-debug stmt in + its basic block, because then it is possible this + could enable further optimizations (minmax replacement + etc.). See PR71016. */ + if (new_arg0 != gimple_cond_lhs (cond_stmt) + && new_arg0 != gimple_cond_rhs (cond_stmt) + && gimple_bb (arg0_def_stmt) == e0->src) + { + gsi = gsi_for_stmt (arg0_def_stmt); + gsi_prev_nondebug (&gsi); + if (!gsi_end_p (gsi)) + return NULL; + gsi = gsi_for_stmt (arg0_def_stmt); + gsi_next_nondebug (&gsi); + if (!gsi_end_p (gsi)) + return NULL; + } + new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1); + } else return NULL; } --- gcc/testsuite/gcc.target/i386/pr71016.c.jj 2017-01-05 12:06:33.808325995 +0100 +++ gcc/testsuite/gcc.target/i386/pr71016.c 2017-01-05 12:07:54.242293867 +0100 @@ -0,0 +1,10 @@ +/* PR tree-optimization/71016 */ +/* { dg-do compile { target lp64 } } */ +/* { dg-options "-O2 -mlzcnt" } */ +/* { dg-final { scan-assembler-not "cltq" } } */ + +long int +foo (long int i) +{ + return i == 0 ? 17 : __builtin_clzl (i); +} --- gcc/testsuite/gcc.target/aarch64/pr71016.c.jj 2017-01-05 12:09:12.075295113 +0100 +++ gcc/testsuite/gcc.target/aarch64/pr71016.c 2017-01-05 12:09:28.479084620 +0100 @@ -0,0 +1,10 @@ +/* PR tree-optimization/71016 */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +/* { dg-final { scan-assembler-not "sxtw" } } */ + +long int +foo (long int i) +{ + return i == 0 ? 17 : __builtin_clzl (i); +} --- gcc/testsuite/gcc.dg/tree-ssa/pr66726-3.c.jj 2017-01-05 12:10:07.746580739 +0100 +++ gcc/testsuite/gcc.dg/tree-ssa/pr66726-3.c 2017-01-05 11:41:59.000000000 +0100 @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "optimized" } } */ + +extern unsigned short mode_size[]; + +int +oof (int mode) +{ + int tem; + if (64 < mode_size[mode]) + tem = 64; + else + tem = mode_size[mode]; + return tem; +}