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 EC9DD3858D32; Mon, 19 Jun 2023 01:01:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EC9DD3858D32 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 (m0360072.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 35J0uL18006452; Mon, 19 Jun 2023 01:01:56 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=qFe8+E428DrS9bJfg/T6OpvPN0/yED/onTeuVn7uOGI=; b=JTR/WPu0HHUz2eHRfYhNlyWUqkNb3sTMWSU7s9JfWtGLsVUIHOHPPaLA/RWo5UmOrItp HwWh+E3IMawOKhTmnDdFKQmanhMoDFQsdyaXZNcOQy6vdlFdlZdvPR8yjCgtuodJaI3T uaC/0I7bgSy0QiTE8gyexPgYBWzGq4r+jW5PY39PjEu9g04hVxgv419WpnQtPi94Cby2 4nO27La6CieYOygf4vFxzd2FGbAH8XHuJxQm2zCiSxUZneQDdAhHPj0858UT1wkSVqnT fW4DIjpORrGqXLHsfWYn1LkXf7Q+3ce/YhzdMMgcUA3y03fdR8XV8KvkUEtBKRnV4yYd Wg== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3racwvr32v-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 19 Jun 2023 01:01:56 +0000 Received: from m0360072.ppops.net (m0360072.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 35J0vRKg008681; Mon, 19 Jun 2023 01:01:55 GMT Received: from ppma05wdc.us.ibm.com (1b.90.2fa9.ip4.static.sl-reverse.com [169.47.144.27]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3racwvr32m-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 19 Jun 2023 01:01:55 +0000 Received: from pps.filterd (ppma05wdc.us.ibm.com [127.0.0.1]) by ppma05wdc.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 35J02Gtk000825; Mon, 19 Jun 2023 01:01:55 GMT Received: from smtprelay05.wdc07v.mail.ibm.com ([9.208.129.117]) by ppma05wdc.us.ibm.com (PPS) with ESMTPS id 3r94f5r6x8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 19 Jun 2023 01:01:54 +0000 Received: from smtpav01.wdc07v.mail.ibm.com (smtpav01.wdc07v.mail.ibm.com [10.39.53.228]) by smtprelay05.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 35J11rHL42467776 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 19 Jun 2023 01:01:53 GMT Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D07DE5805B; Mon, 19 Jun 2023 01:01:53 +0000 (GMT) Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 791645804B; Mon, 19 Jun 2023 01:01:53 +0000 (GMT) Received: from ltcden2-lp1.aus.stglabs.ibm.com (unknown [9.3.90.43]) by smtpav01.wdc07v.mail.ibm.com (Postfix) with ESMTPS; Mon, 19 Jun 2023 01:01:53 +0000 (GMT) From: Jiufu Guo To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, linkw@gcc.gnu.org, bergner@linux.ibm.com Subject: Re: [PATCH V3 1/4] rs6000: build constant via li;rotldi References: <20230616083412.1877704-1-guojiufu@linux.ibm.com> <20230616101228.GF19790@gate.crashing.org> Date: Mon, 19 Jun 2023 09:01:51 +0800 In-Reply-To: <20230616101228.GF19790@gate.crashing.org> (Segher Boessenkool's message of "Fri, 16 Jun 2023 05:12:28 -0500") Message-ID: <7nfs6o6z9c.fsf@ltcden2-lp1.aus.stglabs.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: u-y4W5xwJVtKeJ0XAsbY2jq9_YccjKVs X-Proofpoint-GUID: 012k3u262TS7ewKHyHbF5mWSV4KjaDvX X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-06-18_16,2023-06-16_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 suspectscore=0 spamscore=0 mlxscore=0 bulkscore=0 mlxlogscore=999 malwarescore=0 adultscore=0 priorityscore=1501 clxscore=1015 phishscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306190003 X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,KAM_SHORT,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,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 Fri, Jun 16, 2023 at 04:34:12PM +0800, Jiufu Guo wrote: >> +/* Check if value C can be built by 2 instructions: one is 'li', another is >> + rotldi. >> + >> + If so, *SHIFT is set to the shift operand of rotldi(rldicl), and *MASK >> + is set to -1, and return true. Return false otherwise. */ > > Don't say "is set to -1", the point of having this is so you say "is set > to the "li" value". Just like you describe what SHIFT is for. Yes, thanks! > >> +static bool >> +can_be_built_by_li_and_rotldi (HOST_WIDE_INT c, int *shift, >> + HOST_WIDE_INT *mask) >> +{ >> + int n; > > Put shis later, like: Thanks! > >> + /* Check if C can be rotated to a positive or negative value >> + which 'li' instruction is able to load. */ > int n; >> + if (can_be_rotated_to_lowbits (c, 15, &n) >> + || can_be_rotated_to_lowbits (~c, 15, &n)) >> + { >> + *mask = HOST_WIDE_INT_M1; >> + *shift = HOST_BITS_PER_WIDE_INT - n; >> + return true; >> + } > > It is tricky to see ~c will always work, since what is really done is -c > instead. Can you just use that here? Some explanation: A negative value of 'li' is: 0b11..11xxx there are 49 leading '1's, and the other 15 tailing bits can be 0 or 1. With the '~' operation, there are 49 '0's. After the value is rotated, there are still 49 '1's. (xxx may also be at head/tail.) For the rotated value, with the '~' operation, there are still 49 '0's. So, for a value, if there are 49 successive '1's (may cross head/tail). It should be able to rotate to low 15 bits after the '~' operation. It would not be enough if using the '-' operation, since '-x=~x+1' in the bit aspect. As the below case 'li_rotldi_3': 0xffff8531ffffffffLL (rotate left 0x8531 32bit). The '~c' is 0x7ace00000000, this can be rotated from 0x7ace. (~0x8531). But '-c' is 0x7ace00000001. this value is not good. > >> @@ -10266,15 +10291,14 @@ static void >> rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) >> { >> rtx temp; >> + int shift; >> + HOST_WIDE_INT mask; >> HOST_WIDE_INT ud1, ud2, ud3, ud4; >> >> ud1 = c & 0xffff; >> - c = c >> 16; >> - ud2 = c & 0xffff; >> - c = c >> 16; >> - ud3 = c & 0xffff; >> - c = c >> 16; >> - ud4 = c & 0xffff; >> + ud2 = (c >> 16) & 0xffff; >> + ud3 = (c >> 32) & 0xffff; >> + ud4 = (c >> 48) & 0xffff; >> >> if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) >> || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000))) >> @@ -10305,6 +10329,17 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) >> emit_move_insn (dest, gen_rtx_XOR (DImode, temp, >> GEN_INT ((ud2 ^ 0xffff) << 16))); >> } >> + else if (can_be_built_by_li_and_rotldi (c, &shift, &mask)) >> + { >> + temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode); >> + unsigned HOST_WIDE_INT imm = (c | ~mask); >> + imm = (imm >> shift) | (imm << (HOST_BITS_PER_WIDE_INT - shift)); >> + >> + emit_move_insn (temp, GEN_INT (imm)); >> + if (shift != 0) >> + temp = gen_rtx_ROTATE (DImode, temp, GEN_INT (shift)); >> + emit_move_insn (dest, temp); >> + } > > If you would rewrite so it isn't such a run-on thing with "else if", > instead using early outs, or even some factoring, you could declare the > variable used only in a tiny scope in that tiny scope instead. Yes! Early returning is better for a lot of cases. I would like to have a refactor patch. > >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/powerpc/const-build.c >> @@ -0,0 +1,54 @@ >> +/* { dg-do run } */ >> +/* { dg-options "-O2 -save-temps" } */ >> +/* { dg-require-effective-target has_arch_ppc64 } */ > > Please put a tiny comment here saying what this test is *for*? The file > name is a bit of hint already, but you can indicate much more in one or > two lines :-) Oh, yes, thanks for point out this! > > With those adjustments, okay for trunk. Thanks! > > (If -c doesn't work, it needs more explanation). Sure, some words as above. BR, Jeff (Jiufu Guo) > > > Segher