From patchwork Tue Apr 25 10:00:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 754676 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 3wBzJ44bKzz9s7q for ; Tue, 25 Apr 2017 20:01:23 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="Q2a7XcNX"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=XT6a0/ca18msb4f4sd08dkshMA34l V/F63SLPaI5QXPeFjs7xsTdOWPSuP8Jqe84xJF5SdPyqrmtmxgg4kHzFhgO71p2H SQqnTSgoYNasHF+3JliGzKLso+DYGvKARaPiC181BewyPqi4+EOr4/SGAY1JLKAT 2+kvnueBZEFCiE= 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:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; s=default; bh=hKu9CqRslWlY2N+8HnGYX0OHFT8=; b=Q2a 7XcNXjZmQSjZ64aYhHVXk+1aTxkzCELq/Ei+gLVdufaYmERr0467wDsgsgAPKaSg r44FjyKnu4xL0A3RPWTdLgcFtWezKA2rL0AXiocIxjricJBqeprUYoz11HRFwJv+ f6h1v521uWJwusQVVQqA9pBen01H04xbUF19rtGI= Received: (qmail 80678 invoked by alias); 25 Apr 2017 10:01:06 -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 80098 invoked by uid 89); 25 Apr 2017 10:00:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=bi, greatest X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Apr 2017 10:00:30 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 64810C008614; Tue, 25 Apr 2017 10:00:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 64810C008614 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jakub@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 64810C008614 Received: from tucnak.zalov.cz (ovpn-116-29.ams2.redhat.com [10.36.116.29]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 79CF87FEAB; Tue, 25 Apr 2017 10:00:29 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v3PA0QNT001220; Tue, 25 Apr 2017 12:00:27 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v3PA0Pco001219; Tue, 25 Apr 2017 12:00:25 +0200 Date: Tue, 25 Apr 2017 12:00:25 +0200 From: Jakub Jelinek To: Richard Earnshaw , Ramana Radhakrishnan , Nick Clifton Cc: gcc-patches@gcc.gnu.org Subject: [ARM ABI PATCH] Change ARM ABI to match AAPCS, provide -Wpsabi notes (PR target/77728) Message-ID: <20170425100025.GU1809@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes Hi! As mentioned in the PR, r225465 aka PR65956 changed the ABI on ARM to match updated AAPCS, but the change had a bug - for structures it considered DECL_ALIGN of any TYPE_FIELDS, rather than just actual data components (AAPCS says members, for C++ and Itanium C++ ABI that is likely direct non-static data members and non-virtual base classes; that means it also considered alignment of static data members (at least this was consistent ABI difference), or DECL_ALIGN of TYPE_DECLs (which is bigger problem, because that alignment is pretty randomish, it has different value in types in templates depending on whether they have been instantiated earlier or not)). The following patch fixes the ABI bug and adds -Wpsabi diagnostics (inform rather than warning, so it doesn't break with -Werror and matches i386.c -Wpsabi notes where there is no bug on the compiled code side). Earlier version of the patch has been bootstrapped/regtested on armv7hl-linux-gnueabi, but there have been various changes since then. Ok for trunk/7.1 if it passes testing? 2017-04-25 Ramana Radhakrishnan Jakub Jelinek PR target/77728 * config/arm/arm.c: Include gimple.h. (aapcs_layout_arg): Emit -Wpsabi note if arm_needs_doubleword_align returns negative, increment ncrn only if it returned positive. (arm_needs_doubleword_align): Return int instead of bool, ignore DECL_ALIGN of non-FIELD_DECL TYPE_FIELDS chain members, but if there is any such non-FIELD_DECL > PARM_BOUNDARY aligned decl, return -1 instead of false. (arm_function_arg): Emit -Wpsabi note if arm_needs_doubleword_align returns negative, increment nregs only if it returned positive. (arm_setup_incoming_varargs): Likewise. (arm_function_arg_boundary): Emit -Wpsabi note if arm_needs_doubleword_align returns negative, return DOUBLEWORD_ALIGNMENT only if it returned positive. testsuite/ * g++.dg/abi/pr77728-1.C: New test. Jakub --- gcc/config/arm/arm.c.jj 2017-04-25 09:20:49.740670794 +0200 +++ gcc/config/arm/arm.c 2017-04-25 11:07:11.003121070 +0200 @@ -64,6 +64,7 @@ #include "rtl-iter.h" #include "optabs-libfuncs.h" #include "gimplify.h" +#include "gimple.h" /* This file should be included last. */ #include "target-def.h" @@ -81,7 +82,7 @@ struct four_ints /* Forward function declarations. */ static bool arm_const_not_ok_for_debug_p (rtx); -static bool arm_needs_doubleword_align (machine_mode, const_tree); +static int arm_needs_doubleword_align (machine_mode, const_tree); static int arm_compute_static_chain_stack_bytes (void); static arm_stack_offsets *arm_get_frame_offsets (void); static void arm_add_gc_roots (void); @@ -6349,8 +6350,20 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, /* C3 - For double-word aligned arguments, round the NCRN up to the next even number. */ ncrn = pcum->aapcs_ncrn; - if ((ncrn & 1) && arm_needs_doubleword_align (mode, type)) - ncrn++; + if (ncrn & 1) + { + int res = arm_needs_doubleword_align (mode, type); + /* Only warn during RTL expansion of call stmts, otherwise we would + warn e.g. during gimplification even on functions that will be + always inlined, and we'd warn multiple times. Don't warn when + called in expand_function_start either, as we warn instead in + arm_function_arg_boundary in that case. */ + if (res < 0 && warn_psabi && currently_expanding_gimple_stmt) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 7.1", type); + else if (res > 0) + ncrn++; + } nregs = ARM_NUM_REGS2(mode, type); @@ -6455,12 +6468,16 @@ arm_init_cumulative_args (CUMULATIVE_ARG } } -/* Return true if mode/type need doubleword alignment. */ -static bool +/* Return 1 if double word alignment is required for argument passing. + Return -1 if double word alignment used to be required for argument + passing before PR77728 ABI fix, but is not required anymore. + Return 0 if double word alignment is not required and wasn't requried + before either. */ +static int arm_needs_doubleword_align (machine_mode mode, const_tree type) { if (!type) - return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode); + return GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY; /* Scalar and vector types: Use natural alignment, i.e. of base type. */ if (!AGGREGATE_TYPE_P (type)) @@ -6470,12 +6487,21 @@ arm_needs_doubleword_align (machine_mode if (TREE_CODE (type) == ARRAY_TYPE) return TYPE_ALIGN (TREE_TYPE (type)) > PARM_BOUNDARY; + int ret = 0; /* Record/aggregate types: Use greatest member alignment of any member. */ for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) if (DECL_ALIGN (field) > PARM_BOUNDARY) - return true; + { + if (TREE_CODE (field) == FIELD_DECL) + return 1; + else + /* Before PR77728 fix, we were incorrectly considering also + other aggregate fields, like VAR_DECLs, TYPE_DECLs etc. + Make sure we can warn about that with -Wpsabi. */ + ret = -1; + } - return false; + return ret; } @@ -6532,10 +6558,15 @@ arm_function_arg (cumulative_args_t pcum } /* Put doubleword aligned quantities in even register pairs. */ - if (pcum->nregs & 1 - && ARM_DOUBLEWORD_ALIGN - && arm_needs_doubleword_align (mode, type)) - pcum->nregs++; + if ((pcum->nregs & 1) && ARM_DOUBLEWORD_ALIGN) + { + int res = arm_needs_doubleword_align (mode, type); + if (res < 0 && warn_psabi) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 7.1", type); + else if (res > 0) + pcum->nregs++; + } /* Only allow splitting an arg between regs and memory if all preceding args were allocated to regs. For args passed by reference we only count @@ -6554,9 +6585,15 @@ arm_function_arg (cumulative_args_t pcum static unsigned int arm_function_arg_boundary (machine_mode mode, const_tree type) { - return (ARM_DOUBLEWORD_ALIGN && arm_needs_doubleword_align (mode, type) - ? DOUBLEWORD_ALIGNMENT - : PARM_BOUNDARY); + if (!ARM_DOUBLEWORD_ALIGN) + return PARM_BOUNDARY; + + int res = arm_needs_doubleword_align (mode, type); + if (res < 0 && warn_psabi) + inform (input_location, "parameter passing for argument of type %qT " + "changed in GCC 7.1", type); + + return res > 0 ? DOUBLEWORD_ALIGNMENT : PARM_BOUNDARY; } static int @@ -26516,8 +26553,15 @@ arm_setup_incoming_varargs (cumulative_a if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) { nregs = pcum->aapcs_ncrn; - if ((nregs & 1) && arm_needs_doubleword_align (mode, type)) - nregs++; + if (nregs & 1) + { + int res = arm_needs_doubleword_align (mode, type); + if (res < 0 && warn_psabi) + inform (input_location, "parameter passing for argument of " + "type %qT changed in GCC 7.1", type); + else if (res > 0) + nregs++; + } } else nregs = pcum->nregs; --- gcc/testsuite/g++.dg/abi/pr77728-1.C.jj 2017-04-25 09:30:39.262786365 +0200 +++ gcc/testsuite/g++.dg/abi/pr77728-1.C 2017-04-25 10:14:59.134254239 +0200 @@ -0,0 +1,171 @@ +// { dg-do compile { target arm_eabi } } +// { dg-options "-Wpsabi" } + +#include + +template +struct A { double p; }; + +A<0> v; + +template +struct B +{ + typedef A T; + int i, j; +}; + +struct C : public B<0> {}; +struct D {}; +struct E : public D, C {}; +struct F : public B<1> {}; +struct G : public F { static double y; }; +struct H : public G {}; +struct I : public D { long long z; }; +struct J : public D { static double z; int i, j; }; + +template +struct K : public D { typedef A T; int i, j; }; + +struct L { static double h; int i, j; }; + +int +fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } +{ + return a + b.i; +} + +int +fn2 (int a, B<1> b) +{ + return a + b.i; +} + +int +fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } +{ + return a + b.i; +} + +int +fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...) +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn5 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<1> n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn6 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, C n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn7 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, E n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn8 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, H n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn9 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, I n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...) +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...) +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn12 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<2> n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +void +test () +{ + static B<0> b0; + static B<1> b1; + static L l; + static C c; + static E e; + static H h; + static I i; + static J j; + static K<0> k0; + static K<2> k2; + fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } + fn2 (1, b1); + fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } + fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4); + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } + fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4); + fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4); + fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4); + fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4); + fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4); + fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4); + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } + fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4); + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } + fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4); +}