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 829793858402 for ; Fri, 12 Nov 2021 13:00:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 829793858402 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 1ACCHYKH030162; Fri, 12 Nov 2021 13:00:45 GMT Received: from ppma03fra.de.ibm.com (6b.4a.5195.ip4.static.sl-reverse.com [149.81.74.107]) by mx0a-001b2d01.pphosted.com with ESMTP id 3c9r558uxq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 12 Nov 2021 13:00:45 +0000 Received: from pps.filterd (ppma03fra.de.ibm.com [127.0.0.1]) by ppma03fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 1ACCxQNr032136; Fri, 12 Nov 2021 13:00:43 GMT Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by ppma03fra.de.ibm.com with ESMTP id 3c5hbb5f8t-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 12 Nov 2021 13:00:43 +0000 Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1ACD0e4P63308050 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 12 Nov 2021 13:00:40 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B519F52069; Fri, 12 Nov 2021 13:00:40 +0000 (GMT) Received: from [9.171.58.83] (unknown [9.171.58.83]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTPS id 7B77D5206C; Fri, 12 Nov 2021 13:00:40 +0000 (GMT) Message-ID: <28d0884c-fdf8-e553-328e-b06d1fe1c085@linux.ibm.com> Date: Fri, 12 Nov 2021 14:00:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH 1/7] ifcvt: Check if cmovs are needed. Content-Language: en-US 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> <2ca7d43a-0bde-d30f-7689-0b56759dae5d@linux.ibm.com> From: Robin Dapp In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: Mnzy2qKJaPnfyHW-T1E2nfQOoFHpUTzS X-Proofpoint-ORIG-GUID: Mnzy2qKJaPnfyHW-T1E2nfQOoFHpUTzS X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475 definitions=2021-11-12_04,2021-11-12_01,2020-04-07_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 phishscore=0 mlxlogscore=999 priorityscore=1501 suspectscore=0 impostorscore=0 bulkscore=0 spamscore=0 mlxscore=0 malwarescore=0 lowpriorityscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2111120074 X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, 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: Fri, 12 Nov 2021 13:00:52 -0000 Hi Richard, > It's hard to judge this in isolation because it's not clear when > and how the new arguments are going to be used, but it seems OK > in principle. Do you still want: > > /* If earliest == jump, try to build the cmove insn directly. > This is helpful when combine has created some complex condition > (like for alpha's cmovlbs) that we can't hope to regenerate > through the normal interface. */ > > if (if_info->cond_earliest == if_info->jump) > { > > to be used when cc_cmp and rev_cc_cmp are nonnull? My initial hunch was to just leave it in place as I did not manage to trigger it. As it is going to be called and costed both ways (with cc_cmp, rev_cc_cmp and without) it is probably better to move it into the else branch. The single usage of this is in patch 5/7. We are passing the already existing condition from the jump and its reverse to see if the backend can come up with something better than when creating a new comparison. >> +static rtx emit_conditional_move (rtx, rtx, rtx, rtx, machine_mode); >> +rtx emit_conditional_move (rtx, rtx, rtx, rtx, rtx, machine_mode); > > This is redundant with the header file declaration. > Removed it. > I think it'd be better to call one of these functions something else, > rather than make the interpretation of the third parameter depend on > the total number of parameters. In the second overload, the comparison > rtx effectively replaces four parameters of the existing > emit_conditional_move, so perhaps that's the one that should remain > emit_conditional_move. Maybe the first one should be called > emit_conditional_move_with_rev or something. Not entirely fond of calling the first one _with_rev because essentially both try normal and reversed variants but I agree that the naming is not ideal. I don't have any great ideas how to properly untangle it so I would go with your suggestions in order to move forward. As there is only one caller of the second function, we could also let the caller handle the reversing. Then, the third function would need to be non-static, though. The third, static emit_conditional_move I already renamed locally to emit_conditional_move_1. > Part of me wonders if this would be simpler if we created a structure > to describe a comparison and passed that around instead of individual > fields, but I guess it could become a rat hole. I also thought about this as it would allow us to use either representation as required by the usage site. Even tried it in a branch locally but indeed it became ugly quickly so I postponed it for now. Regards Robin