From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80045.outbound.protection.outlook.com [40.107.8.45]) by sourceware.org (Postfix) with ESMTPS id 816B0385AE66 for ; Thu, 9 Jun 2022 09:40:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 816B0385AE66 ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=SKAukTv2QnFsygr8pCTvXkynau6UCxAMrt8eDJCsX2cSZZlYJHkKZbvIEtsAN+Rtuc4O6MmHhqQjBMOBYPuXm0dwQjEci4R/CvOkHxnvdZLtH5VQUFtHXiT71g+TPMTkBgjoPxjVfk2RRXfcOIlz5WEvAybQW7JIs71f0PSdFMjHHFPKP4bYswWZkMkbbFV1U1EHF+6+wCWRbIUFujeXaKMEwITupM0OvVv/vRx3nQOtqXLKcVfaIlwQraBstYgCrfDDpccKNO2bunoWGuMo0MKey6DTGJU3fXvnQGTlWZNtAr8LA4oZiBGH77lru6QseuWUqko8g1nowX+0WhyjZA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4YjYe8yaSUYsAJzkDkOaHuGt8v60NBw5UUpAZC01euc=; b=gvXnemUBeyPtyIlQL3Bu4O/+epPDpzAurr9lizKQIzmo3I6SlghOzg3t8mUzBPLXb2xpLws79VU/lKB4GwEDtE098GfIpIWALLWX9zifg/A3yLIs4euFtKo+oa7Hn/GJtZTMk75jkvZTfVJ+KcKu333Gz+1YY9b/hCGqKO0h/hMhopsInACCWrUAi0KvoYOkZnNBrMm4PZn8lyYl//4lYGy/o0LRhEZD4D47VcJ14CkLtZNOynUDZ5Ky/f/9nefxhV9/alcxNLZoWQRcCpWAq9M4enynMFYPz7B5Vz8E9MfiZjsm8YG/pFF65DL3C64poF3w5rViTPveYsU+rsVtTg== ARC-Authentication-Results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=gcc.gnu.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com]) Received: from AM6PR02CA0005.eurprd02.prod.outlook.com (2603:10a6:20b:6e::18) by AM0PR08MB3652.eurprd08.prod.outlook.com (2603:10a6:208:d5::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5332.13; Thu, 9 Jun 2022 09:40:28 +0000 Received: from AM5EUR03FT016.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:6e:cafe::c1) by AM6PR02CA0005.outlook.office365.com (2603:10a6:20b:6e::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5332.13 via Frontend Transport; Thu, 9 Jun 2022 09:40:28 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM5EUR03FT016.mail.protection.outlook.com (10.152.16.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5332.12 via Frontend Transport; Thu, 9 Jun 2022 09:40:28 +0000 Received: ("Tessian outbound 5b5a41c043d3:v120"); Thu, 09 Jun 2022 09:40:28 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: c64edc2582599205 X-CR-MTA-TID: 64aa7808 Received: from d81a4e395137.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id F9DD921D-CB9F-4FEE-9F7E-89A6ADB91468.1; Thu, 09 Jun 2022 09:40:19 +0000 Received: from EUR04-HE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id d81a4e395137.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 09 Jun 2022 09:40:19 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TvjxPalHw5j051e+UxptooW6W8qb0p3VCmitULJFBZJPMwnAui56OtReWsVCfRm2FvcV7CWFfjpxWxlYXzbyuhaXPO++ttfffq2wzXQTOMmlvxCF/xNpgJt/Ur9jTZzsgEjaqzZGNm2qPfuR+p6+hATQB5QmgUVALTqhS2t+6dnddw7UqT7N7bCEObp7ByCPcCfWu9yzHX/wAgTQAiRMZROku9ORTPC7iCzU/cQyolBxmFqJ6u9PPa3JKOBCAOlvZZ0OQCr2y9bXDKsa+gxpy5wUTq64vMjebul5HsFrL7ZVP/KcZB3kxUsM2NmXy4vYbdZVbaTF9cFbX5c4YUMxFA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4YjYe8yaSUYsAJzkDkOaHuGt8v60NBw5UUpAZC01euc=; b=Kqa5Vx1EidNWJRLPhrBVNtu5a6IX/+6sz/NKulRWI8OC943/H9eKQYyAySi3cN2MkPwXtRncl0qx+iGl16+/BgMngd753Vf6fa7Q4h5H6+go1i1zBPhJiIAXI+0JNGJmTJpMmy43nT8d/DfLhq6qttJVSF3z+RIFW2t6K2Zs+/9UJFDFTapaVHFiH4eUzvwHTKGFp7+7CqNy9/LoNHw6jtXRFMiyj+pcph5RHJwonoKKc7NsSiKPHxMSHQ1ViR6INlhEJ+LnOWtI6SWzdL7mNhf7cbz+vu2bjHYVAS6kBOhddcnH2Lccue44DlCRsDEw07fdvEE0M8+1x4m8sXX3Ng== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from VI1PR08MB3390.eurprd08.prod.outlook.com (2603:10a6:803:7d::27) by AM9PR08MB7227.eurprd08.prod.outlook.com (2603:10a6:20b:412::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5332.13; Thu, 9 Jun 2022 09:40:14 +0000 Received: from VI1PR08MB3390.eurprd08.prod.outlook.com ([fe80::4cab:84d0:9b6c:a39e]) by VI1PR08MB3390.eurprd08.prod.outlook.com ([fe80::4cab:84d0:9b6c:a39e%7]) with mapi id 15.20.5314.019; Thu, 9 Jun 2022 09:40:14 +0000 Message-ID: <551e2992-1eeb-902d-1c32-829f973ce6cc@arm.com> Date: Thu, 9 Jun 2022 11:40:16 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: aarch64: Fix bitfield alignment in param passing [PR105549] Content-Language: en-US To: Christophe Lyon via Gcc-patches , richard.sandiford@arm.com References: <20220607162431.243236-1-christophe.lyon@arm.com> <08233c21-a521-171b-78d6-5c6fb8460f95@arm.com> From: Christophe Lyon In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: SA0PR11CA0210.namprd11.prod.outlook.com (2603:10b6:806:1bc::35) To VI1PR08MB3390.eurprd08.prod.outlook.com (2603:10a6:803:7d::27) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 15d27f15-e629-4abf-76ec-08da49fc1699 X-MS-TrafficTypeDiagnostic: AM9PR08MB7227:EE_|AM5EUR03FT016:EE_|AM0PR08MB3652:EE_ X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: tsChzSMloxR3ZoUYOHmn7787Rb19FFNjO1ooqNDBu7+kMzknrefykFDnQEbEjE7SXKQpAF56H7xr49XSJIanXsI1DSF5uNGlsCqePLYeFDDIBsifwi+UW8/y4+E25ZHr6QtLo+ElKFLoxPz4bibllgGWXpO/Ld9oIVqobU8REI3VFnC/hKO+GoXa4e1U9/66D4BTXLSllKe6vj2tpG+WAoFNhkC74dfW0ukl3YXG46i5WbsCjON3faWKGkriFhk4a25t1wrTPaV0y8b9Zmdu/JNua+vUqNvUbJpc3cgmm81Ql5G9yoofPk7i8DqcG8cE+CFRY+68lQf8zvwpCI7/mvhGpziHf8tMgprQb5DI+Coxi3fA4R2LPws/y7IzWO48OmUSMU3ZzcbZlab+Nu1zBA/Ag9QEvPw3i+jQuwAYpdAHgqSzb33lAbp63ABvUEkrvUsDHF9t7UJ3Y1mTrq6EaMaY9iDR+HXPBBvh9JpNi2pxT6LxDKwd7kr+Ir385D54/ns9jrFpzOzpJIIyuV5vKqIoihRbDqk7cha6WatSV/2meCdQ4xS44X66fm3Vx3GbpHERI/QJoV8raahLeNEEPSVOwFrGbkk7xZrEX+4S5eG96PqlwH+++yQvyPGb7oK92v25Spvi5fHpqMY3HKPMqJJdYMiBmxRvQGXavjwNPM6YlR2wrBWsf+cuLfP7zypqyLjzM/fIeZlEO8cpFh1aN8tjwDyqvitQXqjFzrvqtUw/yqt9cGXk9wcFOGAu7lsI4uj6kdFhbay/DV+38OPJQBXlItSLb2ZYDWb/IaLmReFpSiYicT6ENPnNc+dq4bldJmst8vnAsdxzSYN4hq0T3M1fz9euBe6EFCCE93L1KRc= X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR08MB3390.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(4636009)(366004)(316002)(38100700002)(84970400001)(8676002)(31696002)(66556008)(86362001)(66476007)(6636002)(66946007)(36756003)(44832011)(186003)(2616005)(31686004)(30864003)(53546011)(2906002)(6506007)(83380400001)(6512007)(26005)(508600001)(6486002)(8936002)(5660300002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR08MB7227 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT016.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 2bdeea20-e06a-42bf-fa57-08da49fc0dc1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: eTb7nUuf524shyfJpa6VGOrG6KyxkAG83aHXmV1RgsHUu362tusx5QxuANQ7T8bejgdzMphsOkXHrjHlCJL6qjgx1qvsSFFtj11Ea7Skp39tKmq/FteB/XwlqR4kb1AO4HycL8kjgklKlTsGztpIYZJPolk8iS94EQS0XO7kx3swB1fXzQJHqCz9FeybYWbg3PFTvYUg0XYKWnNbJR09lEsP6V6AwTKVGAtS9+bAweDvZkTTXCmO9Wc/eiHAvjdZL9QCXg29/6x4f3+Ut4zAm7JydCLErLUjlO2GbshGr/AwRepCeNQ7yV8Ien9oJ8wQbV55pxTWpPKLvC/3C2ZYVpkEJAZu50NPFyPrtk98bsZiEP/gRvXvHBtqURk4COoahmcj9prn8PloeLtOC6SdTBqqL6IiFZr7/h7wcb7uIz+27mU5Gl6SH2Y79LOlnnJUnRIw+cbP8IfYDlL17pn4/QkcdnDzZ//swVLXYKx3UsC3vy7KCUYIZe1RcYSSApklzfmIQJDoX0bFWxdvhFPLVjoiG0scH2dMb/Bw+TZs8fPehDMi2kOBScZbl9SR8WVaFeoEplv7keneL77F2zqwC+GLc04PsQkUaedjdyeZwwx9a0GwKMXRY+dKTw1wFK4Y4jcOXi3ABJ+3L2YT9B38RGoLQ1yLzhzN3CeG/DflUUKjYokoj80w3fPZk5g1kRwT5ciiuo/S3fznAB9skuQOR8ljFnj17Cvb9GXIumSTxYhFEcAY2qFobI1EZ5TQ7K/nSwpDOnQoeO15vS9voMxGZ48kVNTtHh6PRCGAEqcOkBaGRFJW/IXTDL1nOkp8PgkfZclZZnuOn4RUUMa9EZB+0A== X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFS:(13230001)(4636009)(36840700001)(46966006)(40470700004)(6486002)(31696002)(2906002)(84970400001)(508600001)(86362001)(40460700003)(356005)(82310400005)(316002)(70586007)(70206006)(8676002)(81166007)(36860700001)(31686004)(83380400001)(8936002)(5660300002)(44832011)(30864003)(6636002)(186003)(336012)(47076005)(26005)(36756003)(6512007)(2616005)(53546011)(6506007)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jun 2022 09:40:28.5307 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 15d27f15-e629-4abf-76ec-08da49fc1699 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM5EUR03FT016.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB3652 X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, FORGED_SPF_HELO, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_DMARC_NONE, KAM_LOTSOFHASH, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SCC_5_SHORT_WORD_LINES, SPF_HELO_PASS, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE, UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Thu, 09 Jun 2022 09:40:34 -0000 On 6/8/22 15:19, Richard Sandiford wrote: > Christophe Lyon writes: >> On 6/7/22 19:44, Richard Sandiford wrote: >>> Christophe Lyon via Gcc-patches writes: >>>> While working on enabling DFP for AArch64, I noticed new failures in >>>> gcc.dg/compat/struct-layout-1.exp (t028) which were not actually >>>> caused by DFP types handling. These tests are generated during 'make >>>> check' and enabling DFP made generation different (not sure if new >>>> non-DFP tests are generated, or if existing ones are generated >>>> differently, the tests in question are huge and difficult to compare). >>>> >>>> Anyway, I reduced the problem to what I attach at the end of the new >>>> gcc.target/aarch64/aapcs64/va_arg-17.c test and rewrote it in the same >>>> scheme as other va_arg* AArch64 tests. Richard Sandiford further >>>> reduced this to a non-vararg function, added as a second testcase. >>>> >>>> This is a tough case mixing bitfields and alignment, where >>>> aarch64_function_arg_alignment did not follow what its descriptive >>>> comment says: we want to use the natural alignment of the bitfield >>>> type only if the user didn't override the alignment for the bitfield >>>> itself. >>>> >>>> The fix is thus very small, and this patch adds two new tests >>>> (va_arg-17.c and pr105549.c). va_arg-17.c contains the reduced >>>> offending testcase from struct-layout-1.exp for reference. >>>> >>>> We also take the opportunity to fix the comment above >>>> aarch64_function_arg_alignment since the value of the abi_break >>>> parameter was changed in a previous commit, no longer match the >>>> description. >>>> >>>> 2022-06-02 Christophe Lyon >>>> >>>> gcc/ >>>> PR target/105549 >>>> * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): >>>> Check DECL_USER_ALIGN for bitfield. >>>> >>>> gcc/testsuite/ >>>> PR target/105549 >>>> * gcc.target/aarch64/aapcs64/va_arg-17.c: New. >>>> * gcc.target/aarch64/pr105549.c: New. >>>> >>>> >>>> ############### Attachment also inlined for ease of reply ############### >>>> >>>> >>>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc >>>> index 40fc5e633992036a2c06867857a681792178ef00..2c6ccce7cb5dc32097d24514ee525729efb6b7ff 100644 >>>> --- a/gcc/config/aarch64/aarch64.cc >>>> +++ b/gcc/config/aarch64/aarch64.cc >>>> @@ -7262,9 +7262,9 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, >>>> /* Given MODE and TYPE of a function argument, return the alignment in >>>> bits. The idea is to suppress any stronger alignment requested by >>>> the user and opt for the natural alignment (specified in AAPCS64 \S >>>> - 4.1). ABI_BREAK is set to true if the alignment was incorrectly >>>> - calculated in versions of GCC prior to GCC-9. This is a helper >>>> - function for local use only. */ >>>> + 4.1). ABI_BREAK is set to the old alignment if the alignment was >>>> + incorrectly calculated in versions of GCC prior to GCC-9. This is >>>> + a helper function for local use only. */ >>>> >>>> static unsigned int >>>> aarch64_function_arg_alignment (machine_mode mode, const_tree type, >>>> @@ -7304,7 +7304,10 @@ aarch64_function_arg_alignment (machine_mode mode, const_tree type, >>>> "s" contains only one Fundamental Data Type (the int field) >>>> but gains 8-byte alignment and size thanks to "e". */ >>>> alignment = std::max (alignment, DECL_ALIGN (field)); >>>> - if (DECL_BIT_FIELD_TYPE (field)) >>>> + >>>> + /* Take bit-field type's alignment into account only if the >>>> + user didn't override this field's alignment. */ >>>> + if (DECL_BIT_FIELD_TYPE (field) && !DECL_USER_ALIGN (field)) >>> >>> I think we need to check DECL_PACKED instead. On its own, an alignment >>> attribute on the field can only increase alignment, not decrease it. >>> In constrast, the packed attribute effectively forces the alignment to >>> 1 byte, so has an effect even without an alignment attribute. Adding an >>> explicit alignment on top can then increase the alignment from 1 to any >>> value (bigger or smaller than the original underlying type). >> >> Right, but the comment before aarch64_function_arg_alignment says: >> >> "The idea is to suppress any stronger alignment requested by the user >> and opt for the natural alignment (specified in AAPCS64 \S 4.1)" >> >> When using DECL_PACKED, wouldn't we check the opposite of this (ie. that >> the user requested a smaller alignment)? I mean we'd not "suppress >> stronger alignment" since such cases do not have DECL_PACKED? > > I think "stronger alignment" here means "greater alignment" rather > than "less alignment". But in these examples we're dealing with > alignments of the fields. I think that part is OK, and that the > intention is to ignore any greater alignment specified at the structure > level, independently of the fields. > > In other words, if field list X occupies 16 bytes, then S1 and S2 > below should be handled in the same way as far as register assignment > is concerned: > > struct S1 { X }; > struct S2 { X } __attribute__((aligned(16))); > > The idea is that structures are just a sequence of fields/members > and don't have any "magic" properties beyond that. > >> However I'm not sure which part of the ABI is mentioned in the comment, >> in my copy 4.1 is "Design Goals" and does not elaborate on bitfields and >> parameters. > > Yeah, the section numbers change as new stuff is added, and probably > also changed in the move to rst. I think the comment is referring > to the following, and in particular to the first bullet point under > "Aggregates": > > -------------------------------------------------------------------------- > Composite Types > --------------- > > A Composite Type is a collection of one or more Fundamental Data Types that are handled as a single entity at the procedure call level. A Composite Type can be any of: > > - An aggregate, where the members are laid out sequentially in memory (possibly with inter-member padding) > > - A union, where each of the members has the same address > > - An array, which is a repeated sequence of some other type (its base type). > > The definitions are recursive; that is, each of the types may contain a Composite Type as a member. > > * The *member alignment* of an element of a composite type is the > alignment of that member after the application of any language alignment > modifiers to that member > > * The *natural alignment* of a composite type is the maximum of > each of the member alignments of the 'top-level' members of the composite > type i.e. before any alignment adjustment of the entire composite is > applied > > Aggregates > ^^^^^^^^^^ > > - The alignment of an aggregate shall be the alignment of its most-aligned member. > > - The size of an aggregate shall be the smallest multiple of its alignment that is sufficient to hold all of its members. > -------------------------------------------------------------------------- > > So: > > - Types start out with a "natural alignment". For built-in types this > is defined directly in the AAPCS64. For composite types it is worked > out as above (plus similar rules for unions and arrays, snipped above). > > - Non-bitfield members have an alignment that is by default the same as > the natural alignment of their type. However, for C/C++ this can be > modified by things like: > > - an extra alignment applied via a typedef > - a packed attribute on the field/member > - a packed attribute on the containing struct (which propagates to > all fields/members) > - a pack pragma > - an alignment attribute on the field/member > - probably others too > > - Bitfield members are handled as described separately in the AAPCS64. > >>> E.g. for: >>> >>> --------------------------------------------------------------------- >>> typedef unsigned long long ull __attribute__((aligned(ALIGN))); >>> >>> #ifndef EXTRA >>> #define EXTRA unsigned long long x; >>> #endif >>> >>> struct S1 { __attribute__((aligned(1))) ull i : 1; EXTRA }; >>> struct S2 { __attribute__((aligned(2))) ull i : 1; EXTRA }; >>> struct S4 { __attribute__((aligned(4))) ull i : 1; EXTRA }; >>> struct S8 { __attribute__((aligned(8))) ull i : 1; EXTRA }; >>> struct S16 { __attribute__((aligned(16))) ull i : 1; EXTRA }; >>> >>> struct Sp { ull i : 1; EXTRA }__attribute__((packed)); >>> struct S1p { __attribute__((packed, aligned(1))) ull i : 1; EXTRA }; >>> struct S2p { __attribute__((packed, aligned(2))) ull i : 1; EXTRA }; >>> struct S4p { __attribute__((packed, aligned(4))) ull i : 1; EXTRA }; >>> struct S8p { __attribute__((packed, aligned(8))) ull i : 1; EXTRA }; >>> struct S16p { __attribute__((packed, aligned(16))) ull i : 1; EXTRA }; >>> >>> int f1(int a, struct S1 s) { return s.i; } >>> int f2(int a, struct S2 s) { return s.i; } >>> int f4(int a, struct S4 s) { return s.i; } >>> int f8(int a, struct S8 s) { return s.i; } >>> int f16(int a, struct S16 s) { return s.i; } >>> >>> int fp(int a, struct Sp s) { return s.i; } >>> int f1p(int a, struct S1p s) { return s.i; } >>> int f2p(int a, struct S2p s) { return s.i; } >>> int f4p(int a, struct S4p s) { return s.i; } >>> int f8p(int a, struct S8p s) { return s.i; } >>> int f16p(int a, struct S16p s) { return s.i; } >>> --------------------------------------------------------------------- >>> >>> there are at least 4 interesting cases: >>> >>> {-DALIGN=8,-DALIGN=16} x {-DEXTRA=,} >>> >>> From my reading of the ABI, clang gets all of them right. >> Which sections of the ABI? I've re-read AAPCS64 (5.9.4 bit-fields >> subdivision, 8.1.8 bit-fields), and these specific cases are not clear >> to me :-( > > Hope the above answers this. It helps, thanks. However.... > >>> The case GCC currently gets wrong is: >>> >>> fp f1p f2p f4p f8p for -DALIGN=16 [A] >>> >>> f1 to f16 are equivalent for -DALIGN=16: all the structures have 16-byte >>> alignment, despite the user alignments on the fields. We currently >>> handle those correctly. .... I replaced !DECL_USER_ALIGN (field) with DECL_PACKED (field) and I noticed no change with the testcase you provided. In more details, with ALIGN=8, we currently generate: str x1, [sp] for f1, f2, f4, f8, f8p stp x2, x3, [sp] for f16, f16p strb w1, [sp, 8] for fp, f1p strh w1, [sp, 8] for f2p str w1, [sp, 8] for f4p with ALIGN=16: stp x2, x3, [sp] for f1, f2, f4, f8, f16, f16p strb w1, [sp, 8] for fp, f1p strh w1, [sp, 8] for f2p str w1, [sp, 8] for f4p str x1, [sp] for f8p My patch has no impact on the ALIGN=8 case (as expected), and with ALIGN=16 it replaces stp x2, x3, [sp] with stp x1, x2, [sp] in f1, f2, f4, f8 Isn't this what we want? >>> Unfortunately, fixing [A] means that this is an ABI break from GCC 12, >>> so we'll need (yet another) -Wpsabi warning. In this case I guess we're >>> actually restoring the pre-GCC 9.1 behaviour. >> gasp! I hoped we'd be safe as this is a bug fix ;-) >> >> Do you mean that commit r9-5650-gc590597c45948c was in fact a mistake? > > No, it fixed many cases. I just think [A] were caught by accident, > due to the AST representation being difficult to use. > >> I've been looking at it and at r11-8226-g49813aad3292f7, it's a bit >> convoluted :-) > > Yeah, it's not going to be pretty. The current code warns about > cases where the alignment has been bumped from 64 bits to 128 bits. > Here we'll be warning about the opposite direction (if taking GCC 12 > as a baseline). > > Thanks, > Richard > >> >> >>> >>> An interesting oddity with these rules is that for: >>> >>> --------------------------------------------------------------------- >>> struct S1 { unsigned long long a, b; } __attribute__((aligned(16))); >>> struct S2 { struct S1 s; }; >>> >>> int f1(int a, struct S1 s1) { return s1.a; } >>> int f2(int a, struct S2 s2) { return s2.s.a; } >>> --------------------------------------------------------------------- >>> >>> S1 is not treated as 16-byte aligned, but S2 is (because the analysis >>> isn't recursive). Clang and GCC seem to be consistent about that though. >>> >>> Thanks, >>> Richard >>> >>>> bitfield_alignment >>>> = std::max (bitfield_alignment, >>>> TYPE_ALIGN (DECL_BIT_FIELD_TYPE (field))); >>>> diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c >>>> new file mode 100644 >>>> index 0000000000000000000000000000000000000000..24895c3ab48309b601f6f22c176f1e52350c2257 >>>> --- /dev/null >>>> +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c >>>> @@ -0,0 +1,105 @@ >>>> +/* Test AAPCS64 layout and __builtin_va_arg. >>>> + >>>> + This test covers a corner case where a composite type parameter fits in one >>>> + register: we do not need a double-word alignment when accessing it in the >>>> + va_arg stack area. */ >>>> + >>>> +/* { dg-do run { target aarch64*-*-* } } */ >>>> + >>>> +#ifndef IN_FRAMEWORK >>>> +#define AAPCS64_TEST_STDARG >>>> +#define TESTFILE "va_arg-17.c" >>>> +#include "type-def.h" >>>> + >>>> +enum E6 { e6_0, e6_1, e6_2, e6_3, e6_65533 = 65533, e6_65534, e6_65535 }; >>>> +typedef enum E6 Tal16E6 __attribute__((aligned (16))); >>>> +typedef unsigned int Tuint; >>>> + >>>> +int fails; >>>> + >>>> +union S2844 { >>>> + Tuint a:((((10) - 1) & 31) + 1); >>>> + Tal16E6 __attribute__((aligned (2), packed)) b:31; >>>> + struct{}c[0]; >>>> +} ; >>>> +union S2844 s2844; >>>> +union S2844 a2844[5]; >>>> + >>>> +#define HAS_DATA_INIT_FUNC >>>> +void init_data () >>>> +{ >>>> + memset (&s2844, '\0', sizeof (s2844)); >>>> + memset (a2844, '\0', sizeof (a2844)); >>>> + s2844.a = 799U; >>>> + a2844[2].a = 586U; >>>> +} >>>> + >>>> +#include "abitest.h" >>>> +#else >>>> + ARG (int , 1 , W0 , LAST_NAMED_ARG_ID) >>>> + DOTS >>>> + ANON_PROMOTED (float , 1.0f, double, 1.0, D0, 1) >>>> + ANON (union S2844 , s2844 , X1 , 2) >>>> + ANON (long long , 2LL , X2 , 3) >>>> + ANON (union S2844 , a2844[2] , X3 , 4) >>>> + LAST_ANON (union S2844 , a2844[2] , X4 , 5) >>>> +#endif >>>> + >>>> +#if 0 >>>> + /* This test is derived from a case generated by struct-layout-1.exp: */ >>>> + >>>> +enum E6 { e6_0, e6_1, e6_2, e6_3, e6_65533 = 65533, e6_65534, e6_65535 }; >>>> +typedef enum E6 Tal16E6 __attribute__((aligned (16))); >>>> +typedef unsigned int Tuint; >>>> + >>>> +int fails; >>>> + >>>> +union S2844 { >>>> + Tuint a:((((10) - 1) & 31) + 1); >>>> + Tal16E6 __attribute__((aligned (2), packed)) b:31; >>>> + struct{}c[0]; >>>> +} ; >>>> +union S2844 s2844; >>>> +union S2844 a2844[5]; >>>> + >>>> +typedef __builtin_va_list __gnuc_va_list; >>>> +typedef __gnuc_va_list va_list; >>>> + >>>> +void check2844va (int z, ...) { >>>> + union S2844 arg, *p; >>>> + va_list ap; >>>> + >>>> + __builtin_va_start(ap,z); >>>> + if (__builtin_va_arg(ap,double) != 1.0) >>>> + printf ("fail %d.%d\n", 2844, 0), ++fails; >>>> + >>>> + p = &s2844; >>>> + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ >>>> + if (p->a != arg.a) >>>> + printf ("fail %d.%d\n", 2844, 1), ++fails; >>>> + >>>> + if (__builtin_va_arg(ap,long long) != 3LL) >>>> + printf ("fail %d.%d\n", 2844, 2), ++fails; >>>> + >>>> + p = &a2844[2]; >>>> + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ >>>> + if (p->a != arg.a) >>>> + printf ("fail %d.%d\n", 2844, 3), ++fails; >>>> + >>>> + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ >>>> + if (p->a != arg.a) >>>> + printf ("fail %d.%d\n", 2844, 4), ++fails; >>>> + >>>> + __builtin_va_end(ap); >>>> +} >>>> + >>>> +int main (void) { >>>> + int i, j; >>>> + memset (&s2844, '\0', sizeof (s2844)); >>>> + memset (a2844, '\0', sizeof (a2844)); >>>> + s2844.a = 799U; >>>> + a2844[2].a = 586U; >>>> + check2844va (1, 1.0, s2844, 2LL, a2844[2], a2844[2]); >>>> + exit (fails != 0); >>>> +} >>>> +#endif /* 0 */ >>>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr105549.c b/gcc/testsuite/gcc.target/aarch64/pr105549.c >>>> new file mode 100644 >>>> index 0000000000000000000000000000000000000000..55a91ed6bc48b56eed61d668971e890dbd3250ef >>>> --- /dev/null >>>> +++ b/gcc/testsuite/gcc.target/aarch64/pr105549.c >>>> @@ -0,0 +1,12 @@ >>>> +/* { dg-do compile } */ >>>> +/* { dg-options "-save-temps" } */ >>>> + >>>> +enum e { E1 }; >>>> +typedef enum e e __attribute__((aligned(16))); >>>> +union u { >>>> + __attribute__((aligned(2), packed)) e a : 1; >>>> + int x[4]; >>>> +}; >>>> +union u g(int a, union u u2) { return u2; } >>>> + >>>> +/* { dg-final { scan-assembler "stp\tx1, x2, \\\[sp, 8\\\]" } } */