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 1D2F33858C5E for ; Thu, 12 Oct 2023 11:16:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1D2F33858C5E 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 (m0353724.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 39CBDgk1027481; Thu, 12 Oct 2023 11:16:10 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=STy1gfUn6SRgFqmKtvorLgjkZrAb1rh3sw5AokjOptQ=; b=DtynzI99TDU5Fj/npjyxKqIwoAbtmJHgDhV8viP4PdkWS0BrI9L5wPQNbTeO9LLj7Db7 PNf676OTDjIByFaL6oGbc3H87gcKo7dqcrSvuh5ZGJvA0f9ulerZglwz6dpXsBE0x17S R8FX7a193jUJUuD+p8iy+W+bxqUzd5oTNM19bQn7cVRRmH1kmp3S0eNxSQuiHzrDXHPg CZLw0NmUJJgbZaB/ICo8ViQWlDeRIU5UBmPD82wGzMj6vdFTfvl5SWQRiO5TlXSyUsB6 pvhVkfAz7uG88VMmvlkBzFZ56ci4OyeZBYTjZvk0a2QNqBZoEo4teGWmJu8ZQHHkRJu8 /w== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3tpfkwg7rp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 12 Oct 2023 11:16:10 +0000 Received: from m0353724.ppops.net (m0353724.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 39CBDmqs027981; Thu, 12 Oct 2023 11:16:09 GMT Received: from ppma23.wdc07v.mail.ibm.com (5d.69.3da9.ip4.static.sl-reverse.com [169.61.105.93]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3tpfkwg7r4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 12 Oct 2023 11:16:09 +0000 Received: from pps.filterd (ppma23.wdc07v.mail.ibm.com [127.0.0.1]) by ppma23.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 39CAeOcH000643; Thu, 12 Oct 2023 11:16:09 GMT Received: from smtprelay05.wdc07v.mail.ibm.com ([172.16.1.72]) by ppma23.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3tkk5kxv3h-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 12 Oct 2023 11:16:09 +0000 Received: from smtpav03.dal12v.mail.ibm.com (smtpav03.dal12v.mail.ibm.com [10.241.53.102]) by smtprelay05.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 39CBG7R121365302 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 12 Oct 2023 11:16:08 GMT Received: from smtpav03.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D551358056; Thu, 12 Oct 2023 11:16:07 +0000 (GMT) Received: from smtpav03.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A1D405803F; Thu, 12 Oct 2023 11:16:07 +0000 (GMT) Received: from genoa (unknown [9.40.192.157]) by smtpav03.dal12v.mail.ibm.com (Postfix) with ESMTPS; Thu, 12 Oct 2023 11:16:07 +0000 (GMT) From: Jiufu Guo To: Michael Meissner Cc: gcc-patches@gcc.gnu.org, Segher Boessenkool , "Kewen.Lin" , David Edelsohn , Peter Bergner Subject: Re: [PATCH] PR target/111778 - Fix undefined shifts in PowerPC compiler References: Date: Thu, 12 Oct 2023 19:16:04 +0800 In-Reply-To: (Michael Meissner's message of "Thu, 12 Oct 2023 04:24:00 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: sqp0i4CWXSi91U2oiY63sVChbAazUb7N X-Proofpoint-GUID: 1QcyoINvTcIjFJeebvFDNRMYsv-VEKdX X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-10-12_05,2023-10-12_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 impostorscore=0 phishscore=0 clxscore=1015 mlxscore=0 suspectscore=0 priorityscore=1501 spamscore=0 mlxlogscore=999 bulkscore=0 adultscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000 definitions=main-2310120091 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP 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, Thanks for your quick fix! Michael Meissner writes: > I was building a cross compiler to PowerPC on my x86_86 workstation with the > latest version of GCC on October 11th. I could not build the compiler on the > x86_64 system as it died in building libgcc. I looked into it, and I > discovered the compiler was recursing until it ran out of stack space. If I > build a native compiler with the same sources on a PowerPC system, it builds > fine. > > I traced this down to a change made around October 10th: > > | commit 8f1a70a4fbcc6441c70da60d4ef6db1e5635e18a (HEAD) > | Author: Jiufu Guo > | Date: Tue Jan 10 20:52:33 2023 +0800 > | > | rs6000: build constant via li/lis;rldicl/rldicr > | > | If a constant is possible left/right cleaned on a rotated value from > | a negative value of "li/lis". Then, using "li/lis ; rldicl/rldicr" > | to build the constant. > > The code was doing a -1 << 64 which is undefined behavior because different > machines produce different results. On the x86_64 system, (-1 << 64) produces > -1 while on a PowerPC 64-bit system, (-1 << 64) produces 0. The x86_64 then > recurses until the stack runs out of space. > > If I apply this patch, the compiler builds fine on both x86_64 as a PowerPC > crosss compiler and on a native PowerPC system. > > Can I check this into the master branch to fix the problem? > > 2023-10-12 Michael Meissner > > gcc/ > > PR target/111778 > * config/rs6000/rs6000.cc (can_be_built_by_li_lis_and_rldicl): Protect > code from shifts that are undefined. > (can_be_built_by_li_lis_and_rldicr): Likewise. > (can_be_built_by_li_and_rldic): Protect code from shifts that > undefined. Also replace uses of 1ULL with HOST_WIDE_INT_1U. > > --- > gcc/config/rs6000/rs6000.cc | 29 ++++++++++++++++++++++++++--- > 1 file changed, 26 insertions(+), 3 deletions(-) > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index 2828f01413c..cc24dd5301e 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -10370,6 +10370,11 @@ can_be_built_by_li_lis_and_rldicl (HOST_WIDE_INT c, int *shift, > /* Leading zeros may be cleaned by rldicl with a mask. Change leading zeros > to ones and then recheck it. */ > int lz = clz_hwi (c); > + > + /* If lz == 0, the left shift is undefined. */ > + if (!lz) > + return false; > + Thanks! This should be checked. If "lz" is zero, it means for input "C", there is no leading zeros which are cleanded by "rldicl". And then no future analyzing is needed. > HOST_WIDE_INT unmask_c > = c | (HOST_WIDE_INT_M1U << (HOST_BITS_PER_WIDE_INT - lz)); > int n; > @@ -10398,6 +10403,11 @@ can_be_built_by_li_lis_and_rldicr (HOST_WIDE_INT c, int *shift, > /* Tailing zeros may be cleaned by rldicr with a mask. Change tailing zeros > to ones and then recheck it. */ > int tz = ctz_hwi (c); > + > + /* If tz == HOST_BITS_PER_WIDE_INT, the left shift is undefined. */ > + if (tz >= HOST_BITS_PER_WIDE_INT) > + return false; > + This is correct in theory and could make sure "tz" is ok. Just one minor thing: "ctz_hwi" would not return value greater than HOST_BITS_PER_WIDE_INT other than 0, right? > HOST_WIDE_INT unmask_c = c | ((HOST_WIDE_INT_1U << tz) - 1); > int n; > if (can_be_rotated_to_lowbits (~unmask_c, 15, &n) > @@ -10428,8 +10438,15 @@ can_be_built_by_li_and_rldic (HOST_WIDE_INT c, int *shift, HOST_WIDE_INT *mask) > right bits are shifted as 0's, and left 1's(and x's) are cleaned. */ > int tz = ctz_hwi (c); > int lz = clz_hwi (c); > + > + /* If lz == HOST_BITS_PER_WIDE_INT, the left shift is undefined. */ > + if (lz >= HOST_BITS_PER_WIDE_INT) > + return false; > + This maybe similar. > int middle_ones = clz_hwi (~(c << lz)); > - if (tz + lz + middle_ones >= ones) > + if (tz + lz + middle_ones >= ones > + && (tz - lz) < HOST_BITS_PER_WIDE_INT > + && tz < HOST_BITS_PER_WIDE_INT) > { > *mask = ((1LL << (HOST_BITS_PER_WIDE_INT - tz - lz)) - 1LL) << tz; > *shift = tz; > @@ -10440,7 +10457,8 @@ can_be_built_by_li_and_rldic (HOST_WIDE_INT c, int *shift, HOST_WIDE_INT *mask) > int leading_ones = clz_hwi (~c); > int tailing_ones = ctz_hwi (~c); > int middle_zeros = ctz_hwi (c >> tailing_ones); > - if (leading_ones + tailing_ones + middle_zeros >= ones) > + if (leading_ones + tailing_ones + middle_zeros >= ones > + && middle_zeros < HOST_BITS_PER_WIDE_INT) Thanks. > { > *mask = ~(((1ULL << middle_zeros) - 1ULL) << tailing_ones); > *shift = tailing_ones + middle_zeros; > @@ -10450,10 +10468,15 @@ can_be_built_by_li_and_rldic (HOST_WIDE_INT c, int *shift, HOST_WIDE_INT *mask) > /* xx1..1xx: --> xx0..01..1xx: some 1's(following x's) are cleaned. */ > /* Get the position for the first bit of successive 1. > The 24th bit would be in successive 0 or 1. */ > - HOST_WIDE_INT low_mask = (1LL << 24) - 1LL; > + HOST_WIDE_INT low_mask = (HOST_WIDE_INT_1U << 24) - HOST_WIDE_INT_1U; Yes. > int pos_first_1 = ((c & (low_mask + 1)) == 0) > ? clz_hwi (c & low_mask) > : HOST_BITS_PER_WIDE_INT - ctz_hwi (~(c | low_mask)); > + > + /* Make sure the left and right shifts are defined. */ > + if (!IN_RANGE (pos_first_1, 1, HOST_BITS_PER_WIDE_INT-1)) > + return false; > + Yes, this change would be safer. Thanks again for the enhancement! BR, Jeff (Jiufu Guo) > middle_ones = clz_hwi (~c << pos_first_1); > middle_zeros = ctz_hwi (c >> (HOST_BITS_PER_WIDE_INT - pos_first_1)); > if (pos_first_1 < HOST_BITS_PER_WIDE_INT > -- > 2.41.0