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 D1A493858C83 for ; Tue, 16 May 2023 20:07:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D1A493858C83 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 (m0353722.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 34GJrFdp018320 for ; Tue, 16 May 2023 20:07:44 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=1kJQj4nxj5dgytUQ6hyNcyh7N+Vvl0MnbWOpP3pc3sA=; b=BbzAJjy4ADbF5cKZMnlQVPUwBxYbwQfzaEZK3it01oZ7TGLGzevaN+POY49ZNDN7Tgim g16EUyW7Plg9V2kCfj4AF0BVCX0n5Dl/TJrTboVwI/3MEdQDJvAfDpzeDx+fg2T7xzNf pCjLxfsOomTejzXgh9TcjxRoxDJUFQpTk8G17MG+80n1DkzB8fMQ0xjD0ugGvIlbI+Vs QCIiXLWAEd5UBIG4wHZVX9ccbngxZBzYrbjID2TXlqjXu8jNHSFl0u/W/sgybL30jlBj pT092lZgLMTp9P+Ymj6UWqWCwu7ryXCouu9fSUsH5nH6h6djZndOwNk0uqHO2oOJd84n KA== Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3qmgcj897h-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 16 May 2023 20:07:39 +0000 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 34G7Zhtv028201 for ; Tue, 16 May 2023 20:05:13 GMT Received: from smtprelay04.fra02v.mail.ibm.com ([9.218.2.228]) by ppma03ams.nl.ibm.com (PPS) with ESMTPS id 3qj264stne-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 16 May 2023 20:05:13 +0000 Received: from smtpav04.fra02v.mail.ibm.com (smtpav04.fra02v.mail.ibm.com [10.20.54.103]) by smtprelay04.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 34GK59DE35062296 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 16 May 2023 20:05:10 GMT Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DEAFC20043; Tue, 16 May 2023 20:05:09 +0000 (GMT) Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id AC3B420040; Tue, 16 May 2023 20:05:09 +0000 (GMT) Received: from [9.171.17.43] (unknown [9.171.17.43]) by smtpav04.fra02v.mail.ibm.com (Postfix) with ESMTP; Tue, 16 May 2023 20:05:09 +0000 (GMT) Message-ID: <0f57fed5-bcc9-05fd-4453-f40d5342da8c@linux.ibm.com> Date: Tue, 16 May 2023 22:05:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH] s390: Implement TARGET_ATOMIC_ALIGN_FOR_MODE Content-Language: en-US To: Stefan Schulze Frielinghaus , gcc-patches@gcc.gnu.org References: <20230516064322.2584953-1-stefansf@linux.ibm.com> From: Andreas Krebbel In-Reply-To: <20230516064322.2584953-1-stefansf@linux.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: -NOBVdIbU1l2YoLY6FpDqA5DgufRn8pn X-Proofpoint-ORIG-GUID: -NOBVdIbU1l2YoLY6FpDqA5DgufRn8pn X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-05-16_12,2023-05-16_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 mlxlogscore=999 clxscore=1015 mlxscore=0 suspectscore=0 bulkscore=0 phishscore=0 priorityscore=1501 impostorscore=0 malwarescore=0 spamscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000 definitions=main-2305160169 X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,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: On 5/16/23 08:43, Stefan Schulze Frielinghaus wrote: > So far atomic objects are aligned according to their default alignment. > For 128 bit scalar types like int128 or long double this results in an > 8 byte alignment which is wrong and must be 16 byte. > > libstdc++ already computes a correct alignment, though, still adding a > test case in order to make sure that both implementations are > compatible. > > Bootstrapped and regtested. Ok for mainline? Since this is an ABI > break, is a backport to GCC 13 reasonable? Ok for mainline. I would also like to have it in GCC 13. It is an ABI breakage but on the other hand it also fixes an ABI inconsistency between C and C++ which we should fix asap I think. Andreas > > gcc/ChangeLog: > > * config/s390/s390.cc (TARGET_ATOMIC_ALIGN_FOR_MODE): > New. > (s390_atomic_align_for_mode): New. > > gcc/testsuite/ChangeLog: > > * g++.target/s390/atomic-align-1.C: New test. > * gcc.target/s390/atomic-align-1.c: New test. > * gcc.target/s390/atomic-align-2.c: New test. > --- > gcc/config/s390/s390.cc | 8 ++++++ > .../g++.target/s390/atomic-align-1.C | 25 +++++++++++++++++++ > .../gcc.target/s390/atomic-align-1.c | 23 +++++++++++++++++ > .../gcc.target/s390/atomic-align-2.c | 18 +++++++++++++ > 4 files changed, 74 insertions(+) > create mode 100644 gcc/testsuite/g++.target/s390/atomic-align-1.C > create mode 100644 gcc/testsuite/gcc.target/s390/atomic-align-1.c > create mode 100644 gcc/testsuite/gcc.target/s390/atomic-align-2.c > > diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc > index 505de995da8..4813bf91dc4 100644 > --- a/gcc/config/s390/s390.cc > +++ b/gcc/config/s390/s390.cc > @@ -450,6 +450,14 @@ s390_preserve_fpr_arg_p (int regno) > && regno >= FPR0_REGNUM); > } > > +#undef TARGET_ATOMIC_ALIGN_FOR_MODE > +#define TARGET_ATOMIC_ALIGN_FOR_MODE s390_atomic_align_for_mode > +static unsigned int > +s390_atomic_align_for_mode (machine_mode mode) > +{ > + return GET_MODE_BITSIZE (mode); > +} > + > /* A couple of shortcuts. */ > #define CONST_OK_FOR_J(x) \ > CONST_OK_FOR_CONSTRAINT_P((x), 'J', "J") > diff --git a/gcc/testsuite/g++.target/s390/atomic-align-1.C b/gcc/testsuite/g++.target/s390/atomic-align-1.C > new file mode 100644 > index 00000000000..43aa0bc39ed > --- /dev/null > +++ b/gcc/testsuite/g++.target/s390/atomic-align-1.C > @@ -0,0 +1,25 @@ > +/* { dg-do compile { target int128 } } */ > +/* { dg-options "-std=c++11" } */ > +/* { dg-final { scan-assembler-times {\.align\t2} 2 } } */ > +/* { dg-final { scan-assembler-times {\.align\t4} 2 } } */ > +/* { dg-final { scan-assembler-times {\.align\t8} 3 } } */ > +/* { dg-final { scan-assembler-times {\.align\t16} 2 } } */ > + > +#include > + > +// 2 > +std::atomic var_char; > +std::atomic var_short; > +// 4 > +std::atomic var_int; > +// 8 > +std::atomic var_long; > +std::atomic var_long_long; > +// 16 > +std::atomic<__int128> var_int128; > +// 4 > +std::atomic var_float; > +// 8 > +std::atomic var_double; > +// 16 > +std::atomic var_long_double; > diff --git a/gcc/testsuite/gcc.target/s390/atomic-align-1.c b/gcc/testsuite/gcc.target/s390/atomic-align-1.c > new file mode 100644 > index 00000000000..b2e1233e3ee > --- /dev/null > +++ b/gcc/testsuite/gcc.target/s390/atomic-align-1.c > @@ -0,0 +1,23 @@ > +/* { dg-do compile { target int128 } } */ > +/* { dg-options "-std=c11" } */ > +/* { dg-final { scan-assembler-times {\.align\t2} 2 } } */ > +/* { dg-final { scan-assembler-times {\.align\t4} 2 } } */ > +/* { dg-final { scan-assembler-times {\.align\t8} 3 } } */ > +/* { dg-final { scan-assembler-times {\.align\t16} 2 } } */ > + > +// 2 > +_Atomic char var_char; > +_Atomic short var_short; > +// 4 > +_Atomic int var_int; > +// 8 > +_Atomic long var_long; > +_Atomic long long var_long_long; > +// 16 > +_Atomic __int128 var_int128; > +// 4 > +_Atomic float var_float; > +// 8 > +_Atomic double var_double; > +// 16 > +_Atomic long double var_long_double; > diff --git a/gcc/testsuite/gcc.target/s390/atomic-align-2.c b/gcc/testsuite/gcc.target/s390/atomic-align-2.c > new file mode 100644 > index 00000000000..0bf17341bf8 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/s390/atomic-align-2.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile { target int128 } } */ > +/* { dg-options "-O -std=c11" } */ > +/* { dg-final { scan-assembler-not {abort} } } */ > + > +/* The stack is 8 byte aligned which means GCC has to manually align a 16 byte > + aligned object. This is done by allocating not 16 but rather 24 bytes for > + variable X and then manually aligning a pointer inside the memory block. > + Validate this by ensuring that the if-statement is optimized out. */ > + > +void bar (_Atomic unsigned __int128 *ptr); > + > +void foo (void) { > + _Atomic unsigned __int128 x; > + unsigned long n = (unsigned long)&x; > + if (n % 16 != 0) > + __builtin_abort (); > + bar (&x); > +}