It might make sense to move:
/* Don't even try if the comparison operands are weird
except that the target supports cbranchcc4. */
if (! general_operand (cmp_a, GET_MODE (cmp_a))
|| ! general_operand (cmp_b, GET_MODE (cmp_b)))
{
if (!have_cbranchcc4
|| GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
|| cmp_b != const0_rtx)
return NULL_RTX;
}
into the “else” arm, since it seems odd to be checking cmp_a and cmp_b
when we're not going to use them. Looks like the later call to
emit_conditional_move should get the same treatment.
Moved it.
Why's that the case though? The swapped form is the canonical one,
so it's the one that the target ought to accept.
My wording was a bit misleading in the comment and I tried to improve it
in the attached v2.
Before, we had a loop with two iterations that tried "emit_cmov (cond,
op2, op3)". op2 and op3 can (or even were) If this did not succeed we
would revert the condition as well as op3 and op3 in-place and try
again. I found that a bit cumbersome and intended to make this explicit
but it's still kind of involved, particularly since cond may come in
reversed, we additionally swap op2 and op3 and all the way back again.
I remember from the first iteration of these patches that this (double
revert) was needed for exactly one test case in the test suite on x86.
When re-running it today I could not reproduce it anymore, though.
Regards
Robin
>From 0526fa57290941c8b7febd138ab4c637b83f3d05 Mon Sep 17 00:00:00 2001
From: Robin Dapp <rd...@linux.ibm.com>
Date: Wed, 27 Nov 2019 13:53:40 +0100
Subject: [PATCH v2 4/7] ifcvt/optabs: Allow using a CC comparison for
emit_conditional_move.
Currently we only ever call emit_conditional_move with the comparison
(as well as its comparands) we got from the jump. Thus, backends are
going to emit a CC comparison for every conditional move that is being
generated instead of re-using the existing CC.
This, combined with emitting temporaries for each conditional move,
causes sky-high costs for conditional moves.
This patch allows to re-use a CC so the costing situation is improved a
bit.
---
gcc/ifcvt.c | 40 ++++++++-----
gcc/optabs.c | 164 ++++++++++++++++++++++++++++++++++-----------------
gcc/optabs.h | 1 +
3 files changed, 135 insertions(+), 70 deletions(-)
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 91b54dbea9c..7ce60d0af8d 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -771,7 +771,7 @@ static int noce_try_addcc (struct noce_if_info *);
static int noce_try_store_flag_constants (struct noce_if_info *);
static int noce_try_store_flag_mask (struct noce_if_info *);
static rtx noce_emit_cmove (struct noce_if_info *, rtx, enum rtx_code, rtx,
- rtx, rtx, rtx);
+ rtx, rtx, rtx, rtx = NULL, rtx = NULL);
static int noce_try_cmove (struct noce_if_info *);
static int noce_try_cmove_arith (struct noce_if_info *);
static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **);
@@ -1710,7 +1710,8 @@ noce_try_store_flag_mask (struct noce_if_info *if_info)
static rtx
noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code,
- rtx cmp_a, rtx cmp_b, rtx vfalse, rtx vtrue)
+ rtx cmp_a, rtx cmp_b, rtx vfalse, rtx vtrue, rtx cc_cmp,
+ rtx rev_cc_cmp)
{
rtx target ATTRIBUTE_UNUSED;
int unsignedp ATTRIBUTE_UNUSED;
@@ -1742,23 +1743,30 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code,
end_sequence ();
}
- /* Don't even try if the comparison operands are weird
- except that the target supports cbranchcc4. */
- if (! general_operand (cmp_a, GET_MODE (cmp_a))
- || ! general_operand (cmp_b, GET_MODE (cmp_b)))
- {
- if (!have_cbranchcc4
- || GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
- || cmp_b != const0_rtx)
- return NULL_RTX;
- }
-
unsignedp = (code == LTU || code == GEU
|| code == LEU || code == GTU);
- target = emit_conditional_move (x, code, cmp_a, cmp_b, VOIDmode,
- vtrue, vfalse, GET_MODE (x),
- unsignedp);
+ if (cc_cmp != NULL_RTX && rev_cc_cmp != NULL_RTX)
+ target = emit_conditional_move (x, cc_cmp, rev_cc_cmp,
+ vtrue, vfalse, GET_MODE (x));
+ else
+ {
+ /* Don't even try if the comparison operands are weird
+ except that the target supports cbranchcc4. */
+ if (! general_operand (cmp_a, GET_MODE (cmp_a))
+ || ! general_operand (cmp_b, GET_MODE (cmp_b)))
+ {
+ if (!have_cbranchcc4
+ || GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
+ || cmp_b != const0_rtx)
+ return NULL_RTX;
+ }
+
+ target = emit_conditional_move (x, code, cmp_a, cmp_b, VOIDmode,
+ vtrue, vfalse, GET_MODE (x),
+ unsignedp);
+ }
+
if (target)
return target;
diff --git a/gcc/optabs.c b/gcc/optabs.c
index 62a6bdb4c59..7410f23e0ad 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -52,6 +52,8 @@ static void prepare_float_lib_cmp (rtx, rtx, enum rtx_code, rtx *,
static rtx expand_unop_direct (machine_mode, optab, rtx, rtx, int);
static void emit_libcall_block_1 (rtx_insn *, rtx, rtx, rtx, bool);
+static rtx emit_conditional_move (rtx, rtx, rtx, rtx, machine_mode);
+
/* Debug facility for use in GDB. */
void debug_optab_libfuncs (void);
@@ -4747,7 +4749,6 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1,
machine_mode mode, int unsignedp)
{
rtx comparison;
- rtx_insn *last;
enum insn_code icode;
enum rtx_code reversed;
@@ -4774,6 +4775,7 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1,
/* get_condition will prefer to generate LT and GT even if the old
comparison was against zero, so undo that canonicalization here since
comparisons against zero are cheaper. */
+
if (code == LT && op1 == const1_rtx)
code = LE, op1 = const0_rtx;
else if (code == GT && op1 == constm1_rtx)
@@ -4782,17 +4784,30 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1,
if (cmode == VOIDmode)
cmode = GET_MODE (op0);
- enum rtx_code orig_code = code;
+ /* If the first condition operand is constant and the second is not, swap
+ them. In that case we also need to reverse the comparison.
+ If both are non-constant It is possible that the conditional move
+ will not expand with operands in this order, e.g. when we are about to
+ create a min/max expression and the operands do not match the condition.
+ In that case we also need the reversed condition code and comparison. */
+
+ rtx rev_comparison = NULL_RTX;
bool swapped = false;
- if (swap_commutative_operands_p (op2, op3)
- && ((reversed = reversed_comparison_code_parts (code, op0, op1, NULL))
- != UNKNOWN))
+
+ code = unsignedp ? unsigned_condition (code) : code;
+ comparison = simplify_gen_relational (code, VOIDmode, cmode, op0, op1);
+
+ if ((reversed = reversed_comparison_code_parts (code, op0, op1, NULL))
+ != UNKNOWN)
{
- std::swap (op2, op3);
- code = reversed;
- swapped = true;
+ reversed = unsignedp ? unsigned_condition (reversed) : reversed;
+ rev_comparison = simplify_gen_relational (reversed, VOIDmode, cmode,
+ op0, op1);
}
+ if (swap_commutative_operands_p (op2, op3) && reversed != UNKNOWN)
+ swapped = true;
+
if (mode == VOIDmode)
mode = GET_MODE (op2);
@@ -4804,58 +4819,99 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1,
if (!target)
target = gen_reg_rtx (mode);
- for (int pass = 0; ; pass++)
+ if (comparison && COMPARISON_P (comparison))
+ prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1),
+ GET_CODE (comparison), NULL_RTX,
+ unsignedp, OPTAB_WIDEN, &comparison, &cmode);
+ else
+ return NULL_RTX;
+
+ if (rev_comparison && COMPARISON_P (rev_comparison))
+ prepare_cmp_insn (XEXP (rev_comparison, 0), XEXP (rev_comparison, 1),
+ GET_CODE (rev_comparison), NULL_RTX,
+ unsignedp, OPTAB_WIDEN, &rev_comparison, &cmode);
+
+ if (!swapped)
+ return emit_conditional_move (target, comparison, rev_comparison,
+ op2, op3, mode);
+ else
+ return emit_conditional_move (target, rev_comparison, comparison,
+ op3, op2, mode);
+}
+
+/* Helper function for emitting a conditional move. Given a COMPARISON
+ and a reversed REV_COMPARISON it will try to expand a conditional move
+ with COMPARISON first and try with REV_COMPARISON if that fails. */
+
+rtx
+emit_conditional_move (rtx target, rtx comparison, rtx rev_comparison,
+ rtx op2, rtx op3, machine_mode mode)
+{
+
+ rtx res = emit_conditional_move (target, comparison, op2, op3, mode);
+
+ if (res != NULL_RTX)
+ return res;
+
+ return emit_conditional_move (target, rev_comparison, op3, op2, mode);
+}
+
+/* Helper for emitting a conditional move. */
+
+static rtx
+emit_conditional_move (rtx target, rtx comparison,
+ rtx op2, rtx op3, machine_mode mode)
+{
+ rtx_insn *last;
+ enum insn_code icode;
+
+ if (comparison == NULL_RTX || !COMPARISON_P (comparison))
+ return NULL_RTX;
+
+ /* If the two source operands are identical, that's just a move. */
+ if (rtx_equal_p (op2, op3))
{
- code = unsignedp ? unsigned_condition (code) : code;
- comparison = simplify_gen_relational (code, VOIDmode, cmode, op0, op1);
+ if (!target)
+ target = gen_reg_rtx (mode);
- /* We can get const0_rtx or const_true_rtx in some circumstances. Just
- punt and let the caller figure out how best to deal with this
- situation. */
- if (COMPARISON_P (comparison))
- {
- saved_pending_stack_adjust save;
- save_pending_stack_adjust (&save);
- last = get_last_insn ();
- do_pending_stack_adjust ();
- machine_mode cmpmode = cmode;
- prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1),
- GET_CODE (comparison), NULL_RTX, unsignedp,
- OPTAB_WIDEN, &comparison, &cmpmode);
- if (comparison)
- {
- class expand_operand ops[4];
+ emit_move_insn (target, op3);
+ return target;
+ }
- create_output_operand (&ops[0], target, mode);
- create_fixed_operand (&ops[1], comparison);
- create_input_operand (&ops[2], op2, mode);
- create_input_operand (&ops[3], op3, mode);
- if (maybe_expand_insn (icode, 4, ops))
- {
- if (ops[0].value != target)
- convert_move (target, ops[0].value, false);
- return target;
- }
- }
- delete_insns_since (last);
- restore_pending_stack_adjust (&save);
- }
+ if (mode == VOIDmode)
+ mode = GET_MODE (op2);
- if (pass == 1)
- return NULL_RTX;
+ icode = direct_optab_handler (movcc_optab, mode);
- /* If the preferred op2/op3 order is not usable, retry with other
- operand order, perhaps it will expand successfully. */
- if (swapped)
- code = orig_code;
- else if ((reversed = reversed_comparison_code_parts (orig_code, op0, op1,
- NULL))
- != UNKNOWN)
- code = reversed;
- else
- return NULL_RTX;
- std::swap (op2, op3);
+ if (icode == CODE_FOR_nothing)
+ return NULL_RTX;
+
+ if (!target)
+ target = gen_reg_rtx (mode);
+
+ saved_pending_stack_adjust save;
+ save_pending_stack_adjust (&save);
+ last = get_last_insn ();
+ do_pending_stack_adjust ();
+
+ class expand_operand ops[4];
+
+ create_output_operand (&ops[0], target, mode);
+ create_fixed_operand (&ops[1], comparison);
+ create_input_operand (&ops[2], op2, mode);
+ create_input_operand (&ops[3], op3, mode);
+
+ if (maybe_expand_insn (icode, 4, ops))
+ {
+ if (ops[0].value != target)
+ convert_move (target, ops[0].value, false);
+ return target;
}
+
+ delete_insns_since (last);
+ restore_pending_stack_adjust (&save);
+
+ return NULL_RTX;
}
diff --git a/gcc/optabs.h b/gcc/optabs.h
index 3bbceff92d9..f853b93f37f 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -281,6 +281,7 @@ extern void emit_indirect_jump (rtx);
/* Emit a conditional move operation. */
rtx emit_conditional_move (rtx, enum rtx_code, rtx, rtx, machine_mode,
rtx, rtx, machine_mode, int);
+rtx emit_conditional_move (rtx, rtx, rtx, rtx, rtx, machine_mode);
/* Emit a conditional negate or bitwise complement operation. */
rtx emit_conditional_neg_or_complement (rtx, rtx_code, machine_mode, rtx,
--
2.31.1
