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: 19098 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-gh0-f198.google.com (mail-gh0-f198.google.com [209.85.160.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id C9BE32390D for ; Wed, 14 Aug 2013 07:30:06 +0000 (UTC) Received: by mail-gh0-f198.google.com with SMTP id r13sf10162759ghr.9 for ; Wed, 14 Aug 2013 00:30:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; 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=z0Hg1jIDiNP3eeuyPcf59oo0G0jlSXmYuHITN94Xw8Y=; b=FIjZ6OFPZL0lHavnwWldJB32HnafWavOcQF+oWuvU58gsvE7yO9ZC8qwqQlZPAO2Mz krTJUte7a5SmaTwaj23O9o+oUL2gwl6fmJf3u2MOtQT1zA3UVV7XZAYzx8OahH3b2dmb 0QpSGp823DNoPONpOJDmzO/Vo8xal077smM3dt3sXlIBDNsQ/nAS3qvz/0Xw35IxrdgC X4M+4lgcHmXj1bS31/KCUstDCA7ja3bOpNgX2eAGNkQGyVudx7nqDHGNtnWSJ91R2bX3 YdMPstQYwI7YYk9ED3xlkaXh2a+djgBXFyGPzZ+OSU6f2KHy5eI8cKGvcgSYvGEcb7yN 4evQ== X-Received: by 10.58.178.129 with SMTP id cy1mr1478184vec.39.1376465406103; Wed, 14 Aug 2013 00:30:06 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.109.35 with SMTP id hp3ls2893128qeb.76.gmail; Wed, 14 Aug 2013 00:30:06 -0700 (PDT) X-Received: by 10.58.76.130 with SMTP id k2mr6948531vew.24.1376465405962; Wed, 14 Aug 2013 00:30:05 -0700 (PDT) Received: from mail-ve0-f178.google.com (mail-ve0-f178.google.com [209.85.128.178]) by mx.google.com with ESMTPS id sg6si1810695vcb.133.2013.08.14.00.30.05 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 14 Aug 2013 00:30:05 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.128.178 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.178; Received: by mail-ve0-f178.google.com with SMTP id ox1so7426122veb.37 for ; Wed, 14 Aug 2013 00:30:05 -0700 (PDT) X-Gm-Message-State: ALoCoQk8A8/RRDIDKmUi9Vf9frX5ttOkEsd5YdSe9P9C0xw9o8gHMDjQHpV4fa7NRsfUeQ/Pmujt X-Received: by 10.221.36.4 with SMTP id sy4mr8393234vcb.11.1376465405859; Wed, 14 Aug 2013 00:30:05 -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 u4csp197824vcz; Wed, 14 Aug 2013 00:30:05 -0700 (PDT) X-Received: by 10.68.36.38 with SMTP id n6mr8542092pbj.15.1376465404790; Wed, 14 Aug 2013 00:30:04 -0700 (PDT) Received: from mail-pa0-f43.google.com (mail-pa0-f43.google.com [209.85.220.43]) by mx.google.com with ESMTPS id or6si28458288pac.15.2013.08.14.00.30.04 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 14 Aug 2013 00:30:04 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.43 is neither permitted nor denied by best guess record for domain of kugan.vivekanandarajah@linaro.org) client-ip=209.85.220.43; Received: by mail-pa0-f43.google.com with SMTP id hz10so9777345pad.30 for ; Wed, 14 Aug 2013 00:30:04 -0700 (PDT) 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-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.128.178 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: , 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);