From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26515 invoked by alias); 4 Jul 2015 13:21:52 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 26504 invoked by uid 89); 4 Jul 2015 13:21:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f182.google.com Received: from mail-wi0-f182.google.com (HELO mail-wi0-f182.google.com) (209.85.212.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 04 Jul 2015 13:21:50 +0000 Received: by wibdq8 with SMTP id dq8so118883260wib.1 for ; Sat, 04 Jul 2015 06:21:47 -0700 (PDT) X-Received: by 10.181.11.165 with SMTP id ej5mr36759521wid.32.1436016107256; Sat, 04 Jul 2015 06:21:47 -0700 (PDT) Received: from [10.56.84.22] (089144216022.atnat0025.highway.a1.net. [89.144.216.22]) by mx.google.com with ESMTPSA id p2sm18541158wix.11.2015.07.04.06.21.45 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 04 Jul 2015 06:21:46 -0700 (PDT) User-Agent: K-9 Mail for Android In-Reply-To: <5597D24B.8010900@linaro.org> References: <55974BF2.3060603@linaro.org> <20150704085143.GA14895@nbbrfq.cc.univie.ac.at> <5597D24B.8010900@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: [PR66726] Factor conversion out of COND_EXPR From: Bernhard Reutner-Fischer Date: Sat, 04 Jul 2015 13:21:00 -0000 To: Kugan CC: "gcc-patches@gcc.gnu.org" ,Jeff Law Message-ID: <66DA0BC5-457E-4CFA-9735-5BB75AB251A5@gmail.com> X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00248.txt.bz2 On July 4, 2015 2:32:11 PM GMT+02:00, Kugan wrote: >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. I meant in addition, just to be sure. Patch itself looks plausible to me but I cannot approve it. Thanks, > >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 >>> ---------------------- >>> >>