From patchwork Fri Jan 25 17:13:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Richard Earnshaw (lists)" X-Patchwork-Id: 1031210 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-494751-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="p15y6wjM"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43mQbT5K76z9s55 for ; Sat, 26 Jan 2019 04:13:40 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=rNYqKI2sEeMspr9lCPy51g8g8oZxirVnpp1c8pPRLB8W7PC78A zyZynS+qszXTQ+q/m/wx365UsZygXkb+Kwvma5St1iRGmaL4eeIVcfjHweJpCh4r XPo9WN0fKMODZxO3D3aCyhRRS/evATcm+mCJO6GcJNuO0anKj/gTp1uhs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=fWsimgW1Gr9DgZAGs0vSj7W4CRM=; b=p15y6wjMx2tHC0s39pcl EA2eOtupEGxHHlo8zgq9v2QQZvZeKllMfBGhWlZKeRrpJDUej4fh/KiNlz5wrAI4 GYiAIql1GNIR9VlwT9haDb9keP9kcVxxE5ZveTTnaR67DNqiImT/afxO3CzLN/uP 15NJEW0ulQVg71bxbiwiGII= Received: (qmail 107375 invoked by alias); 25 Jan 2019 17:13:33 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 107358 invoked by uid 89); 25 Jan 2019 17:13:33 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy=c8, MIN, C8 X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 25 Jan 2019 17:13:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A8464A78; Fri, 25 Jan 2019 09:13:29 -0800 (PST) Received: from e120077-lin.cambridge.arm.com (e120077-lin.cambridge.arm.com [10.2.206.231]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0D9B93F5AF; Fri, 25 Jan 2019 09:13:28 -0800 (PST) To: gcc-patches From: "Richard Earnshaw (lists)" Subject: [aarch64] Fix ABI breakage with 128-bit bitfield types. Openpgp: preference=signencrypt Message-ID: Date: Fri, 25 Jan 2019 17:13:27 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 This is pretty unlikely in real code, but similar to Arm, the AArch64 ABI has a bug with the handling of 128-bit bit-fields, where if the bit-field dominates the overall alignment the back-end code may end up passing the argument correctly. This is a regression that started in gcc-6 when the ABI support code was updated to support overaligned types. The fix is very similar in concept to the Arm fix. 128-bit bit-fields are fortunately extremely rare, so I'd be very surprised if anyone has been bitten by this. PR target/88469 gcc/ * config/aarch64/aarch64.c (aarch64_function_arg_alignment): Add new argument ABI_BREAK. Set to true if the calculated alignment has changed in gcc-9. Check bit-fields for their base type alignment. (aarch64_layout_arg): Warn if argument passing has changed in gcc-9. (aarch64_function_arg_boundary): Likewise. (aarch64_gimplify_va_arg_expr): Likewise. gcc/testsuite/ * gcc.target/aarch64/aapcs64/test_align-10.c: New test. * gcc.target/aarch64/aapcs64/test_align-11.c: New test. * gcc.target/aarch64/aapcs64/test_align-12.c: New test. Committed to trunk. diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 5df5a8b7843..d6a9955804f 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -3765,12 +3765,16 @@ 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). - This is a helper function for local use only. */ + 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. */ static unsigned int -aarch64_function_arg_alignment (machine_mode mode, const_tree type) +aarch64_function_arg_alignment (machine_mode mode, const_tree type, + bool *abi_break) { + *abi_break = false; if (!type) return GET_MODE_ALIGNMENT (mode); @@ -3786,9 +3790,22 @@ aarch64_function_arg_alignment (machine_mode mode, const_tree type) return TYPE_ALIGN (TREE_TYPE (type)); unsigned int alignment = 0; + unsigned int bitfield_alignment = 0; for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) if (TREE_CODE (field) == FIELD_DECL) - alignment = std::max (alignment, DECL_ALIGN (field)); + { + alignment = std::max (alignment, DECL_ALIGN (field)); + if (DECL_BIT_FIELD_TYPE (field)) + bitfield_alignment + = std::max (bitfield_alignment, + TYPE_ALIGN (DECL_BIT_FIELD_TYPE (field))); + } + + if (bitfield_alignment > alignment) + { + *abi_break = true; + return bitfield_alignment; + } return alignment; } @@ -3805,6 +3822,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, int ncrn, nvrn, nregs; bool allocate_ncrn, allocate_nvrn; HOST_WIDE_INT size; + bool abi_break; /* We need to do this once per argument. */ if (pcum->aapcs_arg_processed) @@ -3881,25 +3899,28 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, entirely general registers. */ if (allocate_ncrn && (ncrn + nregs <= NUM_ARG_REGS)) { - gcc_assert (nregs == 0 || nregs == 1 || nregs == 2); /* C.8 if the argument has an alignment of 16 then the NGRN is - rounded up to the next even number. */ + rounded up to the next even number. */ if (nregs == 2 && ncrn % 2 /* The == 16 * BITS_PER_UNIT instead of >= 16 * BITS_PER_UNIT comparison is there because for > 16 * BITS_PER_UNIT alignment nregs should be > 2 and therefore it should be passed by reference rather than value. */ - && aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT) + && (aarch64_function_arg_alignment (mode, type, &abi_break) + == 16 * BITS_PER_UNIT)) { + if (abi_break && warn_psabi && currently_expanding_gimple_stmt) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 9.1", type); ++ncrn; gcc_assert (ncrn + nregs <= NUM_ARG_REGS); } /* NREGS can be 0 when e.g. an empty structure is to be passed. - A reg is still generated for it, but the caller should be smart + A reg is still generated for it, but the caller should be smart enough not to use it. */ if (nregs == 0 || nregs == 1 || GET_MODE_CLASS (mode) == MODE_INT) pcum->aapcs_reg = gen_rtx_REG (mode, R0_REGNUM + ncrn); @@ -3931,9 +3952,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, on_stack: pcum->aapcs_stack_words = size / UNITS_PER_WORD; - if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT) - pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size, - 16 / UNITS_PER_WORD); + if (aarch64_function_arg_alignment (mode, type, &abi_break) + == 16 * BITS_PER_UNIT) + { + int new_size = ROUND_UP (pcum->aapcs_stack_size, 16 / UNITS_PER_WORD); + if (pcum->aapcs_stack_size != new_size) + { + if (abi_break && warn_psabi && currently_expanding_gimple_stmt) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 9.1", type); + pcum->aapcs_stack_size = new_size; + } + } return; } @@ -4022,7 +4052,13 @@ aarch64_function_arg_regno_p (unsigned regno) static unsigned int aarch64_function_arg_boundary (machine_mode mode, const_tree type) { - unsigned int alignment = aarch64_function_arg_alignment (mode, type); + bool abi_break; + unsigned int alignment = aarch64_function_arg_alignment (mode, type, + &abi_break); + if (abi_break & warn_psabi) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 9.1", type); + return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY); } @@ -13320,7 +13356,10 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist), f_stack, NULL_TREE); size = int_size_in_bytes (type); - align = aarch64_function_arg_alignment (mode, type) / BITS_PER_UNIT; + + bool abi_break; + align + = aarch64_function_arg_alignment (mode, type, &abi_break) / BITS_PER_UNIT; dw_align = false; adjust = 0; @@ -13367,7 +13406,12 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, nregs = rsize / UNITS_PER_WORD; if (align > 8) - dw_align = true; + { + if (abi_break && warn_psabi) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 9.1", type); + dw_align = true; + } if (BLOCK_REG_PADDING (mode, type, 1) == PAD_DOWNWARD && size < UNITS_PER_WORD) diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c new file mode 100644 index 00000000000..af0c8a1f412 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c @@ -0,0 +1,44 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-10.c" + +struct s +{ + /* Should have 128-bit alignment. */ + __int128 y : 65; + char z: 7; +}; + +typedef struct s T; + +#define EXPECTED_STRUCT_SIZE 16 +extern void link_failure (void); +int +foo () +{ + /* Optimization gets rid of this before linking. */ + if (sizeof (struct s) != EXPECTED_STRUCT_SIZE) + link_failure (); +} + +T a = { 1, 4 }; +T b = { 9, 16 }; +T c = { 25, 36 }; + +#include "abitest.h" +#else + ARG (int, 3, W0) + ARG (T, a, X2) + ARG (int, 5, W4) + ARG (T, b, X6) +#ifndef __AAPCS64_BIG_ENDIAN__ + ARG (int, 7, STACK) +#else + ARG (int, 7, STACK + 4) +#endif + /* Natural alignment should be 16. */ + LAST_ARG (T, c, STACK + 16) +#endif diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-11.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-11.c new file mode 100644 index 00000000000..357694902cd --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-11.c @@ -0,0 +1,44 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-11.c" + +struct s +{ + /* Should have 128-bit alignment and still detected as a bitfield. */ + __int128 y : 64; + char z: 7; +}; + +typedef struct s T; + +#define EXPECTED_STRUCT_SIZE 16 +extern void link_failure (void); +int +foo () +{ + /* Optimization gets rid of this before linking. */ + if (sizeof (struct s) != EXPECTED_STRUCT_SIZE) + link_failure (); +} + +T a = { 1, 4 }; +T b = { 9, 16 }; +T c = { 25, 36 }; + +#include "abitest.h" +#else + ARG (int, 3, W0) + ARG (T, a, X2) + ARG (int, 5, W4) + ARG (T, b, X6) +#ifndef __AAPCS64_BIG_ENDIAN__ + ARG (int, 7, STACK) +#else + ARG (int, 7, STACK + 4) +#endif + /* Natural alignment should be 16. */ + LAST_ARG (T, c, STACK + 16) +#endif diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-12.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-12.c new file mode 100644 index 00000000000..5b3f74b51dc --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-12.c @@ -0,0 +1,45 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-12.c" + +struct s +{ + /* Should have 64-bit alignment. */ + long long y : 57; + char z: 7; +}; + +typedef struct s T; + +#define EXPECTED_STRUCT_SIZE 8 +extern void link_failure (void); +int +foo () +{ + /* Optimization gets rid of this before linking. */ + if (sizeof (struct s) != EXPECTED_STRUCT_SIZE) + link_failure (); +} + +T a = { 1, 4 }; +T b = { 9, 16 }; +T c = { 25, 36 }; + +#include "abitest.h" +#else + ARG (int, 3, W0) + ARG (T, a, X1) + ARG (int, 5, W2) + ARG (T, b, X3) + ARG (__int128, 11, X4) + ARG (__int128, 13, X6) +#ifndef __AAPCS64_BIG_ENDIAN__ + ARG (int, 7, STACK) +#else + ARG (int, 7, STACK + 4) +#endif + LAST_ARG (T, c, STACK + 8) +#endif