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: 156615 Delivered-To: patch@linaro.org Received: by 2002:a02:48:0:0:0:0:0 with SMTP id 69csp598940jaa; Fri, 25 Jan 2019 09:13:46 -0800 (PST) X-Google-Smtp-Source: ALg8bN7cb0qOWb/HrTu6u8/WoZ8A0j0lH2Gys24Y0MYAveUyTe03aMAMQAsGGzgcDelM17jlLsHG X-Received: by 2002:a63:e001:: with SMTP id e1mr4444436pgh.39.1548436426362; Fri, 25 Jan 2019 09:13:46 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1548436426; cv=none; d=google.com; s=arc-20160816; b=sLvnjcO7ElYg36PgxVt1BFJC9xTKsY9r4Ev0AE1jI2w1joCJaO3XxodI+8Xs8VfzRl BsZFlU2s6xBxrA9+TuJ257tf6h4aFrojOfK/qlpnNy1fEEjzPK2Gdab/hfyTQCti0P/L PFmEa9KP6hDNdxtkPLT3OVK5ziti+gfHhIVD44fL+M5XM8Pbm+TH51/tyJ06I1/H3npX zKGUrmtPzUDSoIdPr0k2mYNkGmJEec7dyH46BpDgKoIqqdDdq+7OGUnImNp59uuTmKNQ 4H18wvTOc7flL3T+gPxyMv3G0PpwmmxFDzJIjXRUsu9/zY6QsTYNOh0AioRQFtvI8iLO BcPQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=mime-version:user-agent:date:message-id:openpgp:subject:from:to :delivered-to:sender:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:mailing-list:dkim-signature :domainkey-signature; bh=C34eFpi03HKTVlX4Q4qwy0axqxXhY3hvviWKrgCCvyw=; b=LlQnAdwUyjsmGiPMuuxZm5kijiOReOfWivBBTRPym4mffOAEjD4jhX1bZetSLvyrX8 gZJTNgfm0odv6xGjelS/R/3Mo2g31wtNyntit8tv0lGb/9A/On7jUQrDRVrUajtJCVTL y3n4CXkIiA3ZIwLK8sLj2tdzZz0d0KynEJq+jd8lU6xwWtVLQ2hEp6ObEaLzfIWaGXrt CmoNuAQrzmY2LtevPjOhje8c2yYnjxWGn2pORmRFRmabOAwbamtvx39dF6DMkd+e2oFA DeSvqvUkPp/TqcJs96Dt5pCr59PcpBYothI3sszoBaTg+qgbyiWW4177q6VTjDFrUdg9 fPFQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=usAJ5w5O; spf=pass (google.com: domain of gcc-patches-return-494751-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="gcc-patches-return-494751-patch=linaro.org@gcc.gnu.org" Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id c1si26851435pld.194.2019.01.25.09.13.46 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 25 Jan 2019 09:13:46 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-494751-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=usAJ5w5O; spf=pass (google.com: domain of gcc-patches-return-494751-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="gcc-patches-return-494751-patch=linaro.org@gcc.gnu.org" 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=n5H2RqG6941fPiZcgDja0+v0msjVlIkS6sH5iY50WoxWDEAb3D XGNwWrhJZGrXGbanMKqC/YZ6asSUeQx8FRjN+Fqk+sreALoHnswotylTNYGp0ncd IX9BFVOr3ljtePYLmvlx5ryd1kWD/8tvzbMxMyHUQd7x5d+K1sPOPQyOY= 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=usAJ5w5OG9w1xjgQolS0 Yi3yS6nsJy//zkbhkgDOw2epn3xr75afBbFN6fUWYqlnYy3DzwQpd/JBFxJJR+kL D1UDlZLetepS6WJVPAH7lDK7I9R8H/sQzHlRr1JMnSe9Xq3e4iw8KWSjHltZry5h K+VPBLITo4rmEsDmtqpPtiY= 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