On Thu, Feb 23, 2017 at 02:47:11PM +0100, Bernd Schmidt wrote:
> On 02/23/2017 02:36 PM, Jakub Jelinek wrote:
> > and both UNLT and GE can be reversed. But if the arguments of the condition
> > are canonicalized, we run into:
> > /* Test for an integer condition, or a floating-point comparison
> > in which NaNs can be ignored. */
> > if (CONST_INT_P (arg0)
> > || (GET_MODE (arg0) != VOIDmode
> > && GET_MODE_CLASS (mode) != MODE_CC
> > && !HONOR_NANS (mode)))
> > return reverse_condition (code);
> > and thus always return UNKNOWN.
>
> So... do you think we could add (in gcc-8, probably, although if it fixes
> this regression...)
>
> else if (GET_MODE (arg0) != VOIDmode
> && GET_MODE_CLASS (mode) != MODE_CC
> && HONOR_NANS (mode))
> return reverse_condition_maybe_unordered (code);
>
> to make this work?
Maybe, though I'd feel safer about trying it only in gcc 8. I can certainly
test such a change on a couple of targets. It would not be sufficient, we'd
either need to also reverse_condition_maybe_unordered for the UN* codes
we don't handle yet, or break so that we perhaps reach this spot.
Updated (not yet tested) version of the patch is below.
2017-02-23 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/79389
* ifcvt.c (struct noce_if_info): Add rev_cond field.
(noce_reversed_cond_code): New function.
(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): Call noce_get_condition with then_bb == trap_bb
instead of false as last argument never attempt to reverse it
afterwards.
--- gcc/ifcvt.c.jj 2017-02-22 22:32:34.411724499 +0100
+++ gcc/ifcvt.c 2017-02-23 14:45:54.011715821 +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;
@@ -843,6 +846,17 @@ static int noce_try_minmax (struct noce_
static int noce_try_abs (struct noce_if_info *);
static int noce_try_sign_mask (struct noce_if_info *);
+/* Return the comparison code for reversed condition for IF_INFO,
+ or UNKNOWN if reversing the condition is not possible. */
+
+static inline enum rtx_code
+noce_reversed_cond_code (struct noce_if_info *if_info)
+{
+ if (if_info->rev_cond)
+ return GET_CODE (if_info->rev_cond);
+ return reversed_comparison_code (if_info->cond, if_info->jump);
+}
+
/* Return TRUE if SEQ is a good candidate as a replacement for the
if-convertible sequence described in IF_INFO. */
@@ -888,6 +902,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 +920,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 +1231,7 @@ 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))
+ && noce_reversed_cond_code (if_info) != UNKNOWN)
reversep = 1;
else
return FALSE;
@@ -1371,9 +1392,7 @@ 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 = noce_reversed_cond_code (if_info) != UNKNOWN;
reversep = false;
if (diff == STORE_FLAG_VALUE || diff == -STORE_FLAG_VALUE)
{
@@ -1553,11 +1572,18 @@ 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))
+ && noce_reversed_cond_code (if_info) != 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 +1678,7 @@ 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 = (noce_reversed_cond_code (if_info) != UNKNOWN))
&& if_info->b == const0_rtx
&& rtx_equal_p (if_info->a, if_info->x)))
{
@@ -2086,6 +2110,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 +2142,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 +2152,7 @@ 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 (noce_reversed_cond_code (if_info) != UNKNOWN)
{
int reversep = 0;
if (rtx_equal_p (b, x))
@@ -2137,7 +2162,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 +2204,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 +2262,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 +2317,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 +2607,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 +2775,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 +4097,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;
@@ -4634,7 +4672,6 @@ find_cond_trap (basic_block test_bb, edg
rtx_insn *trap, *jump;
rtx cond;
rtx_insn *cond_earliest;
- enum rtx_code code;
/* Locate the block with the trap instruction. */
/* ??? While we look for no successors, we really ought to allow
@@ -4654,7 +4691,7 @@ find_cond_trap (basic_block test_bb, edg
/* If this is not a standard conditional jump, we can't parse it. */
jump = BB_END (test_bb);
- cond = noce_get_condition (jump, &cond_earliest, false);
+ cond = noce_get_condition (jump, &cond_earliest, then_bb == trap_bb);
if (! cond)
return FALSE;
@@ -4670,17 +4707,8 @@ find_cond_trap (basic_block test_bb, edg
if (GET_MODE (XEXP (cond, 0)) == BLKmode)
return FALSE;
- /* Reverse the comparison code, if necessary. */
- code = GET_CODE (cond);
- if (then_bb == trap_bb)
- {
- code = reversed_comparison_code (cond, jump);
- if (code == UNKNOWN)
- return FALSE;
- }
-
/* Attempt to generate the conditional trap. */
- rtx_insn *seq = gen_cond_trap (code, copy_rtx (XEXP (cond, 0)),
+ rtx_insn *seq = gen_cond_trap (GET_CODE (cond), copy_rtx (XEXP (cond, 0)),
copy_rtx (XEXP (cond, 1)),
TRAP_CODE (PATTERN (trap)));
if (seq == NULL)
Jakub
