From patchwork Wed Aug 14 07:29:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 266979 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 "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 335772C0182 for ; Wed, 14 Aug 2013 17:30:13 +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=ofztD7fipNykRrWnL vnKIpXDJjxG+9288J7LFqv8xJKx129bPqiGcO7Jgt7hk0YKDspw2Fr6f0r8xCpKE 8KQDp5D/YJm1VCAJgwN4F74TeBIuZ57adqoxmaK0XyIsBSrJcOg/3NKHEC61S/N/ q/kbOcNauDKucTy9HVm9J99TwI= 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=JIppLMTmR4MjWuAsSGWaxv9 ZAuc=; b=BFpZ5VeywpKXJOJNm3nuYIzkyhkVCszbNGF8a5j/FAh9DikD7C590MC fAWxzWZuhX0pq1/4X1h4BoOC3+Uqwk37moY7A0U02J8NHuUFJJcoxJlPRHd3oF7X g4ryQRE9W/f3SNsZoa9+yzA6chHHydGs7gkHNK0w02yW2HbSAtNg= Received: (qmail 15166 invoked by alias); 14 Aug 2013 07:30:07 -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 15138 invoked by uid 89); 14 Aug 2013 07:30:07 -0000 X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE autolearn=ham version=3.3.2 Received: from mail-pb0-f50.google.com (HELO mail-pb0-f50.google.com) (209.85.160.50) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 14 Aug 2013 07:30:05 +0000 Received: by mail-pb0-f50.google.com with SMTP id uo5so9018780pbc.37 for ; Wed, 14 Aug 2013 00:30:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type; bh=z0Hg1jIDiNP3eeuyPcf59oo0G0jlSXmYuHITN94Xw8Y=; b=j4kd0Vs9nl00Z+XhogBIIv680sRGBiA121L+wiRY5ZdjRVq9Pk7rMx00wUDs4xHe5o m4MzRsXnTYH5UbUmUPKG/3bTjOXQHiotcghL/9tOo9LqrLKmr70aBcRR+16ZcFCSXpbM OIppP+fQvAhohwfReWtinwOrPL5vusuWHh/5syifTh+QOkAGH/ryzV9PyyuGkK0fqZR1 Kour5B1d/XpVGkm6e0lA9LfFsZjc0I59SbNM/yCRJiDkwY8xbyIXau8CtJ4oBUAh4aKe FmyFH3r/v5KW7vbTiDkuJYku1bKGe2q+a+GWa4qUJlA9gFfvE1xxnZ/RqNJlay9pkrAv fmeQ== X-Gm-Message-State: ALoCoQlXzimOk3rfy54c6bOsQNO/Xw+SRgZbIs6sS/YzeO4nKGJvz5gslgYo36SF8pioWQGui2X7 X-Received: by 10.68.244.73 with SMTP id xe9mr8416811pbc.119.1376465404278; Wed, 14 Aug 2013 00:30:04 -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 tr10sm48466481pbc.22.2013.08.14.00.30.00 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 14 Aug 2013 00:30:03 -0700 (PDT) Message-ID: <520B31F5.7020200@linaro.org> Date: Wed, 14 Aug 2013 16:59:57 +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> In-Reply-To: <1726629.C2vZH2NXuZ@polaris> X-Virus-Found: No 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. > > + /* 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);