Hi!
Uros noted in the PR that in many cases with floating point comparisons
ifcvt fails to RTL if-convert or end up being RTL if-converted in worse
way than it could be (e.g. something that ought to be noce_try_addcc
optimizable is only if-converted using noce_try_cmove_arith, resulting
in worse performance).
The problem is that reversed_comparison_code fails (returns UNKNOWN)
in many cases, because important information is lost during
canonicalize_condition. Initially we e.g. have
(ge (reg:CCFP 17 flags)
(const_int 0 [0]))
which is canonicalized non-reversed as:
(ge (reg:DF 100)
(reg:DF 97))
and reversed as:
(unlt (reg:DF 100)
(reg:DF 97))
But as soon as we only have the (unlt (reg:DF 100) (reg:DF 97)),
reversed_comparison_code fails on it:
case UNLT:
case UNLE:
case UNGT:
case UNGE:
/* We don't have safe way to reverse these yet. */
return UNKNOWN;
On the benchmark (at -O2, at -O3 we run into a split-path pass messing
it up so ifcvt is not possible) due to the order/content of then/else
branches, we canonicalize the condition as reversed, so if_info->cond
is that UNLT, and when we want to reverse it again in e.g. noce_try_addcc
(i.e. get the original non-reversed, just canonicalized), it fails.
The following patch fixes that by remembering both the condition and
reverse condition in noce_if_info structure (if the latter is successful),
and then using the rev_cond instead of cond if we need to reverse cond.
The reversed_comparison_code calls are kept as fallback, e.g. some
functions change if_info->cond and then we don't have the reverse condition
for that, or canonicalize_condition could fail for the reverse.
If rev_cond is available, the advantage is also that that condition is
in canonic form, while just by using reversed_comparison_code and
the original cond's arguments it can be non-canonic.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-02-23 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/79389
* ifcvt.c (struct noce_if_info): Add rev_cond field.
(noce_emit_store_flag): Use rev_cond if non-NULL instead of
reversed_comparison_code. Formatting fix.
(noce_try_store_flag): Test rev_cond != NULL in addition to
reversed_comparison_code.
(noce_try_store_flag_constants): Likewise.
(noce_try_store_flag_mask): Likewise.
(noce_try_addcc): Use rev_cond if non-NULL instead of
reversed_comparison_code.
(noce_try_cmove_arith): Likewise. Formatting fixes.
(noce_try_minmax, noce_try_abs): Clear rev_cond.
(noce_find_if_block): Initialize rev_cond.
(find_cond_trap): If reversed_comparison_code fails, try
noce_get_condition with true as last argument.
--- gcc/ifcvt.c.jj 2017-02-22 22:32:34.411724499 +0100
+++ gcc/ifcvt.c 2017-02-23 10:24:09.364416631 +0100
@@ -777,6 +777,9 @@ struct noce_if_info
/* The jump condition. */
rtx cond;
+ /* Reversed jump condition. */
+ rtx rev_cond;
+
/* New insns should be inserted before this one. */
rtx_insn *cond_earliest;
@@ -888,6 +891,14 @@ noce_emit_store_flag (struct noce_if_inf
if (if_info->then_else_reversed)
reversep = !reversep;
}
+ else if (reversep
+ && if_info->rev_cond
+ && general_operand (XEXP (if_info->rev_cond, 0), VOIDmode)
+ && general_operand (XEXP (if_info->rev_cond, 1), VOIDmode))
+ {
+ cond = if_info->rev_cond;
+ reversep = false;
+ }
if (reversep)
code = reversed_comparison_code (cond, if_info->jump);
@@ -898,7 +909,7 @@ noce_emit_store_flag (struct noce_if_inf
&& (normalize == 0 || STORE_FLAG_VALUE == normalize))
{
rtx src = gen_rtx_fmt_ee (code, GET_MODE (x), XEXP (cond, 0),
- XEXP (cond, 1));
+ XEXP (cond, 1));
rtx set = gen_rtx_SET (x, src);
start_sequence ();
@@ -1209,8 +1220,9 @@ noce_try_store_flag (struct noce_if_info
else if (if_info->b == const0_rtx
&& CONST_INT_P (if_info->a)
&& INTVAL (if_info->a) == STORE_FLAG_VALUE
- && (reversed_comparison_code (if_info->cond, if_info->jump)
- != UNKNOWN))
+ && (if_info->rev_cond
+ || reversed_comparison_code (if_info->cond,
+ if_info->jump) != UNKNOWN))
reversep = 1;
else
return FALSE;
@@ -1371,8 +1383,9 @@ noce_try_store_flag_constants (struct no
diff = trunc_int_for_mode (diff, mode);
- can_reverse = (reversed_comparison_code (if_info->cond, if_info->jump)
- != UNKNOWN);
+ can_reverse = (if_info->rev_cond
+ || reversed_comparison_code (if_info->cond,
+ if_info->jump) != UNKNOWN);
reversep = false;
if (diff == STORE_FLAG_VALUE || diff == -STORE_FLAG_VALUE)
@@ -1553,11 +1566,20 @@ noce_try_addcc (struct noce_if_info *if_
if (GET_CODE (if_info->a) == PLUS
&& rtx_equal_p (XEXP (if_info->a, 0), if_info->b)
- && (reversed_comparison_code (if_info->cond, if_info->jump)
- != UNKNOWN))
+ && (if_info->rev_cond
+ || reversed_comparison_code (if_info->cond,
+ if_info->jump) != UNKNOWN))
{
- rtx cond = if_info->cond;
- enum rtx_code code = reversed_comparison_code (cond, if_info->jump);
+ rtx cond = if_info->rev_cond;
+ enum rtx_code code;
+
+ if (cond == NULL_RTX)
+ {
+ cond = if_info->cond;
+ code = reversed_comparison_code (cond, if_info->jump);
+ }
+ else
+ code = GET_CODE (cond);
/* First try to use addcc pattern. */
if (general_operand (XEXP (cond, 0), VOIDmode)
@@ -1652,9 +1674,9 @@ noce_try_store_flag_mask (struct noce_if
if ((if_info->a == const0_rtx
&& rtx_equal_p (if_info->b, if_info->x))
- || ((reversep = (reversed_comparison_code (if_info->cond,
- if_info->jump)
- != UNKNOWN))
+ || ((reversep = (if_info->rev_cond
+ || reversed_comparison_code (if_info->cond,
+ if_info->jump) != UNKNOWN))
&& if_info->b == const0_rtx
&& rtx_equal_p (if_info->a, if_info->x)))
{
@@ -2086,6 +2108,7 @@ noce_try_cmove_arith (struct noce_if_inf
rtx target;
int is_mem = 0;
enum rtx_code code;
+ rtx cond = if_info->cond;
rtx_insn *ifcvt_seq;
/* A conditional move from two memory sources is equivalent to a
@@ -2117,7 +2140,7 @@ noce_try_cmove_arith (struct noce_if_inf
x = y;
*/
- code = GET_CODE (if_info->cond);
+ code = GET_CODE (cond);
insn_a = if_info->insn_a;
insn_b = if_info->insn_b;
@@ -2127,7 +2150,8 @@ noce_try_cmove_arith (struct noce_if_inf
return FALSE;
/* Possibly rearrange operands to make things come out more natural. */
- if (reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN)
+ if (if_info->rev_cond
+ || reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN)
{
int reversep = 0;
if (rtx_equal_p (b, x))
@@ -2137,7 +2161,13 @@ noce_try_cmove_arith (struct noce_if_inf
if (reversep)
{
- code = reversed_comparison_code (if_info->cond, if_info->jump);
+ if (if_info->rev_cond)
+ {
+ cond = if_info->rev_cond;
+ code = GET_CODE (cond);
+ }
+ else
+ code = reversed_comparison_code (cond, if_info->jump);
std::swap (a, b);
std::swap (insn_a, insn_b);
std::swap (a_simple, b_simple);
@@ -2173,7 +2203,7 @@ noce_try_cmove_arith (struct noce_if_inf
rtx emit_b = NULL_RTX;
rtx_insn *tmp_insn = NULL;
bool modified_in_a = false;
- bool modified_in_b = false;
+ bool modified_in_b = false;
/* If either operand is complex, load it into a register first.
The best way to do this is to copy the original insn. In this
way we preserve any clobbers etc that the insn may have had.
@@ -2231,7 +2261,7 @@ noce_try_cmove_arith (struct noce_if_inf
rtx tmp_reg = tmp_b ? tmp_b : gen_reg_rtx (GET_MODE (b));
emit_b = gen_rtx_SET (tmp_reg, b);
b = tmp_reg;
- }
+ }
}
}
@@ -2286,8 +2316,8 @@ noce_try_cmove_arith (struct noce_if_inf
else
goto end_seq_and_fail;
- target = noce_emit_cmove (if_info, x, code, XEXP (if_info->cond, 0),
- XEXP (if_info->cond, 1), a, b);
+ target = noce_emit_cmove (if_info, x, code, XEXP (cond, 0), XEXP (cond, 1),
+ a, b);
if (! target)
goto end_seq_and_fail;
@@ -2576,6 +2606,7 @@ noce_try_minmax (struct noce_if_info *if
emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION
(if_info->insn_a));
if_info->cond = cond;
if_info->cond_earliest = earliest;
+ if_info->rev_cond = NULL_RTX;
if_info->transform_name = "noce_try_minmax";
return TRUE;
@@ -2743,6 +2774,7 @@ noce_try_abs (struct noce_if_info *if_in
emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION
(if_info->insn_a));
if_info->cond = cond;
if_info->cond_earliest = earliest;
+ if_info->rev_cond = NULL_RTX;
if_info->transform_name = "noce_try_abs";
return TRUE;
@@ -4064,6 +4096,11 @@ noce_find_if_block (basic_block test_bb,
if_info.else_bb = else_bb;
if_info.join_bb = join_bb;
if_info.cond = cond;
+ rtx_insn *rev_cond_earliest;
+ if_info.rev_cond = noce_get_condition (jump, &rev_cond_earliest,
+ !then_else_reversed);
+ gcc_assert (if_info.rev_cond == NULL_RTX
+ || rev_cond_earliest == cond_earliest);
if_info.cond_earliest = cond_earliest;
if_info.jump = jump;
if_info.then_else_reversed = then_else_reversed;
@@ -4676,7 +4713,12 @@ find_cond_trap (basic_block test_bb, edg
{
code = reversed_comparison_code (cond, jump);
if (code == UNKNOWN)
- return FALSE;
+ {
+ cond = noce_get_condition (jump, &cond_earliest, true);
+ if (!cond)
+ return FALSE;
+ code = GET_CODE (cond);
+ }
}
/* Attempt to generate the conditional trap. */
Jakub
