From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 24F793858400 for ; Mon, 18 Oct 2021 11:40:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 24F793858400 Received: from pps.filterd (m0187473.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 19I9CNqA005346; Mon, 18 Oct 2021 07:40:25 -0400 Received: from ppma04ams.nl.ibm.com (63.31.33a9.ip4.static.sl-reverse.com [169.51.49.99]) by mx0a-001b2d01.pphosted.com with ESMTP id 3bryn7tgj2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 18 Oct 2021 07:40:24 -0400 Received: from pps.filterd (ppma04ams.nl.ibm.com [127.0.0.1]) by ppma04ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 19IBaEND008532; Mon, 18 Oct 2021 11:40:22 GMT Received: from b06cxnps4076.portsmouth.uk.ibm.com (d06relay13.portsmouth.uk.ibm.com [9.149.109.198]) by ppma04ams.nl.ibm.com with ESMTP id 3bqpca5nph-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 18 Oct 2021 11:40:22 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 19IBeJOY65995168 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 18 Oct 2021 11:40:19 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9640811C06C; Mon, 18 Oct 2021 11:40:19 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6E01911C052; Mon, 18 Oct 2021 11:40:19 +0000 (GMT) Received: from li-926bd7cc-2dd1-11b2-a85c-f6adc0f5efec.ibm.com (unknown [9.171.76.119]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTPS; Mon, 18 Oct 2021 11:40:19 +0000 (GMT) Subject: Re: [PATCH 1/7] ifcvt: Check if cmovs are needed. To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com References: <20210625160905.23786-1-rdapp@linux.ibm.com> <20210625160905.23786-2-rdapp@linux.ibm.com> <399e9ada-8a0a-e95b-f037-b3f3cb8a6c48@linux.ibm.com> <5ddba534-293b-6d24-0543-4d0c601a5458@linux.ibm.com> From: Robin Dapp Message-ID: <2ca7d43a-0bde-d30f-7689-0b56759dae5d@linux.ibm.com> Date: Mon, 18 Oct 2021 13:40:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------FFF251469F9CD2C9D5B9C4AB" Content-Language: en-US X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: ato-uvkxIMaCMwQZ2AJCk_659mXtCUaf X-Proofpoint-GUID: ato-uvkxIMaCMwQZ2AJCk_659mXtCUaf X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475 definitions=2021-10-18_03,2021-10-14_02,2020-04-07_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 phishscore=0 clxscore=1015 impostorscore=0 suspectscore=0 bulkscore=0 mlxlogscore=999 adultscore=0 malwarescore=0 spamscore=0 mlxscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109230001 definitions=main-2110180071 X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_MSPIKE_H2, 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: Mon, 18 Oct 2021 11:40:31 -0000 This is a multi-part message in MIME format. --------------FFF251469F9CD2C9D5B9C4AB Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi Richard, after giving it a second thought, and seeing that most of the changes to existing code are not strictly necessary anymore, I figured it could be easier not changing the current control flow too much like in the attached patch. The changes remaining are to "outsource" the maybe_expand_insn part and making the emit_conditional_move with full comparison and rev_comparsion externally available. I suppose straightening of the arguably somewhat baroque parts, we can defer to a separate patch. On s390 this works nicely but I haven't yet done a bootstrap on other archs. Regards Robin --------------FFF251469F9CD2C9D5B9C4AB Content-Type: text/x-patch; charset=UTF-8; name="patch.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="patch.diff" commit eb50384ee0cdeeefa61ae89bdbb2875500b7ce60 Author: Robin Dapp Date: Wed Nov 27 13:53:40 2019 +0100 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. diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 6ae883cbdd4..f7765e60548 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -772,7 +772,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 **); @@ -1711,7 +1711,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; @@ -1743,23 +1744,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 019bbb62882..25eecf29ed8 100644 --- a/gcc/optabs.c +++ b/gcc/optabs.c @@ -52,6 +52,9 @@ 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); +rtx emit_conditional_move (rtx, rtx, rtx, rtx, rtx, machine_mode); + /* Debug facility for use in GDB. */ void debug_optab_libfuncs (void); @@ -4875,6 +4878,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) @@ -4925,18 +4929,10 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1, OPTAB_WIDEN, &comparison, &cmpmode); if (comparison) { - 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; - } + rtx res = emit_conditional_move (target, comparison, + op2, op3, mode); + if (res != NULL_RTX) + return res; } delete_insns_since (last); restore_pending_stack_adjust (&save); @@ -4959,6 +4955,73 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1, } } +/* Helper function that, in addition to COMPARISON, also tries + the reversed REV_COMPARISON with swapped OP2 and OP3. As opposed + to when we pass the specific constituents of a comparison, no + additional insns are emitted for it. It might still be necessary + to emit more than one insn for the final conditional move, though. */ + +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) +{ + 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)) + { + if (!target) + target = gen_reg_rtx (mode); + + emit_move_insn (target, op3); + return target; + } + + if (mode == VOIDmode) + mode = GET_MODE (op2); + + icode = direct_optab_handler (movcc_optab, mode); + + if (icode == CODE_FOR_nothing) + return NULL_RTX; + + if (!target) + target = gen_reg_rtx (mode); + + 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; + } + + return NULL_RTX; +} + /* Emit a conditional negate or bitwise complement using the negcc or notcc optabs if available. Return NULL_RTX if such operations 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, --------------FFF251469F9CD2C9D5B9C4AB--