From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 6682E3853804 for ; Thu, 15 Jul 2021 20:54:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6682E3853804 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F0E6331B; Thu, 15 Jul 2021 13:54:13 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7B5433F694; Thu, 15 Jul 2021 13:54:13 -0700 (PDT) From: Richard Sandiford To: Robin Dapp Mail-Followup-To: Robin Dapp , gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 4/7] ifcvt/optabs: Allow using a CC comparison for emit_conditional_move. References: <20210625160905.23786-1-rdapp@linux.ibm.com> <20210625160905.23786-5-rdapp@linux.ibm.com> Date: Thu, 15 Jul 2021 21:54:12 +0100 In-Reply-To: <20210625160905.23786-5-rdapp@linux.ibm.com> (Robin Dapp's message of "Fri, 25 Jun 2021 18:09:02 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Jul 2021 20:54:16 -0000 Robin Dapp writes: > 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 | 16 +++-- > gcc/optabs.c | 163 ++++++++++++++++++++++++++++++++++----------------- > gcc/optabs.h | 1 + > 3 files changed, 121 insertions(+), 59 deletions(-) > > diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c > index ac0c142c9fe..c5b8641e2aa 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, r= tx, > - rtx, rtx, rtx); > + rtx, rtx, rtx, rtx =3D NULL, rtx =3D 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_i= nfo) >=20=20 > 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; > @@ -1756,9 +1757,14 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx= x, enum rtx_code code, > unsignedp =3D (code =3D=3D LTU || code =3D=3D GEU > || code =3D=3D LEU || code =3D=3D GTU); >=20=20 > - target =3D emit_conditional_move (x, code, cmp_a, cmp_b, VOIDmode, > - vtrue, vfalse, GET_MODE (x), > - unsignedp); > + if (cc_cmp !=3D NULL_RTX && rev_cc_cmp !=3D NULL_RTX) > + target =3D emit_conditional_move (x, cc_cmp, rev_cc_cmp, > + vtrue, vfalse, GET_MODE (x)); > + else > + target =3D emit_conditional_move (x, code, cmp_a, cmp_b, VOIDmode, > + vtrue, vfalse, GET_MODE (x), > + unsignedp); 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)) !=3D MODE_CC || cmp_b !=3D const0_rtx) return NULL_RTX; } into the =E2=80=9Celse=E2=80=9D 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. > + > if (target) > return target; >=20=20 > diff --git a/gcc/optabs.c b/gcc/optabs.c > index 62a6bdb4c59..6bf486b9b50 100644 > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -52,6 +52,8 @@ static void prepare_float_lib_cmp (rtx, rtx, enum rtx_c= ode, rtx *, > static rtx expand_unop_direct (machine_mode, optab, rtx, rtx, int); > static void emit_libcall_block_1 (rtx_insn *, rtx, rtx, rtx, bool); >=20=20 > +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 co= de, rtx op0, rtx op1, > machine_mode mode, int unsignedp) > { > rtx comparison; > - rtx_insn *last; > enum insn_code icode; > enum rtx_code reversed; >=20=20 > @@ -4774,6 +4775,7 @@ emit_conditional_move (rtx target, enum rtx_code co= de, 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 sin= ce > comparisons against zero are cheaper. */ > + > if (code =3D=3D LT && op1 =3D=3D const1_rtx) > code =3D LE, op1 =3D const0_rtx; > else if (code =3D=3D GT && op1 =3D=3D constm1_rtx) > @@ -4782,17 +4784,29 @@ emit_conditional_move (rtx target, enum rtx_code = code, rtx op0, rtx op1, > if (cmode =3D=3D VOIDmode) > cmode =3D GET_MODE (op0); >=20=20 > - enum rtx_code orig_code =3D code; > + /* If the first source operand is constant and the second is not, swap > + it into the second. In that case we also need to reverse the > + comparison. It is possible, though, that the conditional move > + will not expand with operands in this order, so we might also need > + to revert to the original comparison and operand order. */ Why's that the case though? The swapped form is the canonical one, so it's the one that the target ought to accept. Thanks, Richard > + > + rtx rev_comparison =3D NULL_RTX; > bool swapped =3D false; > - if (swap_commutative_operands_p (op2, op3) > - && ((reversed =3D reversed_comparison_code_parts (code, op0, op1, = NULL)) > - !=3D UNKNOWN)) > + > + code =3D unsignedp ? unsigned_condition (code) : code; > + comparison =3D simplify_gen_relational (code, VOIDmode, cmode, op0, op= 1); > + > + if ((reversed =3D reversed_comparison_code_parts (code, op0, op1, NULL= )) > + !=3D UNKNOWN) > { > - std::swap (op2, op3); > - code =3D reversed; > - swapped =3D true; > + reversed =3D unsignedp ? unsigned_condition (reversed) : reversed; > + rev_comparison =3D simplify_gen_relational (reversed, VOIDmode, cm= ode, > + op0, op1); > } >=20=20 > + if (swap_commutative_operands_p (op2, op3) && reversed !=3D UNKNOWN) > + swapped =3D true; > + > if (mode =3D=3D VOIDmode) > mode =3D GET_MODE (op2); >=20=20 > @@ -4804,58 +4818,99 @@ emit_conditional_move (rtx target, enum rtx_code = code, rtx op0, rtx op1, > if (!target) > target =3D gen_reg_rtx (mode); >=20=20 > - for (int pass =3D 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 =3D emit_conditional_move (target, comparison, op2, op3, mode); > + > + if (res !=3D 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 =3D=3D 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 =3D unsignedp ? unsigned_condition (code) : code; > - comparison =3D simplify_gen_relational (code, VOIDmode, cmode, op0= , op1); > + if (!target) > + target =3D gen_reg_rtx (mode); >=20=20 > - /* 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 =3D get_last_insn (); > - do_pending_stack_adjust (); > - machine_mode cmpmode =3D 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; > + } >=20=20 > - 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 !=3D target) > - convert_move (target, ops[0].value, false); > - return target; > - } > - } > - delete_insns_since (last); > - restore_pending_stack_adjust (&save); > - } > + if (mode =3D=3D VOIDmode) > + mode =3D GET_MODE (op2); >=20=20 > - if (pass =3D=3D 1) > - return NULL_RTX; > + icode =3D direct_optab_handler (movcc_optab, mode); >=20=20 > - /* If the preferred op2/op3 order is not usable, retry with other > - operand order, perhaps it will expand successfully. */ > - if (swapped) > - code =3D orig_code; > - else if ((reversed =3D reversed_comparison_code_parts (orig_code, = op0, op1, > - NULL)) > - !=3D UNKNOWN) > - code =3D reversed; > - else > - return NULL_RTX; > - std::swap (op2, op3); > + if (icode =3D=3D CODE_FOR_nothing) > + return NULL_RTX; > + > + if (!target) > + target =3D gen_reg_rtx (mode); > + > + saved_pending_stack_adjust save; > + save_pending_stack_adjust (&save); > + last =3D 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 !=3D target) > + convert_move (target, ops[0].value, false); > + return target; > } > + > + delete_insns_since (last); > + restore_pending_stack_adjust (&save); > + > + return NULL_RTX; > } >=20=20 >=20=20 > 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); >=20=20 > /* Emit a conditional negate or bitwise complement operation. */ > rtx emit_conditional_neg_or_complement (rtx, rtx_code, machine_mode, rtx,