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: 271844 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "www.sourceware.org", Issuer "StartCom Class 1 Primary Intermediate Server CA" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 9359A2C0079 for ; Mon, 2 Sep 2013 19:33:30 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=xPb9JEsI7ZNqKoCGr s3iIZmt9c28oZUS6k5yGOW8b4kv164Iu2exLIsHuSbJwEAklZ3VRJW2yfA8Qbd9j y0QlVcVG3SR4pED32PK8ApPzxAQguojyS+J3UCXK9bk+NT907Kdf9CcjcmMbT6JS JvoUwJ8Gowd2O9cppq5nAuK444= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=n48xwluEu4xGBbyI88V1TOg wiaE=; b=LETrqyMOCpFO3pdExGgYrIedNS+sGsZBY2CZpXJUQJtbreXr4BE6E7r CNRhLcb0Jn9tMi7r34rgfDgmd/uvCv7LBqTBLB+5fA9ZRf4mLAnNwT0Z/rT3ZHvA 19fmkdwPwEte3pz1pBSk24BVOM4pxROZ74Oy7HBKyVSJsn8DZ8AM= Received: (qmail 32505 invoked by alias); 2 Sep 2013 09:33:23 -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 32493 invoked by uid 89); 2 Sep 2013 09:33:23 -0000 Received: from mail-pa0-f53.google.com (HELO mail-pa0-f53.google.com) (209.85.220.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 02 Sep 2013 09:33:23 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=ALL_TRUSTED, AWL, BAYES_00, KHOP_THREADED autolearn=ham version=3.3.2 X-HELO: mail-pa0-f53.google.com Received: by mail-pa0-f53.google.com with SMTP id lb1so4962689pab.26 for ; Mon, 02 Sep 2013 02:33:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type; bh=No2xe9ltF16EqxMkD7bT6SBfPJlGyDtG97SBOzo+Pgc=; b=IiJkVs8U4z3zOq88WTXqIL+oTYLqbCW+wP5bpX3Ov7rIqQttSqL0dHU2bAc28f2MzM V9V9U2rNWBcagFPIITryJMcFrqEE/n5WGHWAs+SaZmZ4vMe4DgLVC2EXyqoaGTzw+uPZ 7Mtk0ufMwBygqmN+8T46WcQou4CgDsikiD8SX54Xupkk2U1z+3KvQ4u0gyP2Mo+m9rDK RTRQPYBN6Nj+JvjT4y0W/nayzFTBD3/OTFqfRhbloBwaOFjG0KtlAsuowPEgkPZAoiBY g+NjN0S4PJKoDXHHMeECDGDOSplcYU7JZ7stUvrSAcPBxO6ZU1HfVRjXpJUgoCMPWf+3 l+PQ== X-Gm-Message-State: ALoCoQmzofWzH7zSA43f8Wegn20QSGLA8UO2D/DQmm6BwjLDCezaYOHRRX+80FdIonx1qOTspmOL 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-IsSubscribed: yes 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);