Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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.
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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 > + > + * 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 SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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
+
+ * 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.
+
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 a
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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
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 d36
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
>>> 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
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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 > + > + * 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 > >>> + > >>> + * dojump.c (do_compare_and_jump): Generate rtl without > >>> + zero/sign extension if redundant. > >>> + * cf
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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
+
+ * 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
>>> +
>>> + * dojump.c (do_compare_and_jump): Generate rtl without
>>> + zero/sign extension if redundant.
>>> + * cfgexpand.c (expand_gimple_stmt_1): Likewise.
>>> + * gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New
>>> + function.
>>> + * gimple.h (gimple_assign_is_zero_sign_ext_redundant) : Declare.
>>> +
>>>
>>>
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 88e48c2..60869ce 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2311,6 +2311,20 @@ expand_gimple_stmt_1 (gimple stmt)
if (temp == target)
;
+ /* If the value in SUBREG of temp fits that SUBREG (does not
+ overflow) and is assigned to target SUBREG of the same mode
+ without sign conversion, we can skip the SUBREG
+ and extension. */
+ else if (promoted
+&& gimple_is_rhs_value_fits_in_assign (stmt)
+&& (GET_CODE (temp) == SUBREG)
+&& (GET_MODE_PRECISION (GET_MODE (SUBREG_REG (temp)))
+>= GET_MODE_PRECISION (GET_MODE (target)))
+&& (GET_MODE (SUBREG_REG (target))
+== GET_MODE (SUBREG_REG (temp
+ {
+ emit_move_insn (SUBREG_REG (target), SUBREG_
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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 > > + > > + * 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. > > + > > > >
[PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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 > + > + * 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. > + > >
Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
Ping~
Thanks,
Kugan
+2013-09-25 Kugan Vivekanandarajah
+
+ * dojump.c (do_compare_and_jump): Generate rtl without
+ zero/sign extension if redundant.
+ * cfgexpand.c (expand_gimple_stmt_1): Likewise.
+ * gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New
+ function.
+ * gimple.h (gimple_assign_is_zero_sign_ext_redundant) : 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
>
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
Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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 ChangeLog Description: Binary data patch.diff Description: Binary data
Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
I'd like to ping this patch 2 of 2 that removes redundant zero/sign
extension using value range information.
Bootstrapped and no new regression for x86_64-unknown-linux-gnu and
arm-none-linux-gnueabi.
Thanks you for your time.
Kugan
On 14/08/13 16:59, Kugan wrote:
Hi Eric,
Thanks for reviewing the patch.
On 01/07/13 18:51, Eric Botcazou wrote:
[Sorry for the delay]
For example, when an expression is evaluated and it's value is assigned
to variable of type short, the generated RTL would look something like
the following.
(set (reg:SI 110)
(zero_extend:SI (subreg:HI (reg:SI 117) 0)))
However, if during value range propagation, if we can say for certain
that the value of the expression which is present in register 117 is
within the limits of short and there is no sign conversion, we do not
need to perform the subreg and zero_extend; instead we can generate the
following RTl.
(set (reg:SI 110)
(reg:SI 117)))
Same could be done for other assign statements.
The idea looks interesting. Some remarks:
+2013-06-03 Kugan Vivekanandarajah
+
+* gcc/dojump.c (do_compare_and_jump): generates rtl without
+zero/sign extension if redundant.
+* gcc/cfgexpand.c (expand_gimple_stmt_1): Likewise.
+* gcc/gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New
+function.
+* gcc/gimple.h (gimple_assign_is_zero_sign_ext_redundant) : New
+function definition.
No gcc/ prefix in entries for gcc/ChangeLog. "Generate RTL without..."
I have now changed it to.
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2013-08-14 Kugan Vivekanandarajah
+
+* dojump.c (do_compare_and_jump): Generate rtl without
+zero/sign extension if redundant.
+* cfgexpand.c (expand_gimple_stmt_1): Likewise.
+* gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New
+function.
+* gimple.h (gimple_assign_is_zero_sign_ext_redundant) : New
+function definition.
+
2013-08-09 Jan Hubicka
* cgraph.c (cgraph_create_edge_1): Clear speculative flag.
+/* If the value in SUBREG of temp fits that SUBREG (does not
+ overflow) and is assigned to target SUBREG of the same
mode
+ without sign convertion, we can skip the SUBREG
+ and extension. */
+else if (promoted
+ && gimple_assign_is_zero_sign_ext_redundant (stmt)
+ && (GET_CODE (temp) == SUBREG)
+ && (GET_MODE (target) == GET_MODE (temp))
+ && (GET_MODE (SUBREG_REG (target))
+ == GET_MODE (SUBREG_REG (temp
+ emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp));
else if (promoted)
{
int unsignedp = SUBREG_PROMOTED_UNSIGNED_P (target);
Can we relax the strict mode equality here? This change augments the
same
transformation applied to the RHS when it is also a
SUBREG_PROMOTED_VAR_P at
the beginning of convert_move, but the condition on the mode is less
strict in
the latter case, so maybe it can serve as a model here.
I have now changed it based on convert_move to
+else if (promoted
+ && gimple_assign_is_zero_sign_ext_redundant (stmt)
+ && (GET_CODE (temp) == SUBREG)
+ && (GET_MODE_PRECISION (GET_MODE (SUBREG_REG (temp)))
+ >= GET_MODE_PRECISION (GET_MODE (target)))
+ && (GET_MODE (SUBREG_REG (target))
+ == GET_MODE (SUBREG_REG (temp
+ {
Is this what you wanted me to do.
+ /* Is zero/sign extension redundant as per VRP. */
+ bool op0_ext_redundant = false;
+ bool op1_ext_redundant = false;
+
+ /* If promoted and the value in SUBREG of op0 fits (does not
overflow),
+ it is a candidate for extension elimination. */
+ if (GET_CODE (op0) == SUBREG && SUBREG_PROMOTED_VAR_P (op0))
+op0_ext_redundant =
+ gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT
(treeop0));
+
+ /* If promoted and the value in SUBREG of op1 fits (does not
overflow),
+ it is a candidate for extension elimination. */
+ if (GET_CODE (op1) == SUBREG && SUBREG_PROMOTED_VAR_P (op1))
+op1_ext_redundant =
+ gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT
(treeop1));
Are the gimple_assign_is_zero_sign_ext_redundant checks necessary here?
When set on a SUBREG, SUBREG_PROMOTED_VAR_P guarantees that SUBREG_REG is
always properly extended (otherwise it's a bug) so don't you just need to
compare SUBREG_PROMOTED_UNSIGNED_SET? See do_jump for an existing case.
I am sorry I don’t think I understood you here. How would I know that
extension is redundant without calling
gimple_assign_is_zero_sign_ext_redundant ? Could you please elaborate.
+ /* If zero/sign extension is redundant, generate RTL
+ for operands without zero/sign extension. */
+ if ((op0_ext_redundant || TREE_CODE (treeop0
Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
Hi Eric,
Thanks for reviewing the patch.
On 01/07/13 18:51, Eric Botcazou wrote:
[Sorry for the delay]
For example, when an expression is evaluated and it's value is assigned
to variable of type short, the generated RTL would look something like
the following.
(set (reg:SI 110)
(zero_extend:SI (subreg:HI (reg:SI 117) 0)))
However, if during value range propagation, if we can say for certain
that the value of the expression which is present in register 117 is
within the limits of short and there is no sign conversion, we do not
need to perform the subreg and zero_extend; instead we can generate the
following RTl.
(set (reg:SI 110)
(reg:SI 117)))
Same could be done for other assign statements.
The idea looks interesting. Some remarks:
+2013-06-03 Kugan Vivekanandarajah
+
+ * gcc/dojump.c (do_compare_and_jump): generates rtl without
+ zero/sign extension if redundant.
+ * gcc/cfgexpand.c (expand_gimple_stmt_1): Likewise.
+ * gcc/gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New
+ function.
+ * gcc/gimple.h (gimple_assign_is_zero_sign_ext_redundant) : New
+ function definition.
No gcc/ prefix in entries for gcc/ChangeLog. "Generate RTL without..."
I have now changed it to.
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2013-08-14 Kugan Vivekanandarajah
+
+ * dojump.c (do_compare_and_jump): Generate rtl without
+ zero/sign extension if redundant.
+ * cfgexpand.c (expand_gimple_stmt_1): Likewise.
+ * gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New
+ function.
+ * gimple.h (gimple_assign_is_zero_sign_ext_redundant) : New
+ function definition.
+
2013-08-09 Jan Hubicka
* cgraph.c (cgraph_create_edge_1): Clear speculative flag.
+/* If the value in SUBREG of temp fits that SUBREG (does not
+ overflow) and is assigned to target SUBREG of the same mode
+ without sign convertion, we can skip the SUBREG
+ and extension. */
+else if (promoted
+ && gimple_assign_is_zero_sign_ext_redundant (stmt)
+ && (GET_CODE (temp) == SUBREG)
+ && (GET_MODE (target) == GET_MODE (temp))
+ && (GET_MODE (SUBREG_REG (target))
+ == GET_MODE (SUBREG_REG (temp
+ emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp));
else if (promoted)
{
int unsignedp = SUBREG_PROMOTED_UNSIGNED_P (target);
Can we relax the strict mode equality here? This change augments the same
transformation applied to the RHS when it is also a SUBREG_PROMOTED_VAR_P at
the beginning of convert_move, but the condition on the mode is less strict in
the latter case, so maybe it can serve as a model here.
I have now changed it based on convert_move to
+else if (promoted
+ && gimple_assign_is_zero_sign_ext_redundant (stmt)
+ && (GET_CODE (temp) == SUBREG)
+ && (GET_MODE_PRECISION (GET_MODE (SUBREG_REG (temp)))
+>= GET_MODE_PRECISION (GET_MODE (target)))
+ && (GET_MODE (SUBREG_REG (target))
+ == GET_MODE (SUBREG_REG (temp
+ {
Is this what you wanted me to do.
+ /* Is zero/sign extension redundant as per VRP. */
+ bool op0_ext_redundant = false;
+ bool op1_ext_redundant = false;
+
+ /* If promoted and the value in SUBREG of op0 fits (does not overflow),
+ it is a candidate for extension elimination. */
+ if (GET_CODE (op0) == SUBREG && SUBREG_PROMOTED_VAR_P (op0))
+op0_ext_redundant =
+ gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT (treeop0));
+
+ /* If promoted and the value in SUBREG of op1 fits (does not overflow),
+ it is a candidate for extension elimination. */
+ if (GET_CODE (op1) == SUBREG && SUBREG_PROMOTED_VAR_P (op1))
+op1_ext_redundant =
+ gimple_assign_is_zero_sign_ext_redundant (SSA_NAME_DEF_STMT (treeop1));
Are the gimple_assign_is_zero_sign_ext_redundant checks necessary here?
When set on a SUBREG, SUBREG_PROMOTED_VAR_P guarantees that SUBREG_REG is
always properly extended (otherwise it's a bug) so don't you just need to
compare SUBREG_PROMOTED_UNSIGNED_SET? See do_jump for an existing case.
I am sorry I don’t think I understood you here. How would I know that
extension is redundant without calling
gimple_assign_is_zero_sign_ext_redundant ? Could you please elaborate.
+ /* If zero/sign extension is redundant, generate RTL
+ for operands without zero/sign extension. */
+ if ((op0_ext_redundant || TREE_CODE (treeop0) == INTEGER_CST)
+ && (op1_ext_redundant || TREE_CODE (treeop1) == INTEGER_CST))
Don't you need to be careful with the INTEGER_CSTs here? The CONST_INTs are
always sign-extended in RTL so 0x80 is always re
Re: [PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
[Sorry for the delay]
> For example, when an expression is evaluated and it's value is assigned
> to variable of type short, the generated RTL would look something like
> the following.
>
> (set (reg:SI 110)
> (zero_extend:SI (subreg:HI (reg:SI 117) 0)))
>
> However, if during value range propagation, if we can say for certain
> that the value of the expression which is present in register 117 is
> within the limits of short and there is no sign conversion, we do not
> need to perform the subreg and zero_extend; instead we can generate the
> following RTl.
>
> (set (reg:SI 110)
> (reg:SI 117)))
>
> Same could be done for other assign statements.
The idea looks interesting. Some remarks:
> +2013-06-03 Kugan Vivekanandarajah
> +
> + * gcc/dojump.c (do_compare_and_jump): generates rtl without
> + zero/sign extension if redundant.
> + * gcc/cfgexpand.c (expand_gimple_stmt_1): Likewise.
> + * gcc/gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New
> + function.
> + * gcc/gimple.h (gimple_assign_is_zero_sign_ext_redundant) : New
> + function definition.
No gcc/ prefix in entries for gcc/ChangeLog. "Generate RTL without..."
+/* 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
[ping^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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 + +* 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. +
[ping][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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
+
+* 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.
+
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_o
[PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
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
+
+ * 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.
+
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
