From patchwork Sat Jul 4 12:32:11 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 50655 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f70.google.com (mail-wg0-f70.google.com [74.125.82.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 26404229E5 for ; Sat, 4 Jul 2015 12:32:41 +0000 (UTC) Received: by wgbbj7 with SMTP id bj7sf37285633wgb.2 for ; Sat, 04 Jul 2015 05:32:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:sender :delivered-to:message-id:date:from:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type:x-original-sender :x-original-authentication-results; bh=IrWQnAk0TEIXQsOAKGCyHREtGLkryR1+YRU8X0sm6oo=; b=A42NrA4RUK/OEQGu/xBlfdMZFRl0pviQEoBAchi4K5d5M7HlSP5snQIorwLEBmFNXk MC29cuEH1n70sh4ofhQLvJuTv7Y9+OwzcTEXhYGp/qrBXbSdvABVQZtbqZq2ZZk+Zem4 KKNH8J4qK5G/fufGommM0mBrquZAp7XWIu6qETsx0LRmn9WIFnqW4iyqFeJ5e8DPbfn7 aZcTcBw3W/idB4iv+dMKVvP21NztRTD5Z0oFGXp5oK87/chGYUtYEosYZTSp5NbSZhBd VGG6H+FojP86AlxrWn7b+vIF9U0Xt8OLQzGSfB0hB0227fyvrXd68TaFvW2yUdEcSOW1 qx8Q== X-Gm-Message-State: ALoCoQlqiGj7yUK4owAFrc4i4PpxqN292tU1CyiXB4y8zHSMKZ5F6d6o7blUj9aOSwODZHuBeuZu X-Received: by 10.112.138.2 with SMTP id qm2mr25249611lbb.19.1436013159815; Sat, 04 Jul 2015 05:32:39 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.204.103 with SMTP id kx7ls578106lac.62.gmail; Sat, 04 Jul 2015 05:32:39 -0700 (PDT) X-Received: by 10.152.87.177 with SMTP id az17mr2687598lab.14.1436013159418; Sat, 04 Jul 2015 05:32:39 -0700 (PDT) Received: from mail-la0-x22b.google.com (mail-la0-x22b.google.com. [2a00:1450:4010:c03::22b]) by mx.google.com with ESMTPS id o5si9975636lae.1.2015.07.04.05.32.38 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 04 Jul 2015 05:32:38 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::22b as permitted sender) client-ip=2a00:1450:4010:c03::22b; Received: by lagx9 with SMTP id x9so109672995lag.1 for ; Sat, 04 Jul 2015 05:32:38 -0700 (PDT) X-Received: by 10.152.206.75 with SMTP id lm11mr39370758lac.41.1436013158424; Sat, 04 Jul 2015 05:32:38 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp623837lbb; Sat, 4 Jul 2015 05:32:36 -0700 (PDT) X-Received: by 10.66.234.101 with SMTP id ud5mr45795956pac.23.1436013155847; Sat, 04 Jul 2015 05:32:35 -0700 (PDT) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id z14si19367080pdi.58.2015.07.04.05.32.34 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 04 Jul 2015 05:32:35 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-401985-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 82415 invoked by alias); 4 Jul 2015 12:32:22 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list 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 82403 invoked by uid 89); 4 Jul 2015 12:32:22 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pa0-f44.google.com Received: from mail-pa0-f44.google.com (HELO mail-pa0-f44.google.com) (209.85.220.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 04 Jul 2015 12:32:20 +0000 Received: by pacws9 with SMTP id ws9so71496990pac.0 for ; Sat, 04 Jul 2015 05:32:18 -0700 (PDT) X-Received: by 10.70.90.232 with SMTP id bz8mr85697138pdb.120.1436013138147; Sat, 04 Jul 2015 05:32:18 -0700 (PDT) Received: from [10.1.1.8] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by mx.google.com with ESMTPSA id md10sm3306733pdb.33.2015.07.04.05.32.15 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 04 Jul 2015 05:32:17 -0700 (PDT) Message-ID: <5597D24B.8010900@linaro.org> Date: Sat, 04 Jul 2015 22:32:11 +1000 From: Kugan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Bernhard Reutner-Fischer CC: "gcc-patches@gcc.gnu.org" , Jeff Law Subject: Re: [PR66726] Factor conversion out of COND_EXPR References: <55974BF2.3060603@linaro.org> <20150704085143.GA14895@nbbrfq.cc.univie.ac.at> In-Reply-To: <20150704085143.GA14895@nbbrfq.cc.univie.ac.at> X-IsSubscribed: yes X-Original-Sender: kugan.vivekanandarajah@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::22b as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@gcc.gnu.org X-Google-Group-Id: 836684582541 On 04/07/15 18:51, Bernhard Reutner-Fischer wrote: > On Sat, Jul 04, 2015 at 12:58:58PM +1000, Kugan wrote: >> Please find a patch that attempt to FIX PR66726 by factoring conversion >> out of COND_EXPR as explained in the PR. >> >> Bootstrapped and regression tested on x86-64-none-linux-gnu with no new >> regressions. Is this OK for trunk? >> >> Thanks, >> Kugan >> >> >> gcc/testsuite/ChangeLog: >> >> 2015-07-03 Kugan Vivekanandarajah >> Jeff Law >> >> PR middle-end/66726 >> * gcc.dg/tree-ssa/pr66726.c: New test. > > I'd have scanned the details dump for "factor CONVERT_EXPR out" 1 > to make sure that it's this part that takes care of it. Thanks for the comments. Please see the updated patch with the fixes. Kugan > >> >> gcc/ChangeLog: >> >> 2015-07-03 Kugan Vivekanandarajah >> >> PR middle-end/66726 >> * tree-ssa-phiopt.c (factor_out_conditional_conversion): New function. >> (tree_ssa_phiopt_worker): Call factor_out_conditional_conversion. > >> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c >> index d2a5cee..e8af086 100644 >> --- a/gcc/tree-ssa-phiopt.c >> +++ b/gcc/tree-ssa-phiopt.c > >> @@ -410,6 +413,108 @@ replace_phi_edge_with_variable (basic_block cond_block, >> bb->index); >> } >> >> +/* PR66726: Factor conversion out of COND_EXPR. If the argument of the PHI > > s/the argument/the arguments/ > >> + stmt are CONVERT_STMT, factor out the conversion and perform the conversion >> + to the result of PHI stmt. */ >> + >> +static bool >> +factor_out_conditional_conversion (edge e0, edge e1, gphi *phi, >> + tree arg0, tree arg1) >> +{ >> + gimple def0 = NULL, def1 = NULL, new_stmt; >> + tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE; >> + tree temp, result; >> + gimple_stmt_iterator gsi; >> + >> + /* One of the argument has to be SSA_NAME and other argument can > > s/the argument/the arguments/ > >> + be an SSA_NAME of INTEGER_CST. */ >> + if ((TREE_CODE (arg0) != SSA_NAME >> + && TREE_CODE (arg0) != INTEGER_CST) >> + || (TREE_CODE (arg1) != SSA_NAME >> + && TREE_CODE (arg1) != INTEGER_CST) >> + || (TREE_CODE (arg0) == INTEGER_CST >> + && TREE_CODE (arg1) == INTEGER_CST)) > > inconsistent space for the && lines above; The first should have a > leading tab. > >> + return false; >> + >> + /* Handle only PHI statements with two arguments. TODO: If all >> + other arguments to PHI are INTEGER_CST, we can handle more >> + than two arguments too. */ >> + if (gimple_phi_num_args (phi) != 2) >> + return false; >> + >> + /* If arg0 is an SSA_NAME and the stmt which defines arg0 is >> + ai CONVERT_STMT, use the LHS as new_arg0. */ > > s/ai/a/ > >> + if (TREE_CODE (arg0) == SSA_NAME) >> + { >> + def0 = SSA_NAME_DEF_STMT (arg0); >> + if (!is_gimple_assign (def0) >> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def0))) >> + return false; >> + new_arg0 = gimple_assign_rhs1 (def0); >> + } >> + >> + /* If arg1 is an SSA_NAME and the stmt which defines arg0 is >> + ai CONVERT_STMT, use the LHS as new_arg1. */ > > s/ai/a/ > >> + if (TREE_CODE (arg1) == SSA_NAME) >> + { >> + def1 = SSA_NAME_DEF_STMT (arg1); >> + if (!is_gimple_assign (def1) >> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def1))) >> + return false; >> + new_arg1 = gimple_assign_rhs1 (def1); >> + } >> + >> + /* If arg0 is an INTEGER_CST, fold it to new type. */ >> + if (TREE_CODE (arg0) != SSA_NAME) >> + { >> + if (!POINTER_TYPE_P (TREE_TYPE (new_arg1)) >> + && int_fits_type_p (arg0, TREE_TYPE (new_arg1))) >> + new_arg0 = fold_convert (TREE_TYPE (new_arg1), arg0); >> + else >> + return false; >> + } >> + >> + /* If arg1 is an INTEGER_CST, fold it to new type. */ >> + if (TREE_CODE (arg1) != SSA_NAME) >> + { >> + if (!POINTER_TYPE_P (TREE_TYPE (new_arg0)) >> + && int_fits_type_p (arg1, TREE_TYPE (new_arg0))) >> + new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1); >> + else >> + return false; >> + } >> + >> + /* If types of new_arg0 and new_arg1 are different bailout. */ >> + if (TREE_TYPE (new_arg0) != TREE_TYPE (new_arg1)) >> + return false; >> + >> + /* Replace the PHI stmt with the new_arg0 and new_arg1. Also insert >> + a new CONVER_STMT that converts the phi results. */ > > s/CONVER_STMT/CONVERT_STMT/ > >> + gsi = gsi_after_labels (gimple_bb (phi)); >> + result = PHI_RESULT (phi); >> + temp = make_ssa_name (TREE_TYPE (new_arg0), phi); >> + >> + if (dump_file && (dump_flags & TDF_DETAILS)) >> + { >> + fprintf (dump_file, "PHI "); >> + print_generic_expr (dump_file, gimple_phi_result (phi), 0); >> + fprintf (dump_file, >> + " changed to factor CONVERT_EXPR out from COND_EXPR.\n"); >> + fprintf (dump_file, "New PHI_RESULT is "); >> + print_generic_expr (dump_file, temp, 0); >> + fprintf (dump_file, " and new stmt with CONVERT_EXPR defines "); >> + print_generic_expr (dump_file, result, 0); >> + fprintf (dump_file, ".\n"); >> + } >> + >> + gimple_phi_set_result (phi, temp); >> + SET_PHI_ARG_DEF (phi, e0->dest_idx, new_arg0); >> + SET_PHI_ARG_DEF (phi, e1->dest_idx, new_arg1); >> + new_stmt = gimple_build_assign (result, CONVERT_EXPR, temp); >> + gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); >> + return true; >> +} >> + >> /* The function conditional_replacement does the main work of doing the >> conditional replacement. Return true if the replacement is done. >> Otherwise return false. >> @@ -2144,6 +2249,26 @@ gate_hoist_loads (void) >> This pass also performs a fifth transformation of a slightly different >> flavor. >> >> + Factor conversion in COND_EXPR >> + ---------------------------------- > > excess trailing "----" ;) > > thanks, >> + >> + This transformation factors the conversion out of COND_EXPR with >> + factor_out_conditional_conversion. >> + >> + For example: >> + if (a <= CST) goto ; else goto ; >> + : >> + tmp = (int) a; >> + : >> + tmp = PHI >> + >> + Into: >> + if (a <= CST) goto ; else goto ; >> + : >> + : >> + a = PHI >> + tmp = (int) a; >> + >> Adjacent Load Hoisting >> ---------------------- >> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c index e69de29..93f1ace 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c @@ -0,0 +1,13 @@ + +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-phiopt1-details" } */ + +extern unsigned short mode_size[]; +int +oof (int mode) +{ + return (64 < mode_size[mode] ? 64 : mode_size[mode]); +} + +/* { dg-final { scan-tree-dump-times "factor CONVERT_EXPR out" 1 "phiopt1" } } */ + diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c index d2a5cee..12ab9ee 100644 --- a/gcc/tree-ssa-phiopt.c +++ b/gcc/tree-ssa-phiopt.c @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see static unsigned int tree_ssa_phiopt_worker (bool, bool); static bool conditional_replacement (basic_block, basic_block, edge, edge, gphi *, tree, tree); +static bool factor_out_conditional_conversion (edge, edge, gphi *, tree, tree); static int value_replacement (basic_block, basic_block, edge, edge, gimple, tree, tree); static bool minmax_replacement (basic_block, basic_block, @@ -342,6 +343,8 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads) cfgchanged = true; else if (minmax_replacement (bb, bb1, e1, e2, phi, arg0, arg1)) cfgchanged = true; + else if (factor_out_conditional_conversion (e1, e2, phi, arg0, arg1)) + cfgchanged = true; } } @@ -410,6 +413,108 @@ replace_phi_edge_with_variable (basic_block cond_block, bb->index); } +/* 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. */ + +static bool +factor_out_conditional_conversion (edge e0, edge e1, gphi *phi, + tree arg0, tree arg1) +{ + gimple def0 = NULL, def1 = NULL, new_stmt; + tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE; + tree temp, result; + gimple_stmt_iterator gsi; + + /* One of the arguments has to be SSA_NAME and other argument can + be an SSA_NAME of INTEGER_CST. */ + if ((TREE_CODE (arg0) != SSA_NAME + && TREE_CODE (arg0) != INTEGER_CST) + || (TREE_CODE (arg1) != SSA_NAME + && TREE_CODE (arg1) != INTEGER_CST) + || (TREE_CODE (arg0) == INTEGER_CST + && TREE_CODE (arg1) == INTEGER_CST)) + return false; + + /* Handle only PHI statements with two arguments. TODO: If all + other arguments to PHI are INTEGER_CST, we can handle more + than two arguments too. */ + if (gimple_phi_num_args (phi) != 2) + return false; + + /* If arg0 is an SSA_NAME and the stmt which defines arg0 is + a CONVERT_STMT, use the LHS as new_arg0. */ + if (TREE_CODE (arg0) == SSA_NAME) + { + def0 = SSA_NAME_DEF_STMT (arg0); + if (!is_gimple_assign (def0) + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def0))) + return false; + new_arg0 = gimple_assign_rhs1 (def0); + } + + /* If arg1 is an SSA_NAME and the stmt which defines arg0 is + a CONVERT_STMT, use the LHS as new_arg1. */ + if (TREE_CODE (arg1) == SSA_NAME) + { + def1 = SSA_NAME_DEF_STMT (arg1); + if (!is_gimple_assign (def1) + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def1))) + return false; + new_arg1 = gimple_assign_rhs1 (def1); + } + + /* If arg0 is an INTEGER_CST, fold it to new type. */ + if (TREE_CODE (arg0) != SSA_NAME) + { + if (!POINTER_TYPE_P (TREE_TYPE (new_arg1)) + && int_fits_type_p (arg0, TREE_TYPE (new_arg1))) + new_arg0 = fold_convert (TREE_TYPE (new_arg1), arg0); + else + return false; + } + + /* If arg1 is an INTEGER_CST, fold it to new type. */ + if (TREE_CODE (arg1) != SSA_NAME) + { + if (!POINTER_TYPE_P (TREE_TYPE (new_arg0)) + && int_fits_type_p (arg1, TREE_TYPE (new_arg0))) + new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1); + else + return false; + } + + /* If types of new_arg0 and new_arg1 are different bailout. */ + if (TREE_TYPE (new_arg0) != TREE_TYPE (new_arg1)) + return false; + + /* Replace the PHI stmt with the new_arg0 and new_arg1. Also insert + a new CONVERT_STMT that converts the phi results. */ + gsi = gsi_after_labels (gimple_bb (phi)); + result = PHI_RESULT (phi); + temp = make_ssa_name (TREE_TYPE (new_arg0), phi); + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "PHI "); + print_generic_expr (dump_file, gimple_phi_result (phi), 0); + fprintf (dump_file, + " changed to factor CONVERT_EXPR out from COND_EXPR.\n"); + fprintf (dump_file, "New PHI_RESULT is "); + print_generic_expr (dump_file, temp, 0); + fprintf (dump_file, " and new stmt with CONVERT_EXPR defines "); + print_generic_expr (dump_file, result, 0); + fprintf (dump_file, ".\n"); + } + + gimple_phi_set_result (phi, temp); + SET_PHI_ARG_DEF (phi, e0->dest_idx, new_arg0); + SET_PHI_ARG_DEF (phi, e1->dest_idx, new_arg1); + new_stmt = gimple_build_assign (result, CONVERT_EXPR, temp); + gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); + return true; +} + /* The function conditional_replacement does the main work of doing the conditional replacement. Return true if the replacement is done. Otherwise return false. @@ -2144,6 +2249,26 @@ gate_hoist_loads (void) This pass also performs a fifth transformation of a slightly different flavor. + Factor conversion in COND_EXPR + ------------------------------ + + This transformation factors the conversion out of COND_EXPR with + factor_out_conditional_conversion. + + For example: + if (a <= CST) goto ; else goto ; + : + tmp = (int) a; + : + tmp = PHI + + Into: + if (a <= CST) goto ; else goto ; + : + : + a = PHI + tmp = (int) a; + Adjacent Load Hoisting ----------------------