From patchwork Mon Sep 2 09:33:12 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 19671 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qc0-f199.google.com (mail-qc0-f199.google.com [209.85.216.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id BCDBC248D9 for ; Mon, 2 Sep 2013 09:33:20 +0000 (UTC) Received: by mail-qc0-f199.google.com with SMTP id n7sf5420276qcx.2 for ; Mon, 02 Sep 2013 02:33:20 -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:message-id:date:from:user-agent :mime-version:to:cc:subject:references:in-reply-to:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type; bh=No2xe9ltF16EqxMkD7bT6SBfPJlGyDtG97SBOzo+Pgc=; b=fcGpmTO30BCgSJm9IbD2VPnUC9K/ZFtDaKUnl6BaMo1dBaLLanIoVq+3kW+FUyHXuT P9qOmbkCpvBmUsqEvEhU902jI/8lZraos21QF4S6cSPHqhZcZt/AzwfYaNqvfNKK+p8y TlcaD69Wu4AIUadmZ/hYo8ZkunxEI1MGQgk5X3uwXW/ffsyjmig9Hw6BnAf1xSVwrV54 HJbQbwAIflHFQgDFXvImVZA1HGTyKOXQAs5a/DusRPUi8R7Boidqs7MR4r4YuxmE/W8l XtnQPuGBQzhhZo9ZBdYKszEqfHQ6JiwGpOsyA6R2YfA9Lt3o0KUwGBes8oQ3bR6iB/5v FWTw== X-Received: by 10.236.66.244 with SMTP id h80mr7910219yhd.30.1378114400187; Mon, 02 Sep 2013 02:33:20 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.47.82 with SMTP id b18ls2094504qen.53.gmail; Mon, 02 Sep 2013 02:33:20 -0700 (PDT) X-Received: by 10.221.40.10 with SMTP id to10mr2066934vcb.22.1378114400071; Mon, 02 Sep 2013 02:33:20 -0700 (PDT) Received: from mail-vb0-f51.google.com (mail-vb0-f51.google.com [209.85.212.51]) by mx.google.com with ESMTPS id tj1si2935594vdc.92.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 02 Sep 2013 02:33:20 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.212.51 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.212.51; Received: by mail-vb0-f51.google.com with SMTP id x16so2781920vbf.38 for ; Mon, 02 Sep 2013 02:33:20 -0700 (PDT) X-Gm-Message-State: ALoCoQkgCLIfVIusP4n0sLAg403zLssquhRtj4xUnNczKnTthZlBX1X1/RZkjyK0XsMhw/frkZDE X-Received: by 10.220.1.203 with SMTP id 11mr16982105vcg.15.1378114399976; Mon, 02 Sep 2013 02:33:19 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp104872vcz; Mon, 2 Sep 2013 02:33:19 -0700 (PDT) X-Received: by 10.66.120.74 with SMTP id la10mr25725599pab.9.1378114398429; Mon, 02 Sep 2013 02:33:18 -0700 (PDT) Received: from mail-pd0-f175.google.com (mail-pd0-f175.google.com [209.85.192.175]) by mx.google.com with ESMTPS id hi6si10202278pac.113.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 02 Sep 2013 02:33:18 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.192.175 is neither permitted nor denied by best guess record for domain of kugan.vivekanandarajah@linaro.org) client-ip=209.85.192.175; Received: by mail-pd0-f175.google.com with SMTP id q10so4512692pdj.20 for ; Mon, 02 Sep 2013 02:33:17 -0700 (PDT) X-Received: by 10.68.132.67 with SMTP id os3mr968712pbb.188.1378114397884; Mon, 02 Sep 2013 02:33:17 -0700 (PDT) Received: from [192.168.1.6] (27-33-114-215.tpgi.com.au. [27.33.114.215]) by mx.google.com with ESMTPSA id yg3sm15890008pab.16.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 02 Sep 2013 02:33:17 -0700 (PDT) Message-ID: <52245B58.6090507@linaro.org> Date: Mon, 02 Sep 2013 19:03:12 +0930 From: Kugan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130803 Thunderbird/17.0.8 MIME-Version: 1.0 To: Eric Botcazou CC: gcc-patches@gcc.gnu.org, patches@linaro.org, Richard Biener , rdsandiford@googlemail.com, Richard Earnshaw , ramana.radhakrishnan@arm.com Subject: Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP References: <51ABFC6E.30205@linaro.org> <1726629.C2vZH2NXuZ@polaris> <520B31F5.7020200@linaro.org> In-Reply-To: <520B31F5.7020200@linaro.org> X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: kugan.vivekanandarajah@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.212.51 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , I'd like to ping this patch 2 of 2 that removes redundant zero/sign extension using value range information. Bootstrapped and no new regression for x86_64-unknown-linux-gnu and arm-none-linux-gnueabi. Thanks you for your time. Kugan On 14/08/13 16:59, Kugan wrote: > Hi Eric, > > Thanks for reviewing the patch. > > On 01/07/13 18:51, Eric Botcazou wrote: >> [Sorry for the delay] >> >>> For example, when an expression is evaluated and it's value is assigned >>> to variable of type short, the generated RTL would look something like >>> the following. >>> >>> (set (reg:SI 110) >>> (zero_extend:SI (subreg:HI (reg:SI 117) 0))) >>> >>> However, if during value range propagation, if we can say for certain >>> that the value of the expression which is present in register 117 is >>> within the limits of short and there is no sign conversion, we do not >>> need to perform the subreg and zero_extend; instead we can generate the >>> following RTl. >>> >>> (set (reg:SI 110) >>> (reg:SI 117))) >>> >>> Same could be done for other assign statements. >> >> The idea looks interesting. Some remarks: >> >>> +2013-06-03 Kugan Vivekanandarajah >>> + >>> + * gcc/dojump.c (do_compare_and_jump): generates rtl without >>> + zero/sign extension if redundant. >>> + * gcc/cfgexpand.c (expand_gimple_stmt_1): Likewise. >>> + * gcc/gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New >>> + function. >>> + * gcc/gimple.h (gimple_assign_is_zero_sign_ext_redundant) : New >>> + function definition. >> >> No gcc/ prefix in entries for gcc/ChangeLog. "Generate RTL without..." >> > I have now changed it to. > > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,13 @@ > +2013-08-14 Kugan Vivekanandarajah > + > + * dojump.c (do_compare_and_jump): Generate rtl without > + zero/sign extension if redundant. > + * cfgexpand.c (expand_gimple_stmt_1): Likewise. > + * gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New > + function. > + * gimple.h (gimple_assign_is_zero_sign_ext_redundant) : New > + function definition. > + > 2013-08-09 Jan Hubicka > > * cgraph.c (cgraph_create_edge_1): Clear speculative flag. > >> >> + /* If the value in SUBREG of temp fits that SUBREG (does not >> + overflow) and is assigned to target SUBREG of the same >> mode >> + without sign convertion, we can skip the SUBREG >> + and extension. */ >> + else if (promoted >> + && gimple_assign_is_zero_sign_ext_redundant (stmt) >> + && (GET_CODE (temp) == SUBREG) >> + && (GET_MODE (target) == GET_MODE (temp)) >> + && (GET_MODE (SUBREG_REG (target)) >> + == GET_MODE (SUBREG_REG (temp)))) >> + emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp)); >> else if (promoted) >> { >> int unsignedp = SUBREG_PROMOTED_UNSIGNED_P (target); >> >> Can we relax the strict mode equality here? This change augments the >> same >> transformation applied to the RHS when it is also a >> SUBREG_PROMOTED_VAR_P at >> the beginning of convert_move, but the condition on the mode is less >> strict in >> the latter case, so maybe it can serve as a model here. >> > > I have now changed it based on convert_move to > + else if (promoted > + && gimple_assign_is_zero_sign_ext_redundant (stmt) > + && (GET_CODE (temp) == SUBREG) > + && (GET_MODE_PRECISION (GET_MODE (SUBREG_REG (temp))) > + >= GET_MODE_PRECISION (GET_MODE (target))) > + && (GET_MODE (SUBREG_REG (target)) > + == GET_MODE (SUBREG_REG (temp)))) > + { > > Is this what you wanted me to do. >> >> + /* Is zero/sign extension redundant as per VRP. */ >> + bool op0_ext_redundant = false; >> + bool op1_ext_redundant = false; >> + >> + /* If promoted and the value in SUBREG of op0 fits (does not >> overflow), >> + it is a candidate for extension elimination. */ >> + if (GET_CODE (op0) == SUBREG && SUBREG_PROMOTED_VAR_P (op0)) >> + op0_ext_redundant = >> + gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT >> (treeop0)); >> + >> + /* If promoted and the value in SUBREG of op1 fits (does not >> overflow), >> + it is a candidate for extension elimination. */ >> + if (GET_CODE (op1) == SUBREG && SUBREG_PROMOTED_VAR_P (op1)) >> + op1_ext_redundant = >> + gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT >> (treeop1)); >> >> Are the gimple_assign_is_zero_sign_ext_redundant checks necessary here? >> When set on a SUBREG, SUBREG_PROMOTED_VAR_P guarantees that SUBREG_REG is >> always properly extended (otherwise it's a bug) so don't you just need to >> compare SUBREG_PROMOTED_UNSIGNED_SET? See do_jump for an existing case. >> > I am sorry I don’t think I understood you here. How would I know that > extension is redundant without calling > gimple_assign_is_zero_sign_ext_redundant ? Could you please elaborate. > >> >> + /* If zero/sign extension is redundant, generate RTL >> + for operands without zero/sign extension. */ >> + if ((op0_ext_redundant || TREE_CODE (treeop0) == INTEGER_CST) >> + && (op1_ext_redundant || TREE_CODE (treeop1) == INTEGER_CST)) >> >> Don't you need to be careful with the INTEGER_CSTs here? The >> CONST_INTs are >> always sign-extended in RTL so 0x80 is always represented by >> (const_int -128) >> in QImode, whatever the signedness. If SUBREG_PROMOTED_UNSIGNED_SET >> is true, >> then comparing in QImode and comparing in e.g. SImode wouldn't be >> equivalent. >> > > I have changed this. > > I have attached the modified patch your your review. > > Thanks, > Kugan diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index a7d9170..8f08cc2 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -2311,6 +2311,20 @@ expand_gimple_stmt_1 (gimple stmt) if (temp == target) ; + /* If the value in SUBREG of temp fits that SUBREG (does not + overflow) and is assigned to target SUBREG of the same mode + without sign convertion, we can skip the SUBREG + and extension. */ + else if (promoted + && gimple_assign_is_zero_sign_ext_redundant (stmt) + && (GET_CODE (temp) == SUBREG) + && (GET_MODE_PRECISION (GET_MODE (SUBREG_REG (temp))) + >= GET_MODE_PRECISION (GET_MODE (target))) + && (GET_MODE (SUBREG_REG (target)) + == GET_MODE (SUBREG_REG (temp)))) + { + emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp)); + } else if (promoted) { int unsignedp = SUBREG_PROMOTED_UNSIGNED_P (target); diff --git a/gcc/dojump.c b/gcc/dojump.c index 3f04eac..639d38f 100644 --- a/gcc/dojump.c +++ b/gcc/dojump.c @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see #include "ggc.h" #include "basic-block.h" #include "tm_p.h" +#include "gimple.h" static bool prefer_and_bit_test (enum machine_mode, int); static void do_jump_by_parts_greater (tree, tree, int, rtx, rtx, int); @@ -1108,6 +1109,61 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code, type = TREE_TYPE (treeop0); mode = TYPE_MODE (type); + + /* Is zero/sign extension redundant as per VRP. */ + bool op0_ext_redundant = false; + bool op1_ext_redundant = false; + + /* If promoted and the value in SUBREG of op0 fits (does not overflow), + it is a candidate for extension elimination. */ + if (GET_CODE (op0) == SUBREG && SUBREG_PROMOTED_VAR_P (op0)) + op0_ext_redundant = + gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT (treeop0)); + + /* If promoted and the value in SUBREG of op1 fits (does not overflow), + it is a candidate for extension elimination. */ + if (GET_CODE (op1) == SUBREG && SUBREG_PROMOTED_VAR_P (op1)) + op1_ext_redundant = + gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT (treeop1)); + + /* If zero/sign extension is redundant, generate RTL + for operands without zero/sign extension. */ + if ((op0_ext_redundant || TREE_CODE (treeop0) == INTEGER_CST) + && (op1_ext_redundant || TREE_CODE (treeop1) == INTEGER_CST)) + { + if ((TREE_CODE (treeop1) == INTEGER_CST) + && (!mode_signbit_p (GET_MODE (op0), op1))) + { + /* First operand is constant. */ + rtx new_op0 = gen_reg_rtx (GET_MODE (SUBREG_REG (op0))); + emit_move_insn (new_op0, SUBREG_REG (op0)); + op0 = new_op0; + } + else if ((TREE_CODE (treeop0) == INTEGER_CST) + && (!mode_signbit_p (GET_MODE (op1), op0))) + { + /* Other operand is constant. */ + rtx new_op1 = gen_reg_rtx (GET_MODE (SUBREG_REG (op1))); + + emit_move_insn (new_op1, SUBREG_REG (op1)); + op1 = new_op1; + } + /* If both the comapre registers fits SUBREG and of the same mode. */ + else if ((TREE_CODE (treeop0) != INTEGER_CST) + && (TREE_CODE (treeop1) != INTEGER_CST) + && (GET_MODE (op0) == GET_MODE (op1)) + && (GET_MODE (SUBREG_REG (op0)) == GET_MODE (SUBREG_REG (op1)))) + { + rtx new_op0 = gen_reg_rtx (GET_MODE (SUBREG_REG (op0))); + rtx new_op1 = gen_reg_rtx (GET_MODE (SUBREG_REG (op1))); + + emit_move_insn (new_op0, SUBREG_REG (op0)); + emit_move_insn (new_op1, SUBREG_REG (op1)); + op0 = new_op0; + op1 = new_op1; + } + } + if (TREE_CODE (treeop0) == INTEGER_CST && (TREE_CODE (treeop1) != INTEGER_CST || (GET_MODE_BITSIZE (mode) diff --git a/gcc/gimple.c b/gcc/gimple.c index f507419..17d90ee 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -200,6 +200,75 @@ gimple_call_reset_alias_info (gimple s) pt_solution_reset (gimple_call_clobber_set (s)); } + +/* process gimple assign stmts and see if the sign/zero extensions are + redundant. i.e. if an assignment gimple statement has RHS expression + value that can fit in LHS type, truncation is redundant. Zero/sign + extensions in this case can be removed. */ +bool +gimple_assign_is_zero_sign_ext_redundant (gimple stmt) +{ + double_int type_min, type_max; + tree int_val = NULL_TREE; + range_info_def *ri; + + /* skip if not assign stmt. */ + if (!is_gimple_assign (stmt)) + return false; + + tree lhs = gimple_assign_lhs (stmt); + + /* We can remove extension only for non-pointer and integral stmts. */ + if (!INTEGRAL_TYPE_P (TREE_TYPE (lhs)) + || POINTER_TYPE_P (TREE_TYPE (lhs))) + return false; + + type_max = tree_to_double_int (TYPE_MAX_VALUE (TREE_TYPE (lhs))); + type_min = tree_to_double_int (TYPE_MIN_VALUE (TREE_TYPE (lhs))); + + /* For binary expressions, if one of the argument is constant and is + larger than tne signed maximum, it can be intepreted as nagative + and sign extended. This could lead to problems so return false in + this case. */ + if (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_binary) + { + tree rhs1 = gimple_assign_rhs1 (stmt); + tree rhs2 = gimple_assign_rhs2 (stmt); + + if (TREE_CODE (rhs1) == INTEGER_CST) + int_val = rhs1; + else if (TREE_CODE (rhs2) == INTEGER_CST) + int_val = rhs2; + + if (int_val != NULL_TREE) + { + tree max = TYPE_MIN_VALUE (TREE_TYPE (lhs)); + + /* if type is unsigned, get the max for signed equivalent. */ + if (!INT_CST_LT (TYPE_MIN_VALUE (TREE_TYPE (lhs)), integer_zero_node)) + max = int_const_binop (RSHIFT_EXPR, + max, build_int_cst (TREE_TYPE (max), 1)); + if (!INT_CST_LT (int_val, max)) + return false; + } + } + + /* Get the value range. */ + ri = SSA_NAME_RANGE_INFO (lhs); + + /* Check if value range is valid. */ + if (!ri || !ri->valid || !ri->vr_range) + return false; + + /* Value range fits type. */ + if (ri->max.scmp (type_max) != 1 + && (type_min.scmp (ri->min) != 1)) + return true; + + return false; +} + + /* Helper for gimple_build_call, gimple_build_call_valist, gimple_build_call_vec and gimple_build_call_from_tree. Build the basic components of a GIMPLE_CALL statement to function FN with NARGS diff --git a/gcc/gimple.h b/gcc/gimple.h index 8ae07c9..769e7e4 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -829,6 +829,7 @@ int gimple_call_flags (const_gimple); int gimple_call_return_flags (const_gimple); int gimple_call_arg_flags (const_gimple, unsigned); void gimple_call_reset_alias_info (gimple); +bool gimple_assign_is_zero_sign_ext_redundant (gimple); bool gimple_assign_copy_p (gimple); bool gimple_assign_ssa_name_copy_p (gimple); bool gimple_assign_unary_nop_p (gimple);