From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id E03C138582BE; Wed, 24 Aug 2022 07:48:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E03C138582BE Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 27O7hndf010825; Wed, 24 Aug 2022 07:48:54 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : references : date : in-reply-to : message-id : mime-version : content-type; s=pp1; bh=RLsaxs9Aq3L6ZSpsjFDvbbbQjuUbP2tc05rSfEatuik=; b=m2Qknzjvqmw0XUCIG2gpEUZjhj8uBtcaUeQzsZ68bF7EAtbONmJCPkYI0fZY/tDILe8S 25H/FXFoDCx4pWWIQkmMM3DJ+B2iBzfz8sHzkTBIlLNkzjCvtFQS6c7Y+kaf8A1Masry QECR/QuF00oNXqrzVA1Vu0FTzY2IOFBXD3Gzqc4u8FrsWk8cvIgc/WGMQRJbgv8ko434 9w1uMv+wGzqsodz6OAJbW7d1sc24hi13XA7Kbpg0RQmt7PH+QnDqw3S+6zlnlakjv94f 8mMytV74YzeDYGETMrlpCU6hSlA23if8kFAbNh/cvL+/ayzeQeUDCd5QEq7R8rLpHpD0 Ag== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3j5fun84aq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 24 Aug 2022 07:48:54 +0000 Received: from m0127361.ppops.net (m0127361.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 27O7hpjj010881; Wed, 24 Aug 2022 07:48:53 GMT Received: from ppma01wdc.us.ibm.com (fd.55.37a9.ip4.static.sl-reverse.com [169.55.85.253]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3j5fun84aa-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 24 Aug 2022 07:48:53 +0000 Received: from pps.filterd (ppma01wdc.us.ibm.com [127.0.0.1]) by ppma01wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 27O7KqDB012192; Wed, 24 Aug 2022 07:48:53 GMT Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by ppma01wdc.us.ibm.com with ESMTP id 3j2q88yx6j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 24 Aug 2022 07:48:53 +0000 Received: from b01ledav001.gho.pok.ibm.com (b01ledav001.gho.pok.ibm.com [9.57.199.106]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 27O7mqW17471662 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 24 Aug 2022 07:48:52 GMT Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C179528059; Wed, 24 Aug 2022 07:48:52 +0000 (GMT) Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 82C7728058; Wed, 24 Aug 2022 07:48:52 +0000 (GMT) Received: from pike (unknown [9.5.12.127]) by b01ledav001.gho.pok.ibm.com (Postfix) with ESMTPS; Wed, 24 Aug 2022 07:48:52 +0000 (GMT) From: Jiufu Guo To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, linkw@gcc.gnu.org Subject: Re: [PATCH V4] rs6000: Optimize cmp on rotated 16bits constant References: <20220725132922.45470-1-guojiufu@linux.ibm.com> <20220823221854.GX25951@gate.crashing.org> Date: Wed, 24 Aug 2022 15:48:49 +0800 In-Reply-To: <20220823221854.GX25951@gate.crashing.org> (Segher Boessenkool's message of "Tue, 23 Aug 2022 17:18:55 -0500") Message-ID: <7er116gjz2.fsf@pike.rch.stglabs.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: EiHzNFOhMZriHde2e-aXkL563jb3Xo9n X-Proofpoint-GUID: zbA6mdHVAfVGtTKGltipIeEPsaTRJzdk X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-24_04,2022-08-22_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 malwarescore=0 adultscore=0 phishscore=0 impostorscore=0 spamscore=0 lowpriorityscore=0 bulkscore=0 suspectscore=0 mlxlogscore=999 priorityscore=1501 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2208240029 X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi! Segher Boessenkool writes: > Hi! > > On Mon, Jul 25, 2022 at 09:29:22PM +0800, Jiufu Guo wrote: >> When checking eq/neq with a constant which has only 16bits, it can be >> optimized to check the rotated data. By this, the constant building >> is optimized. > > "ne", not "neq". Oh, thanks! > >> gcc/ChangeLog: >> >> * config/rs6000/rs6000-protos.h (rotate_from_leading_zeros_const): >> New decl. > > "New declaration." or just "New". Also, don't break lines early please, > especially if that means ending a line in a colon, which then looks as > if you forgot to write something there. OK, will update. > >> * config/rs6000/rs6000.cc (rotate_from_leading_zeros_const): New >> define for checking simply rotated constant. > > "New." or "New definition." or such. OK, will update. > >> +/* Check if C can be rotated from an immediate which contains leading >> + zeros at least CLZ. > > "Which starts (as 64 bit integer) with at least CLZ bits zero" or such. Thanks a lot! Will update. > >> + /* xx0..0xx: rotate enough bits firstly, then check case a. */ >> + const int rot_bits = HOST_BITS_PER_WIDE_INT - clz + 1; >> + unsigned HOST_WIDE_INT rc = (c >> rot_bits) | (c << (clz - 1)); >> + tz = ctz_hwi (rc); >> + if (clz_hwi (rc) + tz >= clz) >> + return tz + rot_bits; > > This could use some more explanation. OK, thanks, will update. > >> +(define_code_iterator eqne [eq ne]) > > >> +;; "i == C" ==> "rotl(i,N) == rotl(C,N)" >> +(define_insn_and_split "*rotate_on_cmpdi" >> + [(set (pc) >> + (if_then_else (eqne (match_operand:DI 1 "gpc_reg_operand" "r") > > Wrong indentation. The ( should be in the same column as the preceding ( > it matches. Oh, thanks for point this out! Will update. > > Setting "pc" to either 0 or 1 is never correct. > >> + "TARGET_POWERPC64 && !reload_completed && can_create_pseudo_p () > > reload_completed in splitters is almost always wrong. It isn't any > better if it is in the insn condition of a define_insn_and_split :-) > Thanks, 'can_create_pseudo_p' would be ok for this patch. Or just FAIL, if !can_create_pseudo_p()? >> + && num_insns_constant (operands[2], DImode) > 1 >> + && (rotate_from_leading_zeros_const (~UINTVAL (operands[2]), 49) > 0 >> + || rotate_from_leading_zeros_const (UINTVAL (operands[2]), 48) > 0)" > > There must be a better way to describe this. Will update this. I'm thinking to replace this with a meaning function, maybe 'compare_rotate_immediate_p'. > >> + if (rot < 0) >> + { >> + sgn = true; >> + rot = rotate_from_leading_zeros_const (~C, 49); >> + } > > Bad indentation. Will update. > >> + rtx cmp = ne ? gen_rtx_NE (CCmode, cc, const0_rtx) >> + : gen_rtx_EQ (CCmode, cc, const0_rtx); > > rtx cmp = gen_rtx_ (...); > > (and define a code_attr EQNE to just output the uppercase EQ or NE). Great! Thanks for always helpful suggestions! > > Why is this doing a conditional branch at all? Unpredictable > conditional branches are extremely costly. This optimization needs to check whether the comparison code is ne/eq or not. To get the comparison code, we need to check the parent insn of the 'cmp' insn. This is why conditional branch patterns in used here. This patch should not change the information (about prediction) of the branch insn. I'm thinking of updating the patch to keep the 'note info REG_BR_PROB' for the branch instruction. I will submit the updated patch for review. > >> +/* { dg-require-effective-target lp64 } */ > > arch_ppc64 OK, will update. > >> +/* { dg-final { scan-assembler-times "cmpldi" 10 } } */ >> +/* { dg-final { scan-assembler-times "cmpdi" 4 } } */ >> +/* { dg-final { scan-assembler-times "rotldi" 14 } } */ > > Please use \m and \M . OK, will update. Thanks again! BR, Jeff (Jiufu) > > Thanks, > > > Segher