Thanks Richard for the review.
On 15/10/13 23:55, Richard Biener wrote:
> On Tue, 15 Oct 2013, Kugan wrote:
>
>> Hi Eric,
>>
>> Can you please help to review this patch?
>> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00452.html
>
> I think that gimple_assign_is_zero_sign_ext_redundant and its
> description is somewhat confused. You seem to have two cases
> here, one being NOP_EXPR or CONVERT_EXPR that are in the IL
> because they are required for type correctness. So,
>
I have changed the name and the comments to make it more clear.
> long = (long) int
>
> which usually expands to
>
> (set:DI (sext:DI reg:SI))
>
> you want to expand as
>
> (set:DI (subreg:DI reg:SI))
>
> where you have to be careful for modes smaller than word_mode.
> You don't seem to implement this optimization though (but
> the gimple_assign_is_zero_sign_ext_redundant talks about it).
>
I am actually handling only the cases smaller than word_mode. If there
is any sign change, I dont do any changes. In the place RTL expansion is
done, I have added these condition as it is done for convert_move and
others.
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)))
> Second are promotions required by the target (PROMOTE_MODE)
> that do arithmetic on wider registers like for
>
> char = char + char
>
> where they cannot do
>
> (set:QI (plus:QI reg:QI reg:QI))
>
> but instead have, for example reg:SI only and you get
>
> (set:SI (plus:SI reg:SI reg:SI))
> (set:SI ([sz]ext:SI (subreg:QI reg:SI)))
>
> that you try to address with the cfgexpand hunk but I believe
> it doesn't work the way you do it. That is because on GIMPLE
> you do not see SImode temporaries and thus no SImode value-ranges.
> Consider
>
> tem = (char) 255 + (char) 1;
>
> which has a value-range of [0,0] but clearly when computed in
> SImode the value-range is [256, 256]. That is, VRP computes
> value-ranges in the expression type, not in some arbitrary
> larger type.
>
> So what you'd have to do is take the value-ranges of the
> two operands of the plus and see whether the plus can overflow
> QImode when computed in SImode (for the example).
>
Yes. Instead of calculating the value ranges of the two operand in
SImode, What I am doing in this case is to look at the value range of
tem and if it is within [CHAR_MIN + 1, CHAR_MAX -1]. As you have
explained earlier, we cant rely on being within the [CHAR_MIN, CHAR_MAX]
as the range could have been modified to fit the LHS type. This ofcourse
will miss some of the cases where we can remove extensions but
simplifies the logic.
> [exposing the effect of PROMOTE_MODE earlier than at RTL expansion
> time may make this less awkward]
Please find the modified patch attached.
+2013-10-16 Kugan Vivekanandarajah <kug...@linaro.org>
+
+ * dojump.c (do_compare_and_jump): Generate rtl without
+ zero/sign extension if redundant.
+ * cfgexpand.c (expand_gimple_stmt_1): Likewise.
+ * gimple.c (gimple_is_rhs_value_fits_in_assign) : New
+ function.
+ * gimple.h (gimple_is_rhs_value_fits_in_assign) : Declare.
+
Thanks,
Kugan
>
> Thanks,
> Richard.
>
>> Thanks,
>> Kugan
>>
>>> +2013-09-25 Kugan Vivekanandarajah <kug...@linaro.org>
>>> +
>>> + * 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) : Declare.
>>> +
>>>
>>>
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 88e48c2..60869ce 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 conversion, we can skip the SUBREG
+ and extension. */
+ else if (promoted
+ && gimple_is_rhs_value_fits_in_assign (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..8ac3bb0 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,64 @@ 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. */
+ 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_is_rhs_value_fits_in_assign (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_is_rhs_value_fits_in_assign (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 (op1), op1)))
+ {
+ /* First operand is constant and signbit is not set (not
+ represented in RTL as a negative 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 (op0), op0)))
+ {
+ /* Other operand is constant and signbit is not set (not
+ represented in RTL as a negative constant). */
+ rtx new_op1 = gen_reg_rtx (GET_MODE (SUBREG_REG (op1)));
+
+ emit_move_insn (new_op1, SUBREG_REG (op1));
+ op1 = new_op1;
+ }
+ 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))))
+ {
+ /* Compare registers fits SUBREG and of the
+ same mode. */
+ 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 59fcf43..6fdde39 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -200,6 +200,107 @@ gimple_call_reset_alias_info (gimple s)
pt_solution_reset (gimple_call_clobber_set (s));
}
+/* Check if RHS value of gimple does not overflows in assign (used in deciding
+ if zero/sign extension is redundant in modes smaller than word_mode).
+ i.e. if an assignment gimple statement has RHS expression value that can
+ fit in LHS type, subreg and extension to fit can be redundant. Zero/sign
+ extensions in this case can be removed.
+
+ If the assignment is
+ 1) NOP_EXPR or CONVERT_EXPR:
+ Check value range of RHS to see if it fits LHS type. If value fits type,
+ extension is redundant.
+
+ 2) In case of gimple statements involving arithmetic operations, RHS values
+ could be promoted during operation and results truncated and assigned back
+ to LHS. Therefore, to see if RHS value overflow, we must take the value
+ ranges of RHS operands and compute RHS expression value range in promoted
+ mode. Alternatively, as a simplification (there will be some false
+ negative), we can look at the LHS value range and see if LHS value range
+ is less than the LHS TYPE_MAX and greater than LHS TYPE_MIN. i.e, if the
+ LHS value range is within [LHS TYP_MIN + 1, LHS TYPE_MAX - 1], zero/sign
+ extension is redundant. */
+
+bool
+gimple_is_rhs_value_fits_in_assign (gimple stmt)
+{
+ double_int type_min, type_max;
+ double_int min, max;
+ enum value_range_type range_type;
+ tree int_val = NULL_TREE;
+ enum tree_code stmt_code;
+ tree lhs, rhs1;
+
+ if (!is_gimple_assign (stmt))
+ return false;
+
+ stmt_code = gimple_assign_rhs_code (stmt);
+ lhs = gimple_assign_lhs (stmt);
+ rhs1 = gimple_assign_rhs1 (stmt);
+
+ /* We remove extension 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)));
+
+ if ((stmt_code == NOP_EXPR || stmt_code == CONVERT_EXPR))
+ {
+ if ((TREE_CODE (rhs1) != SSA_NAME)
+ || (TYPE_UNSIGNED (TREE_TYPE (lhs))
+ != TYPE_UNSIGNED (TREE_TYPE (rhs1))))
+ return false;
+
+ range_type = get_range_info (rhs1, &min, &max);
+
+ /* For NOP_EXPR and CONVERT_EXPR, if rhs value range fits lhs
+ type, zero/sign extension is redundant. */
+ if (range_type == VR_RANGE
+ && max.cmp (type_max, TYPE_UNSIGNED (TREE_TYPE (lhs))) != 1
+ && (type_min.cmp (min, TYPE_UNSIGNED (TREE_TYPE (lhs))) != 1))
+ return true;
+ }
+
+ /* For binary expressions, if one of the argument is constant and is
+ larger than signed maximum, it can be interpreted as negative
+ number and sign extended in RTL, so return false in this case. */
+ if (TREE_CODE_CLASS (stmt_code) == 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. */
+ range_type = get_range_info (lhs, &min, &max);
+
+ /* LHS value range fits [LHS TYPE_MIN + 1, LHS TYPE_MAX -1]. */
+ if (range_type == VR_RANGE
+ && (max.cmp (type_max, TYPE_UNSIGNED (TREE_TYPE (lhs))) == -1)
+ && (type_min.cmp (min, TYPE_UNSIGNED (TREE_TYPE (lhs))) == -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 5f12805..62ac9fb 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -832,6 +832,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_is_rhs_value_fits_in_assign (gimple);
bool gimple_assign_copy_p (gimple);
bool gimple_assign_ssa_name_copy_p (gimple);
bool gimple_assign_unary_nop_p (gimple);
