From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120707 invoked by alias); 5 Dec 2018 13:09:56 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 120689 invoked by uid 89); 5 Dec 2018 13:09:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY autolearn=ham version=3.3.2 spammy=1011, attr-aligned.c, UD:attr-aligned.c, cross-compilers X-HELO: smtp.CeBiTec.Uni-Bielefeld.DE Received: from smtp.CeBiTec.Uni-Bielefeld.DE (HELO smtp.CeBiTec.Uni-Bielefeld.DE) (129.70.160.84) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Dec 2018 13:09:51 +0000 Received: from localhost (localhost.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTP id 674B2EA3; Wed, 5 Dec 2018 14:09:47 +0100 (CET) Received: from smtp.CeBiTec.Uni-Bielefeld.DE ([127.0.0.1]) by localhost (malfoy.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ME5Rs5-DealM; Wed, 5 Dec 2018 14:09:43 +0100 (CET) Received: from lokon.CeBiTec.Uni-Bielefeld.DE (lokon.CeBiTec.Uni-Bielefeld.DE [129.70.161.152]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTPS id CF6D2E9F; Wed, 5 Dec 2018 14:09:43 +0100 (CET) Received: (from ro@localhost) by lokon.CeBiTec.Uni-Bielefeld.DE (8.15.2+Sun/8.15.2/Submit) id wB5D9gUA011865; Wed, 5 Dec 2018 14:09:42 +0100 (MET) From: Rainer Orth To: Martin Sebor Cc: Gcc Patch List , Eric Botcazou Subject: Re: [PATCH] be more permissive about function alignments (PR 88208) References: <19112bc8-83a2-2c35-2841-d95087cd1178@gmail.com> Date: Wed, 05 Dec 2018 13:09:00 -0000 In-Reply-To: <19112bc8-83a2-2c35-2841-d95087cd1178@gmail.com> (Martin Sebor's message of "Tue, 27 Nov 2018 21:32:57 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (usg-unix-v) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00274.txt.bz2 --=-=-= Content-Type: text/plain Content-length: 3207 Hi Martin, > The tests for the new __builtin_has_attribute function have been > failing on a number of targets because of a couple of assumptions > that only hold on some. > > First, they expect that it's safe to apply attribute aligned with > a smaller alignment than the target provides when GCC rejects such > arguments. The tests pass on i86 and elsewhere but fail on > strictly aligned targets like aarch64 or sparc. After some testing > and thinking I don't think this is helpful -- I believe it's better > to instead silently accept attributes that ask for a less restrictive > alignment than the function ultimately ends up with (see * below). > This is what testing shows Clang does on those targets. The attached > patch implements this change. > > Second, the tests assume that the priority forms of the constructor > and destructor attributes are universally supported. That's also > not the case, even though the manual doesn't mention that. To > avoid these failures the attached patch moves the priority forms > of the attribute constructor and destructor tests into its own > file that's compiled only for init_priority targets. > > Finally, I noticed that attribute aligned accepts zero as > an argument even though it's not a power of two as the manual > documents as a precondition (zero is treated the same as > the attribute without an argument). A zero argument is likely > to be a mistake, especially when the zero comes from macro > expansion, that users might want to know about. Clang rejects > a zero with an error but I think a warning is more in line with > established GCC practice, so the patch also implements that. > > Besides x86_64-linux, I tested this change with cross-compilers > for aarch64-linux-elf, powerpc64le-linux, and sparc-solaris2.11. > I added tests for the changed aligned attribute for those targets > To make the gcc.dg/builtin-has-attribute.c test pass with > the cross-compilers I changed from a runtime test into a compile > only one. > > Martin > > PS I'm not happy about duplicating the same test across all those > targets. It would be much nicer to have a single test somewhere > in dg.exp #include a target-specific header with macros describing > the target-specific parameters. why so complicated? Just have a single attr-aligned.c test, restricted to the target cpus it supports via dg directives and with MINALIGN/MAXALIGN definitions controlled by appropriate target macros? Besides, the new gcc.target/sparc/attr-aligned.c test currently FAILs on 64-bit sparc: FAIL: gcc.target/sparc/attr-aligned.c (test for excess errors) Excess errors: /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/attr-aligned.c:29:1: error: static assertion failed: "alignof (f_) == MAXALIGN" alignof (f_) is 16 for sparcv9. The following patch fixes this, tested on sparc-sun-solaris2.11 with -m32 and -m64. Ok for mainline? Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University 2018-12-04 Rainer Orth PR testsuite/88208 * gcc.target/sparc/attr-aligned.c (MAXALIGN) [__sparcv9 || __arch64__]: Define. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=testsuite-sparc-attr-aligned.patch Content-length: 645 # HG changeset patch # Parent bd6c4f1f5f5ef5a6d912a66d6f3b40f21c8dd411 Provide SPARCv9 MAXALIGN in gcc.target/sparc/attr-aligned.c diff --git a/gcc/testsuite/gcc.target/sparc/attr-aligned.c b/gcc/testsuite/gcc.target/sparc/attr-aligned.c --- a/gcc/testsuite/gcc.target/sparc/attr-aligned.c +++ b/gcc/testsuite/gcc.target/sparc/attr-aligned.c @@ -10,7 +10,11 @@ #define HAS_ALIGN(f, n) __builtin_has_attribute (f, __aligned__ (n)) #define MINALIGN(N) ((N) < 4 ? 4 : (N)) +#if defined(__sparcv9) || defined(__arch64__) +#define MAXALIGN 16 +#else #define MAXALIGN 8 +#endif /* No alignment specified. */ void f (void) { } --=-=-=--