* [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
@ 2013-06-03 2:16 Kugan
2013-06-17 1:31 ` [ping][PATCH][2 " Kugan
2013-07-01 9:22 ` [PATCH][2 " Eric Botcazou
0 siblings, 2 replies; 18+ messages in thread
From: Kugan @ 2013-06-03 2:16 UTC (permalink / raw)
To: gcc-patches, patches
Cc: Richard Biener, rdsandiford, Richard Earnshaw, ramana.radhakrishnan
[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]
Hi,
This patch removes some of the redundant sign/zero extensions using
value range information during RTL expansion.
When GIMPLE_ASSIGN stmts with LHS type smaller than word is expanded to
RTL, if we can prove that RHS expression value can always fit in LHS
type and there is no sign conversion, truncation and extension to fit
the type is redundant. For a SUBREG_PROMOTED_VAR_P, Subreg and Zero/sign
extensions are therefore redundant.
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.
This patch is based on the earlier attempt posted in
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00610.html and addresses
the review comments of Richard Biener. I am post-processing the
expand_expr_real_2 output in expand_gimple_stmt though. Reason for this
is that I would like to process all the possible assignment stmts, not
just CASE_CONVERT case and/or the REDUCE_BITFIELD.
This change along with expansion improve the geomean of spec2k int
benchmark with ref by about ~3.5% on an arm chromebook.
Tested on X86_64 and ARM.
I would like review comments on this.
Thanks,
Kugan
+2013-06-03 Kugan Vivekanandarajah <kuganv@linaro.org>
+
+ * 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.
+
[-- Attachment #2: vrp_extension_elimination_patch2.diff --]
[-- Type: text/x-patch, Size: 6970 bytes --]
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index c187273..ce980bc 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2311,6 +2311,17 @@ 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 (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);
diff --git a/gcc/dojump.c b/gcc/dojump.c
index 3f04eac..cb13f3a 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,60 @@ 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)
+ {
+ /* 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)
+ {
+ /* 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..e9499b6 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 b4de403..e924c5b 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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* [ping][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-06-03 2:16 [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP Kugan
@ 2013-06-17 1:31 ` Kugan
2013-06-21 10:06 ` [ping^2][PATCH][2 " Kugan
2013-07-01 9:22 ` [PATCH][2 " Eric Botcazou
1 sibling, 1 reply; 18+ messages in thread
From: Kugan @ 2013-06-17 1:31 UTC (permalink / raw)
To: gcc-patches
Cc: Richard Biener, rdsandiford, Richard Earnshaw,
ramana.radhakrishnan, ebotcazou
[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]
Can you please help to review this patch? Richard reviewed the original
patch and asked it to be split into two parts. Also, he wanted a review
from RTL maintainers for the RTL changes.
Thanks,
Kugan
On 03/06/13 11:46, Kugan wrote:
> Hi,
>
> This patch removes some of the redundant sign/zero extensions using
> value range information during RTL expansion.
>
> When GIMPLE_ASSIGN stmts with LHS type smaller than word is expanded to
> RTL, if we can prove that RHS expression value can always fit in LHS
> type and there is no sign conversion, truncation and extension to fit
> the type is redundant. For a SUBREG_PROMOTED_VAR_P, Subreg and Zero/sign
> extensions are therefore redundant.
>
> 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.
>
> This patch is based on the earlier attempt posted in
> http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00610.html and addresses
> the review comments of Richard Biener. I am post-processing the
> expand_expr_real_2 output in expand_gimple_stmt though. Reason for this
> is that I would like to process all the possible assignment stmts, not
> just CASE_CONVERT case and/or the REDUCE_BITFIELD.
>
> This change along with expansion improve the geomean of spec2k int
> benchmark with ref by about ~3.5% on an arm chromebook.
>
> Tested on X86_64 and ARM.
>
> I would like review comments on this.
>
> Thanks,
> Kugan
>
>
> +2013-06-03 Kugan Vivekanandarajah <kuganv@linaro.org>
> +
> + * 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.
> +
>
>
>
>
>
>
>
[-- Attachment #2: vrp_extension_elimination_patch2.diff --]
[-- Type: text/x-patch, Size: 6963 bytes --]
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index c187273..ce980bc 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2311,6 +2311,17 @@ 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 (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);
diff --git a/gcc/dojump.c b/gcc/dojump.c
index 3f04eac..cb13f3a 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,60 @@ 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)
+ {
+ /* 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)
+ {
+ /* 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..e9499b6 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 b4de403..e924c5b 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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* [ping^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-06-17 1:31 ` [ping][PATCH][2 " Kugan
@ 2013-06-21 10:06 ` Kugan
0 siblings, 0 replies; 18+ messages in thread
From: Kugan @ 2013-06-21 10:06 UTC (permalink / raw)
To: gcc-patches, ebotcazou
Cc: Richard Biener, rdsandiford, Richard Earnshaw, ramana.radhakrishnan
Hi Eric,
Can you please help to review the general idea and this patch for zero
sign extension elimination with VRP?
Thanks,
Kugan
On 17/06/13 11:01, Kugan wrote:
> Can you please help to review this patch? Richard reviewed the original
> patch and asked it to be split into two parts. Also, he wanted a review
> from RTL maintainers for the RTL changes.
>
> Thanks,
> Kugan
>
> On 03/06/13 11:46, Kugan wrote:
>> Hi,
>>
>> This patch removes some of the redundant sign/zero extensions using
>> value range information during RTL expansion.
>>
>> When GIMPLE_ASSIGN stmts with LHS type smaller than word is expanded to
>> RTL, if we can prove that RHS expression value can always fit in LHS
>> type and there is no sign conversion, truncation and extension to fit
>> the type is redundant. For a SUBREG_PROMOTED_VAR_P, Subreg and Zero/sign
>> extensions are therefore redundant.
>>
>> 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.
>>
>> This patch is based on the earlier attempt posted in
>> http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00610.html and addresses
>> the review comments of Richard Biener. I am post-processing the
>> expand_expr_real_2 output in expand_gimple_stmt though. Reason for this
>> is that I would like to process all the possible assignment stmts, not
>> just CASE_CONVERT case and/or the REDUCE_BITFIELD.
>>
>> This change along with expansion improve the geomean of spec2k int
>> benchmark with ref by about ~3.5% on an arm chromebook.
>>
>> Tested on X86_64 and ARM.
>>
>> I would like review comments on this.
>>
>> Thanks,
>> Kugan
>>
>>
>> +2013-06-03 Kugan Vivekanandarajah <kuganv@linaro.org>
>> +
>> + * 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.
>> +
>>
>>
>>
>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-06-03 2:16 [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP Kugan
2013-06-17 1:31 ` [ping][PATCH][2 " Kugan
@ 2013-07-01 9:22 ` Eric Botcazou
2013-08-14 7:30 ` Kugan
1 sibling, 1 reply; 18+ messages in thread
From: Eric Botcazou @ 2013-07-01 9:22 UTC (permalink / raw)
To: Kugan
Cc: gcc-patches, patches, Richard Biener, rdsandiford,
Richard Earnshaw, ramana.radhakrishnan
[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 <kuganv@linaro.org>
> +
> + * 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..."
+ /* 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.
+ /* 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.
+ /* 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.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-07-01 9:22 ` [PATCH][2 " Eric Botcazou
@ 2013-08-14 7:30 ` Kugan
2013-09-02 9:33 ` Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2013-08-14 7:30 UTC (permalink / raw)
To: Eric Botcazou
Cc: gcc-patches, patches, Richard Biener, rdsandiford,
Richard Earnshaw, ramana.radhakrishnan
[-- Attachment #1: Type: text/plain, Size: 5378 bytes --]
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 <kuganv@linaro.org>
>> +
>> + * 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 <kuganv@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) : New
+ function definition.
+
2013-08-09 Jan Hubicka <jh@suse.cz>
* 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
[-- Attachment #2: vrp_extension_elimination_patch2_r2.diff --]
[-- Type: text/x-patch, Size: 7186 bytes --]
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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-08-14 7:30 ` Kugan
@ 2013-09-02 9:33 ` Kugan
2013-09-26 9:25 ` Kugan Vivekanandarajah
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2013-09-02 9:33 UTC (permalink / raw)
To: Eric Botcazou
Cc: gcc-patches, patches, Richard Biener, rdsandiford,
Richard Earnshaw, ramana.radhakrishnan
[-- Attachment #1: Type: text/plain, Size: 5919 bytes --]
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 <kuganv@linaro.org>
>>> +
>>> + * 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 <kuganv@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) : New
> + function definition.
> +
> 2013-08-09 Jan Hubicka <jh@suse.cz>
>
> * 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
[-- Attachment #2: vrp_extension_elimination_patch2_r2.diff --]
[-- Type: text/x-patch, Size: 7186 bytes --]
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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-09-02 9:33 ` Kugan
@ 2013-09-26 9:25 ` Kugan Vivekanandarajah
2013-10-08 7:32 ` Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Kugan Vivekanandarajah @ 2013-09-26 9:25 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Biener, rdsandiford
[-- Attachment #1: Type: text/plain, Size: 356 bytes --]
Hi,
This is the updated patch for expanding gimple stmts without zer/sign
extensions when it is safe to do that. This is based on the
latest changes to propagating value range information to SSA_NAMEs
and addresses review comments from Eric.
Bootstrapped and regtested on x86_64-unknown-linux-gnu and arm-none
linux-gnueabi. Is this OK ?
Thanks,
Kugan
[-- Attachment #2: ChangeLog --]
[-- Type: application/octet-stream, Size: 548 bytes --]
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index fdca495..ad0a026 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2013-09-25 Kugan Vivekanandarajah <kuganv@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.
+
2013-09-25 Richard Biener <rguenther@suse.de>
PR middle-end/58521
[-- Attachment #3: patch.diff --]
[-- Type: application/octet-stream, Size: 8018 bytes --]
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 88e48c2..6a22f8b 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..9ea5995 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_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 (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..7bb93a6 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -200,6 +200,102 @@ gimple_call_reset_alias_info (gimple s)
pt_solution_reset (gimple_call_clobber_set (s));
}
+/* Check gimple assign stmt and see if zero/sign extension is
+ redundant. 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) For all other types:
+ If the value range of LHS is less than the LHS TYPE_MAX and greater than
+ LHS TYPE_MIN, RHS expresion (from which LHS value range is derived)
+ will also have the same value range. Therefore extension is redundant. */
+
+bool
+gimple_assign_is_zero_sign_ext_redundant (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);
+
+ /* Value range fits type. */
+ 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)
+ || min.is_zero ()))
+ 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..791f606 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_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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-09-26 9:25 ` Kugan Vivekanandarajah
@ 2013-10-08 7:32 ` Kugan
2013-10-15 10:28 ` [PING^2][PATCH][2 " Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2013-10-08 7:32 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Biener, steven
[-- Attachment #1: Type: text/plain, Size: 802 bytes --]
Ping~
Thanks,
Kugan
+2013-09-25 Kugan Vivekanandarajah <kuganv@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.
+
On 26/09/13 18:04, Kugan Vivekanandarajah wrote:
> Hi,
>
> This is the updated patch for expanding gimple stmts without zer/sign
> extensions when it is safe to do that. This is based on the
> latest changes to propagating value range information to SSA_NAMEs
> and addresses review comments from Eric.
>
> Bootstrapped and regtested on x86_64-unknown-linux-gnu and arm-none
> linux-gnueabi. Is this OK ?
>
> Thanks,
> Kugan
>
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 8018 bytes --]
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 88e48c2..6a22f8b 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..9ea5995 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_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 (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..7bb93a6 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -200,6 +200,102 @@ gimple_call_reset_alias_info (gimple s)
pt_solution_reset (gimple_call_clobber_set (s));
}
+/* Check gimple assign stmt and see if zero/sign extension is
+ redundant. 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) For all other types:
+ If the value range of LHS is less than the LHS TYPE_MAX and greater than
+ LHS TYPE_MIN, RHS expresion (from which LHS value range is derived)
+ will also have the same value range. Therefore extension is redundant. */
+
+bool
+gimple_assign_is_zero_sign_ext_redundant (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);
+
+ /* Value range fits type. */
+ 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)
+ || min.is_zero ()))
+ 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..791f606 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_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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-10-08 7:32 ` Kugan
@ 2013-10-15 10:28 ` Kugan
2013-10-15 13:38 ` Richard Biener
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2013-10-15 10:28 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Biener, steven
Hi Eric,
Can you please help to review this patch?
http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00452.html
Thanks,
Kugan
> +2013-09-25 Kugan Vivekanandarajah <kuganv@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.
> +
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-10-15 10:28 ` [PING^2][PATCH][2 " Kugan
@ 2013-10-15 13:38 ` Richard Biener
2013-10-16 1:47 ` Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Richard Biener @ 2013-10-15 13:38 UTC (permalink / raw)
To: Kugan; +Cc: Eric Botcazou, gcc-patches, steven
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,
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).
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).
[exposing the effect of PROMOTE_MODE earlier than at RTL expansion
time may make this less awkward]
Thanks,
Richard.
> Thanks,
> Kugan
>
> > +2013-09-25 Kugan Vivekanandarajah <kuganv@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.
> > +
> >
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-10-15 13:38 ` Richard Biener
@ 2013-10-16 1:47 ` Kugan
2013-10-18 12:40 ` Richard Biener
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2013-10-16 1:47 UTC (permalink / raw)
To: Richard Biener; +Cc: Eric Botcazou, gcc-patches, steven
[-- Attachment #1: Type: text/plain, Size: 4033 bytes --]
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 <kuganv@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 <kuganv@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.
>>> +
>>>
>>>
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 8473 bytes --]
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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-10-16 1:47 ` Kugan
@ 2013-10-18 12:40 ` Richard Biener
2013-10-23 7:20 ` Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Richard Biener @ 2013-10-18 12:40 UTC (permalink / raw)
To: Kugan; +Cc: Eric Botcazou, gcc-patches, steven
On Wed, 16 Oct 2013, Kugan wrote:
> 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.
Not sure if I understand what you are saying here. As for the above
case
> > tem = (char) 255 + (char) 1;
tem is always of type 'char' in GIMPLE (even if later promoted
via PROMOTE_MODE) the value-range is a 'char' value-range and thus
never will exceed [CHAR_MIN, CHAR_MAX]. The only way you can
use that directly is if you can rely on undefined behavior
happening for signed overflow - but if you argue that way you
can simply _always_ drop the (sext:SI (subreg:QI part and you
do not need value ranges for this. For unsigned operations
for example [250, 254] + [8, 10] will simply wrap to [3, 7]
(if I got the math correct) which is inside your [CHAR_MIN + 1,
CHAR_MAX - 1] but if performed in SImode you can get 259 and
thus clearly you cannot drop the (zext:SI (subreg:QI parts.
The same applies to signed types if you do not want to rely
on signed overflow being undefined of course.
Richard.
> > [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 <kuganv@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 <kuganv@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.
> >>> +
> >>>
> >>>
>
>
--
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-10-18 12:40 ` Richard Biener
@ 2013-10-23 7:20 ` Kugan
2013-12-06 4:24 ` Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2013-10-23 7:20 UTC (permalink / raw)
To: Richard Biener; +Cc: Eric Botcazou, gcc-patches, steven
>>> 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).
>>>
Ok, I will handle it as you have suggested here.
> Not sure if I understand what you are saying here. As for the above
> case
>
>>> tem = (char) 255 + (char) 1;
>
> tem is always of type 'char' in GIMPLE (even if later promoted
> via PROMOTE_MODE) the value-range is a 'char' value-range and thus
> never will exceed [CHAR_MIN, CHAR_MAX]. The only way you can
> use that directly is if you can rely on undefined behavior
> happening for signed overflow - but if you argue that way you
> can simply _always_ drop the (sext:SI (subreg:QI part and you
> do not need value ranges for this. For unsigned operations
> for example [250, 254] + [8, 10] will simply wrap to [3, 7]
> (if I got the math correct) which is inside your [CHAR_MIN + 1,
> CHAR_MAX - 1] but if performed in SImode you can get 259 and
> thus clearly you cannot drop the (zext:SI (subreg:QI parts.
> The same applies to signed types if you do not want to rely
> on signed overflow being undefined of course.
>
Thanks for the explanation. I now get it and I will rework the patch.
Thanks,
Kugan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-10-23 7:20 ` Kugan
@ 2013-12-06 4:24 ` Kugan
2014-01-07 11:59 ` Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2013-12-06 4:24 UTC (permalink / raw)
To: Richard Biener; +Cc: Eric Botcazou, gcc-patches, steven
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
>>>> tem = (char) 255 + (char) 1;
>>
>> tem is always of type 'char' in GIMPLE (even if later promoted
>> via PROMOTE_MODE) the value-range is a 'char' value-range and thus
>> never will exceed [CHAR_MIN, CHAR_MAX]. The only way you can
>> use that directly is if you can rely on undefined behavior
>> happening for signed overflow - but if you argue that way you
>> can simply _always_ drop the (sext:SI (subreg:QI part and you
>> do not need value ranges for this. For unsigned operations
>> for example [250, 254] + [8, 10] will simply wrap to [3, 7]
>> (if I got the math correct) which is inside your [CHAR_MIN + 1,
>> CHAR_MAX - 1] but if performed in SImode you can get 259 and
>> thus clearly you cannot drop the (zext:SI (subreg:QI parts.
>> The same applies to signed types if you do not want to rely
>> on signed overflow being undefined of course.
>>
>
> Thanks for the explanation. I now get it and I will rework the patch.
>
I have attempted to implement what Richard suggested. If you think this
is what you want, I will go ahead and implement the missing gimple
binary statements.
Thanks again.
Kugan
[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 8299 bytes --]
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 98983f4..60ce54b 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -3236,6 +3236,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
+ && is_assigned_exp_fit_type (lhs)
+ && (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 2aef34d..0f3aeae 100644
--- a/gcc/dojump.c
+++ b/gcc/dojump.c
@@ -1143,6 +1143,62 @@ 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 = is_assigned_exp_fit_type (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 = is_assigned_exp_fit_type (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))))
+ {
+ /* If both comapre 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/tree.c b/gcc/tree.c
index d363cfc..d22630a 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -408,6 +408,156 @@ tree_node_structure_for_code (enum tree_code code)
}
}
+/* Check expression value for overflow in tcc_binary. */
+static bool
+is_binary_expr_fit_type (enum tree_code stmt_code,
+ tree lhs, double_int min1, double_int max1,
+ double_int min2, double_int max2)
+{
+ double_int dmin, dmax;
+ double_int type_max = tree_to_double_int (TYPE_MAX_VALUE (TREE_TYPE (lhs)));
+ double_int type_min = tree_to_double_int (TYPE_MIN_VALUE (TREE_TYPE (lhs)));
+ bool uns = TYPE_UNSIGNED (TREE_TYPE (lhs));
+
+ switch (stmt_code)
+ {
+ case PLUS_EXPR:
+ dmin = min1 + min2;
+ dmax = max1 + max2;
+ break;
+ case MINUS_EXPR:
+ dmin = min1 - min2;
+ dmax = max1 - max2;
+ break;
+ default:
+ return false;
+ }
+
+ if (dmin.cmp (type_min, uns) == -1)
+ return false;
+ else if (dmin.cmp (type_max, uns) == 1)
+ return false;
+ if (dmax.cmp (type_min, uns) == -1)
+ return false;
+ else if (dmax.cmp (type_max, uns) == 1)
+ return false;
+ return true;
+}
+
+/* Check gimple assign stmt and see if zero/sign extension is
+ redundant. 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. */
+
+bool
+is_assigned_exp_fit_type (tree lhs)
+{
+ double_int type_min, type_max;
+ double_int min1, max1, min2, max2;
+ enum tree_code stmt_code;
+ tree rhs1, rhs2;
+ gimple stmt = SSA_NAME_DEF_STMT (lhs);
+ double_int msb_val;
+
+ if (gimple_code (stmt) != GIMPLE_ASSIGN)
+ return false;
+
+ stmt_code = gimple_assign_rhs_code (stmt);
+ rhs1 = gimple_assign_rhs1 (stmt);
+ bool uns = TYPE_UNSIGNED (TREE_TYPE (lhs));
+
+ /* 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 (uns)
+ msb_val = type_max.rshift (1);
+ else
+ msb_val = type_max + double_int_one;
+
+ if (TREE_CODE_CLASS (stmt_code) == tcc_unary)
+ {
+ /* Get the value range. */
+ if (TREE_CODE (rhs1) == INTEGER_CST)
+ {
+ min1 = tree_to_double_int (rhs1);
+ max1 = tree_to_double_int (rhs1);
+ }
+ else if (get_range_info (rhs1, &min1, &max1) != VR_RANGE)
+ {
+ return false;
+ }
+
+ switch (stmt_code)
+ {
+ case VIEW_CONVERT_EXPR:
+ case CONVERT_EXPR:
+ case NOP_EXPR:
+ if ((TYPE_UNSIGNED (TREE_TYPE (lhs))
+ != TYPE_UNSIGNED (TREE_TYPE (rhs1)))
+ && (TYPE_PRECISION (TREE_TYPE (lhs))
+ != TYPE_PRECISION (TREE_TYPE (rhs1))))
+ return false;
+
+ if (!uns && min1.cmp (msb_val, uns) == 1
+ && max1.cmp (msb_val, uns) == 1)
+ {
+ min1 = min1.sext (TYPE_PRECISION (TREE_TYPE (rhs1)));
+ max1 = max1.sext (TYPE_PRECISION (TREE_TYPE (rhs1)));
+ }
+
+ /* If rhs value range fits lhs type, zero/sign extension is
+ redundant. */
+ if (max1.cmp (type_max, uns) != 1
+ && (type_min.cmp (min1, uns)) != 1)
+ return true;
+ else
+ return false;
+
+ case NEGATE_EXPR:
+ return is_binary_expr_fit_type (MINUS_EXPR, lhs, double_int_zero,
+ double_int_zero, min1, max1);
+ default:
+ return false;
+ }
+ }
+
+ if (TREE_CODE_CLASS (stmt_code) == tcc_binary)
+ {
+ double_int const_val;
+ rhs2 = gimple_assign_rhs2 (stmt);
+
+ /* Get the value range. */
+ if (TREE_CODE (rhs1) == INTEGER_CST)
+ {
+ min1 = tree_to_double_int (rhs1);
+ max1 = tree_to_double_int (rhs1);
+ }
+ else if (get_range_info (rhs1, &min1, &max1) != VR_RANGE)
+ {
+ return false;
+ }
+
+ /* Get the value range. */
+ if (TREE_CODE (rhs2) == INTEGER_CST)
+ {
+ min2 = tree_to_double_int (rhs2);
+ max2 = tree_to_double_int (rhs2);
+ }
+ else if (get_range_info (rhs2, &min2, &max2) != VR_RANGE)
+ {
+ return false;
+ }
+
+ return is_binary_expr_fit_type (stmt_code, lhs, min1, max1, min2, max2);
+ }
+
+ return false;
+}
/* Initialize tree_contains_struct to describe the hierarchy of tree
nodes. */
diff --git a/gcc/tree.h b/gcc/tree.h
index 88c8d56..11a286c 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4134,6 +4134,7 @@ extern tree lhd_gcc_personality (void);
extern void assign_assembler_name_if_neeeded (tree);
extern void warn_deprecated_use (tree, tree);
extern void cache_integer_cst (tree);
+extern bool is_assigned_exp_fit_type (tree lhs);
/* Compare and hash for any structure which begins with a canonical
pointer. Assumes all pointers are interchangeable, which is sort
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2013-12-06 4:24 ` Kugan
@ 2014-01-07 11:59 ` Kugan
2014-01-07 12:24 ` Richard Biener
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2014-01-07 11:59 UTC (permalink / raw)
To: Richard Biener; +Cc: Eric Botcazou, gcc-patches, patches
[-- Attachment #1: Type: text/plain, Size: 568 bytes --]
ping ?
I have reorganised the last patch and now handling only
VIEW_CONVERT_EXPR, CONVERT_EXPR and NOP_EXPR. Once it is reviewed and
necessary changes are made, I will address the other cases as a separate
patch (when it reaches that stage).
Thanks,
Kugan
gcc/
+2014-01-07 Kugan Vivekanandarajah <kuganv@linaro.org>
+
+ * dojump.c (do_compare_and_jump): Generate rtl without
+ zero/sign extension if redundant.
+ * cfgexpand.c (expand_gimple_stmt_1): Likewise.
+ (is_assigned_exp_fit_type) : New function.
+ * cfgexpand.h (is_assigned_exp_fit_type) : Declare.
+
[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 6018 bytes --]
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 7a93975..b2e2f90 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -476,6 +476,66 @@ add_scope_conflicts_1 (basic_block bb, bitmap work, bool for_conflict)
}
}
+
+/* Check gimple assign stmt and see if zero/sign extension is
+ redundant. 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. */
+
+bool
+is_assigned_exp_fit_type (tree lhs)
+{
+ double_int type_min, type_max;
+ double_int min1, max1;
+ enum tree_code stmt_code;
+ tree rhs1;
+ gimple stmt = SSA_NAME_DEF_STMT (lhs);
+
+ if (gimple_code (stmt) != GIMPLE_ASSIGN)
+ return false;
+
+ /* We remove extension for non-pointer and integral stmts. */
+ if (!INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+ || POINTER_TYPE_P (TREE_TYPE (lhs)))
+ return false;
+
+ stmt_code = gimple_assign_rhs_code (stmt);
+ rhs1 = gimple_assign_rhs1 (stmt);
+ 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 (TREE_CODE_CLASS (stmt_code) == tcc_unary)
+ {
+ bool uns = TYPE_UNSIGNED (TREE_TYPE (rhs1));
+ /* Get the value range. */
+ if (TREE_CODE (rhs1) == INTEGER_CST)
+ {
+ min1 = tree_to_double_int (rhs1);
+ max1 = tree_to_double_int (rhs1);
+ }
+ else if (get_range_info (rhs1, &min1, &max1) != VR_RANGE)
+ return false;
+
+ switch (stmt_code)
+ {
+ case VIEW_CONVERT_EXPR:
+ case CONVERT_EXPR:
+ case NOP_EXPR:
+ /* If rhs value range fits lhs type, zero/sign extension is
+ redundant. */
+ if (max1.cmp (type_max, 0) != 1
+ && (type_min.cmp (min1, 0)) != 1)
+ return true;
+ else
+ return false;
+ default:
+ return false;
+ }
+ }
+
+ return false;
+}
+
/* Generate stack partition conflicts between all partitions that are
simultaneously live. */
@@ -3247,6 +3307,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
+ && is_assigned_exp_fit_type (lhs)
+ && (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/cfgexpand.h b/gcc/cfgexpand.h
index 04517a3..c7d73e8 100644
--- a/gcc/cfgexpand.h
+++ b/gcc/cfgexpand.h
@@ -22,5 +22,6 @@ along with GCC; see the file COPYING3. If not see
extern tree gimple_assign_rhs_to_tree (gimple);
extern HOST_WIDE_INT estimated_stack_frame_size (struct cgraph_node *);
+extern bool is_assigned_exp_fit_type (tree lhs);
#endif /* GCC_CFGEXPAND_H */
diff --git a/gcc/dojump.c b/gcc/dojump.c
index 73df6d1..73a4b6b 100644
--- a/gcc/dojump.c
+++ b/gcc/dojump.c
@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see
#include "ggc.h"
#include "basic-block.h"
#include "tm_p.h"
+#include "cfgexpand.h"
static bool prefer_and_bit_test (enum machine_mode, int);
static void do_jump_by_parts_greater (tree, tree, int, rtx, rtx, int);
@@ -1166,6 +1167,62 @@ 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 = is_assigned_exp_fit_type (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 = is_assigned_exp_fit_type (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))))
+ {
+ /* If both comapre 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)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2014-01-07 11:59 ` Kugan
@ 2014-01-07 12:24 ` Richard Biener
2014-01-08 7:39 ` Kugan
0 siblings, 1 reply; 18+ messages in thread
From: Richard Biener @ 2014-01-07 12:24 UTC (permalink / raw)
To: Kugan; +Cc: Eric Botcazou, gcc-patches, patches
On Tue, 7 Jan 2014, Kugan wrote:
> ping ?
>
> I have reorganised the last patch and now handling only
> VIEW_CONVERT_EXPR, CONVERT_EXPR and NOP_EXPR. Once it is reviewed and
> necessary changes are made, I will address the other cases as a separate
> patch (when it reaches that stage).
Note that VIEW_CONVERT_EXPR is wrong here. I think you are
handling this wrong still. From a quick look you want to avoid
the actual promotion for
reg_1 = ....
when reg_1 is promoted and thus the target is (subreg:XX N).
The RHS has been expanded in XXmode. Dependent on the value-range
of reg_1 you want to set N to a paradoxical subreg of the expanded
result. You can always do that if the reg is zero-extended
and else if the MSB is not set for any of the values of reg_1.
I don't see how is_assigned_exp_fit_type reflects this in any way.
Anyway, the patch should not introduce another if (promoted)
case but only short-cut the final convert_move call of the existing
one.
Richard.
> Thanks,
> Kugan
>
> gcc/
>
> +2014-01-07 Kugan Vivekanandarajah <kuganv@linaro.org>
> +
> + * dojump.c (do_compare_and_jump): Generate rtl without
> + zero/sign extension if redundant.
> + * cfgexpand.c (expand_gimple_stmt_1): Likewise.
> + (is_assigned_exp_fit_type) : New function.
> + * cfgexpand.h (is_assigned_exp_fit_type) : Declare.
> +
>
--
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2014-01-07 12:24 ` Richard Biener
@ 2014-01-08 7:39 ` Kugan
2014-01-08 10:40 ` Richard Biener
0 siblings, 1 reply; 18+ messages in thread
From: Kugan @ 2014-01-08 7:39 UTC (permalink / raw)
To: Richard Biener; +Cc: Eric Botcazou, gcc-patches, patches
On 07/01/14 23:23, Richard Biener wrote:
> On Tue, 7 Jan 2014, Kugan wrote:
[snip]
> Note that VIEW_CONVERT_EXPR is wrong here. I think you are
> handling this wrong still. From a quick look you want to avoid
> the actual promotion for
>
> reg_1 = ....
>
> when reg_1 is promoted and thus the target is (subreg:XX N).
> The RHS has been expanded in XXmode. Dependent on the value-range
> of reg_1 you want to set N to a paradoxical subreg of the expanded
> result. You can always do that if the reg is zero-extended
> and else if the MSB is not set for any of the values of reg_1.
Thanks Richard for the explanation. I just want to double confirm I
understand you correctly before I attempt to fix it. So let me try this
for the following example,
for a gimple stmt of the following from:
unsigned short _5;
short int _6;
_6 = (short int)_5;
;; _6 = (short int) _5;
target = (subreg/s/u:HI (reg:SI 110 [ D.4144 ]) 0)
temp = (subreg:HI (reg:SI 118) 0)
So, I must generate the following if it satisfies the other conditions.
(set (reg:SI 110 [ D.4144 ]) (subreg:SI temp ))
Is my understanding correct?
> I don't see how is_assigned_exp_fit_type reflects this in any way.
>
What I tried doing with the patch is:
(insn 13 12 0 (set (reg:SI 110 [ D.4144 ])
(zero_extend:SI (subreg:HI (reg:SI 118) 0))) c5.c:8 -1
(nil))
If the values in register (reg:SI 118) fits HI mode (without
overflowing), I assume that it is not necessary to just drop the higher
bits and zero_extend as done above and generate the following instead.
(insn 13 12 0 (set (reg:SI 110 [ D.4144 ])
(((reg:SI 118) 0))) c5.c:8 -1
(nil))
is_assigned_exp_fit_type just checks if the range fits (in the above
case, the value in eg:SI 118 fits HI mode) and the checks before
emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp)); checks the
modes match.
Is this wrong or am I missing the whole point?
> Anyway, the patch should not introduce another if (promoted)
> case but only short-cut the final convert_move call of the existing
> one.
>
Thanks,
Kugan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
2014-01-08 7:39 ` Kugan
@ 2014-01-08 10:40 ` Richard Biener
0 siblings, 0 replies; 18+ messages in thread
From: Richard Biener @ 2014-01-08 10:40 UTC (permalink / raw)
To: Kugan; +Cc: Eric Botcazou, gcc-patches, patches
On Wed, 8 Jan 2014, Kugan wrote:
>
> On 07/01/14 23:23, Richard Biener wrote:
> > On Tue, 7 Jan 2014, Kugan wrote:
>
> [snip]
>
>
> > Note that VIEW_CONVERT_EXPR is wrong here. I think you are
> > handling this wrong still. From a quick look you want to avoid
> > the actual promotion for
> >
> > reg_1 = ....
> >
> > when reg_1 is promoted and thus the target is (subreg:XX N).
> > The RHS has been expanded in XXmode. Dependent on the value-range
> > of reg_1 you want to set N to a paradoxical subreg of the expanded
> > result. You can always do that if the reg is zero-extended
> > and else if the MSB is not set for any of the values of reg_1.
>
> Thanks Richard for the explanation. I just want to double confirm I
> understand you correctly before I attempt to fix it. So let me try this
> for the following example,
>
> for a gimple stmt of the following from:
> unsigned short _5;
> short int _6;
> _6 = (short int)_5;
>
> ;; _6 = (short int) _5;
> target = (subreg/s/u:HI (reg:SI 110 [ D.4144 ]) 0)
> temp = (subreg:HI (reg:SI 118) 0)
>
> So, I must generate the following if it satisfies the other conditions.
> (set (reg:SI 110 [ D.4144 ]) (subreg:SI temp ))
>
> Is my understanding correct?
I'm no RTL expert in this particular area but yes, I think so. Not
sure what paradoxical subregs are valid, so somebody else should
comment here. You could even generate
(set (reg:SI 110) (reg:SI 118))
iff temp is a SUBREG of a promoted var, as you require that for the
destination as well.
>
> > I don't see how is_assigned_exp_fit_type reflects this in any way.
> >
>
>
> What I tried doing with the patch is:
>
> (insn 13 12 0 (set (reg:SI 110 [ D.4144 ])
> (zero_extend:SI (subreg:HI (reg:SI 118) 0))) c5.c:8 -1
> (nil))
>
> If the values in register (reg:SI 118) fits HI mode (without
> overflowing), I assume that it is not necessary to just drop the higher
> bits and zero_extend as done above and generate the following instead.
>
> (insn 13 12 0 (set (reg:SI 110 [ D.4144 ])
> (((reg:SI 118) 0))) c5.c:8 -1
> (nil))
>
> is_assigned_exp_fit_type just checks if the range fits (in the above
> case, the value in eg:SI 118 fits HI mode) and the checks before
> emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp)); checks the
> modes match.
>
> Is this wrong or am I missing the whole point?
is_assigned_exp_fit_type is weird - it looks at the value-range of _5,
but as you want to elide the extension from _6 to SImode you want
to look at the value-range from _5. So, breaking it down and applying
the promotion to GIMPLE it would look like
unsigned short _5;
short int _6;
_6 = (short int)_5;
_6_7 = (int) _6;
where you want to remove the last line representing the
assignment to (subreg:HI (reg:SI 110)). Whether you can
do that depends on the value-range of _6, not on the
value-range of _5. It's also completely independent
on the operation performed on the RHS.
Well. As far as I understand at least.
Richard.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-01-08 10:40 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-03 2:16 [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP Kugan
2013-06-17 1:31 ` [ping][PATCH][2 " Kugan
2013-06-21 10:06 ` [ping^2][PATCH][2 " Kugan
2013-07-01 9:22 ` [PATCH][2 " Eric Botcazou
2013-08-14 7:30 ` Kugan
2013-09-02 9:33 ` Kugan
2013-09-26 9:25 ` Kugan Vivekanandarajah
2013-10-08 7:32 ` Kugan
2013-10-15 10:28 ` [PING^2][PATCH][2 " Kugan
2013-10-15 13:38 ` Richard Biener
2013-10-16 1:47 ` Kugan
2013-10-18 12:40 ` Richard Biener
2013-10-23 7:20 ` Kugan
2013-12-06 4:24 ` Kugan
2014-01-07 11:59 ` Kugan
2014-01-07 12:24 ` Richard Biener
2014-01-08 7:39 ` Kugan
2014-01-08 10:40 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).