Re: [PATCH] Improve COND_EXPR expansion
On Tue, May 1, 2012 at 9:45 AM, Richard Henderson r...@redhat.com wrote: On 04/30/2012 08:22 PM, Andrew Pinski wrote: * expr.c (convert_tree_comp_to_rtx): New function. (expand_expr_real_2): Try using conditional moves for COND_EXPRs if they exist. * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison modes of DImode for 32bits and TImode. Looks ok, modulo some unwrapped long lines. Those lines might not be so long if this were split out as a subroutine... I'll not quibble about that though... Here is a new patch which splits this out into a subroutine which reduces the long lines. I added some extra comments that might make some stuff like the extra can_conditionally_move_p check. OK? Bootstrapped and tested on x86_64-linux-gnu. Thanks, Andrew Pinski ChangeLog: * expr.c (get_def_for_expr_class): New function. (convert_tree_comp_to_rtx): New function. (expand_cond_expr_using_cmove): New function. (expand_expr_real_2 case COND_EXPR): Call expand_cond_expr_using_cmove first and return if it succeeds. Remove the check for HAVE_conditional_move since we should have already converted it to a conditional move. * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison modes of DImode for 32bits and TImode.
Re: [PATCH] Improve COND_EXPR expansion
On Fri, May 4, 2012 at 12:29 AM, Andrew Pinski
andrew.pin...@caviumnetworks.com wrote:
On Tue, May 1, 2012 at 9:45 AM, Richard Henderson r...@redhat.com wrote:
On 04/30/2012 08:22 PM, Andrew Pinski wrote:
* expr.c (convert_tree_comp_to_rtx): New function.
(expand_expr_real_2): Try using conditional moves for COND_EXPRs if they
exist.
* config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison
modes of DImode for 32bits and TImode.
Looks ok, modulo some unwrapped long lines.
Those lines might not be so long if this were split out as a subroutine...
I'll not quibble about that though...
Here is a new patch which splits this out into a subroutine which
reduces the long lines. I added some extra comments that might make
some stuff like the extra can_conditionally_move_p check.
OK? Bootstrapped and tested on x86_64-linux-gnu.
This time with the patch.
-- Andrew
Thanks,
Andrew Pinski
ChangeLog:
* expr.c (get_def_for_expr_class): New function.
(convert_tree_comp_to_rtx): New function.
(expand_cond_expr_using_cmove): New function.
(expand_expr_real_2 case COND_EXPR): Call
expand_cond_expr_using_cmove first and return if it succeeds.
Remove the check for HAVE_conditional_move since we should have
already converted it to a conditional move.
* config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison
modes of DImode for 32bits and TImode.
Index: gcc/expr.c
===
--- gcc/expr.c (revision 187138)
+++ gcc/expr.c (working copy)
@@ -2346,6 +2346,26 @@ get_def_for_expr (tree name, enum tree_c
return def_stmt;
}
+
+/* Return the defining gimple statement for SSA_NAME NAME if it is an
+ assigment and the class of the expresion on the RHS is CLASS. Return
+ NULL otherwise. */
+
+static gimple
+get_def_for_expr_class (tree name, enum tree_code_class tclass)
+{
+ gimple def_stmt;
+
+ if (TREE_CODE (name) != SSA_NAME)
+return NULL;
+
+ def_stmt = get_gimple_for_ssa_name (name);
+ if (!def_stmt
+ || TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt)) != tclass)
+return NULL;
+
+ return def_stmt;
+}
/* Determine whether the LEN bytes generated by CONSTFUN can be
@@ -7344,6 +7364,64 @@ highest_pow2_factor_for_target (const_tr
return MAX (factor, talign);
}
+/* Convert the tree comparision code TCODE to the rtl one where the
+ signedness is UNSIGNEDP. */
+
+static enum rtx_code
+convert_tree_comp_to_rtx (enum tree_code tcode, int unsignedp)
+{
+ enum rtx_code code;
+ switch (tcode)
+{
+case EQ_EXPR:
+ code = EQ;
+ break;
+case NE_EXPR:
+ code = NE;
+ break;
+case LT_EXPR:
+ code = unsignedp ? LTU : LT;
+ break;
+case LE_EXPR:
+ code = unsignedp ? LEU : LE;
+ break;
+case GT_EXPR:
+ code = unsignedp ? GTU : GT;
+ break;
+case GE_EXPR:
+ code = unsignedp ? GEU : GE;
+ break;
+case UNORDERED_EXPR:
+ code = UNORDERED;
+ break;
+case ORDERED_EXPR:
+ code = ORDERED;
+ break;
+case UNLT_EXPR:
+ code = UNLT;
+ break;
+case UNLE_EXPR:
+ code = UNLE;
+ break;
+case UNGT_EXPR:
+ code = UNGT;
+ break;
+case UNGE_EXPR:
+ code = UNGE;
+ break;
+case UNEQ_EXPR:
+ code = UNEQ;
+ break;
+case LTGT_EXPR:
+ code = LTGT;
+ break;
+
+default:
+ gcc_unreachable ();
+}
+ return code;
+}
+
/* Subroutine of expand_expr. Expand the two operands of a binary
expression EXP0 and EXP1 placing the results in OP0 and OP1.
The value may be stored in TARGET if TARGET is nonzero. The
@@ -7782,6 +7860,96 @@ expand_expr_real (tree exp, rtx target,
return ret;
}
+/* Try to expand the conditional expression which is represented by
+ TREEOP0 ? TREEOP1 : TREEOP2 using conditonal moves. If succeseds
+ return the rtl reg which repsents the result. Otherwise return NULL_RTL.
*/
+
+static rtx
+expand_cond_expr_using_cmove (tree treeop0, tree treeop1, tree treeop2)
+{
+#ifdef HAVE_conditional_move
+ rtx insn;
+ rtx op00, op01, op1, op2;
+ enum rtx_code comparison_code;
+ enum machine_mode comparison_mode;
+ gimple srcstmt;
+ rtx temp;
+ tree type = TREE_TYPE (treeop1);
+ int unsignedp = TYPE_UNSIGNED (type);
+ enum machine_mode mode = TYPE_MODE (type);
+
+ temp = assign_temp (type, 0, 0, 1);
+
+ /* If we cannot do a conditional move on the mode, try doing it
+ with the promoted mode. */
+ if (!can_conditionally_move_p (mode))
+mode = promote_mode (type, mode, unsignedp);
+
+ if (!can_conditionally_move_p (mode))
+return NULL_RTX;
+
+ start_sequence ();
+ expand_operands (treeop1, treeop2,
+ temp, op1, op2, EXPAND_NORMAL);
+
+ if (TREE_CODE (treeop0) == SSA_NAME
+ (srcstmt = get_def_for_expr_class (treeop0, tcc_comparison)))
+{
+ tree type = TREE_TYPE (gimple_assign_rhs1 (srcstmt));
+ enum tree_code cmpcode =
Re: [PATCH] Improve COND_EXPR expansion
On 05/04/12 00:30, Andrew Pinski wrote:
+static rtx
+expand_cond_expr_using_cmove (tree treeop0, tree treeop1, tree treeop2)
+{
+#ifdef HAVE_conditional_move
You'll need ATTRIBUTE_UNUSED markers here.
Otherwise ok.
r~
Re: [PATCH] Improve COND_EXPR expansion
On Mon, 2012-04-30 at 20:22 -0700, Andrew Pinski wrote: Hi, This patch improves the expansion of COND_EXPR into RTL, directly using conditional moves. I had to fix a bug in the x86 backend where emit_conditional_move could cause a crash as we had a comparison mode of DImode which is not handled by the 32bit part. can_conditionally_move_p return true as we had an SImode for the other operands. Note other targets might need a similar fix as x86 had but I could not test those targets and this is really the first time where emit_conditional_move is being called with different modes for the comparison and the other operands mode and the comparison mode is not of the CC class. Hi Andrew, I verified your patch on powerpc64-unknown-linux-gnu. There were no new testcase regressions, and SPEC cpu2006 built ok with your changes. Hope this helps! Bill The main reasoning to do this conversion early rather than wait for ifconv as the resulting code is slightly better. Also the compiler is slightly faster. OK? Bootstrapped and tested on both mips64-linux-gnu (where it was originally written for) and x86_64-linux-gnu. Thanks, Andrew Pinski ChangeLog: * expr.c (convert_tree_comp_to_rtx): New function. (expand_expr_real_2): Try using conditional moves for COND_EXPRs if they exist. * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison modes of DImode for 32bits and TImode.
Re: [PATCH] Improve COND_EXPR expansion
On 04/30/2012 08:22 PM, Andrew Pinski wrote: * expr.c (convert_tree_comp_to_rtx): New function. (expand_expr_real_2): Try using conditional moves for COND_EXPRs if they exist. * config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison modes of DImode for 32bits and TImode. Looks ok, modulo some unwrapped long lines. Those lines might not be so long if this were split out as a subroutine... I'll not quibble about that though... r~
[PATCH] Improve COND_EXPR expansion
Hi,
This patch improves the expansion of COND_EXPR into RTL, directly
using conditional moves.
I had to fix a bug in the x86 backend where emit_conditional_move
could cause a crash as we had a comparison mode of DImode which is not
handled by the 32bit part. can_conditionally_move_p return true as we
had an SImode for the other operands.
Note other targets might need a similar fix as x86 had but I could not
test those targets and this is really the first time where
emit_conditional_move is being called with different modes for the
comparison and the other operands mode and the comparison mode is not
of the CC class.
The main reasoning to do this conversion early rather than wait for
ifconv as the resulting code is slightly better. Also the compiler is
slightly faster.
OK? Bootstrapped and tested on both mips64-linux-gnu (where it was
originally written for) and x86_64-linux-gnu.
Thanks,
Andrew Pinski
ChangeLog:
* expr.c (convert_tree_comp_to_rtx): New function.
(expand_expr_real_2): Try using conditional moves for COND_EXPRs if they exist.
* config/i386/i386.c (ix86_expand_int_movcc): Disallow comparison
modes of DImode for 32bits and TImode.
Index: expr.c
===
--- expr.c (revision 186954)
+++ expr.c (working copy)
@@ -7344,6 +7344,64 @@ highest_pow2_factor_for_target (const_tr
return MAX (factor, talign);
}
+/* Convert the tree comparision code TCODE to the rtl one where the
+ signedness is UNSIGNEDP. */
+
+static enum rtx_code
+convert_tree_comp_to_rtx (enum tree_code tcode, int unsignedp)
+{
+ enum rtx_code code;
+ switch (tcode)
+{
+case EQ_EXPR:
+ code = EQ;
+ break;
+case NE_EXPR:
+ code = NE;
+ break;
+case LT_EXPR:
+ code = unsignedp ? LTU : LT;
+ break;
+case LE_EXPR:
+ code = unsignedp ? LEU : LE;
+ break;
+case GT_EXPR:
+ code = unsignedp ? GTU : GT;
+ break;
+case GE_EXPR:
+ code = unsignedp ? GEU : GE;
+ break;
+case UNORDERED_EXPR:
+ code = UNORDERED;
+ break;
+case ORDERED_EXPR:
+ code = ORDERED;
+ break;
+case UNLT_EXPR:
+ code = UNLT;
+ break;
+case UNLE_EXPR:
+ code = UNLE;
+ break;
+case UNGT_EXPR:
+ code = UNGT;
+ break;
+case UNGE_EXPR:
+ code = UNGE;
+ break;
+case UNEQ_EXPR:
+ code = UNEQ;
+ break;
+case LTGT_EXPR:
+ code = LTGT;
+ break;
+
+default:
+ gcc_unreachable ();
+}
+ return code;
+}
+
/* Subroutine of expand_expr. Expand the two operands of a binary
expression EXP0 and EXP1 placing the results in OP0 and OP1.
The value may be stored in TARGET if TARGET is nonzero. The
@@ -8851,8 +8909,7 @@ expand_expr_real_2 (sepops ops, rtx targ
safe_from_p (original_target, treeop0, 1)
GET_MODE (original_target) == mode
#ifdef HAVE_conditional_move
- (! can_conditionally_move_p (mode)
- || REG_P (original_target))
+ 0
#endif
!MEM_P (original_target))
temp = original_target;
@@ -8860,6 +8917,82 @@ expand_expr_real_2 (sepops ops, rtx targ
temp = assign_temp (type, 0, 0, 1);
do_pending_stack_adjust ();
+#if HAVE_conditional_move
+ if (!can_conditionally_move_p (mode))
+mode = promote_mode (type, mode, unsignedp);
+ if (can_conditionally_move_p (mode))
+ {
+ rtx insn;
+ rtx op00, op01;
+ enum rtx_code comparison_code;
+ enum machine_mode comparison_mode;
+ start_sequence ();
+
+ expand_operands (treeop1, treeop2,
+ temp, op1, op2, EXPAND_NORMAL);
+ if (TREE_CODE (treeop0) == SSA_NAME)
+ {
+ gimple srcstmt;
+ srcstmt = get_gimple_for_ssa_name (treeop0);
+ if (srcstmt
+ TREE_CODE_CLASS (gimple_assign_rhs_code (srcstmt))
+ == tcc_comparison)
+ {
+ tree type = TREE_TYPE (gimple_assign_rhs1 (srcstmt));
+ op00 = expand_normal (gimple_assign_rhs1 (srcstmt));
+ op01 = expand_normal (gimple_assign_rhs2 (srcstmt));
+ comparison_code = convert_tree_comp_to_rtx
(gimple_assign_rhs_code (srcstmt), TYPE_UNSIGNED (type));
+ comparison_mode = TYPE_MODE (type);
+ unsignedp = TYPE_UNSIGNED (type);
+ }
+ else
+ goto non_comparison_cond_expr;
+ }
+ else if (TREE_CODE_CLASS (TREE_CODE (treeop0)) == tcc_comparison)
+ {
+ tree type = TREE_TYPE (TREE_OPERAND (treeop0, 0));
+ op00 = expand_normal (TREE_OPERAND (treeop0, 0));
+ op01 = expand_normal (TREE_OPERAND (treeop0, 1));
+ comparison_code = convert_tree_comp_to_rtx (TREE_CODE (treeop0),
TYPE_UNSIGNED (type));
+ comparison_mode = TYPE_MODE
