From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76606 invoked by alias); 8 Oct 2015 14:01:59 -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 76594 invoked by uid 89); 8 Oct 2015 14:01:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,KAM_ASCII_DIVIDERS,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mx07-00178001.pphosted.com Received: from mx07-00178001.pphosted.com (HELO mx07-00178001.pphosted.com) (62.209.51.94) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 08 Oct 2015 14:01:57 +0000 Received: from pps.filterd (m0046668.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.14.5/8.14.5) with SMTP id t98E0Pqv023438; Thu, 8 Oct 2015 16:01:50 +0200 Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 1xdh6rtqw9-1 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Thu, 08 Oct 2015 16:01:47 +0200 Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id AB11F41; Thu, 8 Oct 2015 14:01:22 +0000 (GMT) Received: from Webmail-eu.st.com (safex1hubcas4.st.com [10.75.90.69]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id A34D9580F; Thu, 8 Oct 2015 14:01:42 +0000 (GMT) Received: from [164.129.122.197] (164.129.122.197) by webmail-eu.st.com (10.75.90.13) with Microsoft SMTP Server (TLS) id 8.3.389.2; Thu, 8 Oct 2015 16:01:42 +0200 Subject: Re: [PATCH ARM]: PR67745: Fix function alignment after __attribute__ 2/2 To: Bernd Schmidt , "law@redhat.com" References: <560A90F2.5010708@st.com> <560C31CD.7060009@redhat.com> <560CDCD7.9080108@st.com> <560D5B36.2020600@st.com> <5614C412.5080400@st.com> <5614F17B.5060402@redhat.com> <5614F7D0.8010409@st.com> <56155841.6000404@redhat.com> CC: "gcc-patches@gcc.gnu.org" , "Ramana.Radhakrishnan@arm.com" , "kyrylo.tkachov@arm.com" , "richard.earnshaw@arm.com" From: Christian Bruel X-No-Archive: yes Message-ID: <56167745.1080409@st.com> Date: Thu, 08 Oct 2015 14:01:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <56155841.6000404@redhat.com> Content-Type: multipart/mixed; boundary="------------010901040203050906010104" X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-10-08_11:2015-10-08,2015-10-08,1970-01-01 signatures=0 X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg00839.txt.bz2 --------------010901040203050906010104 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1784 On 10/07/2015 07:37 PM, Bernd Schmidt wrote: > On 10/07/2015 12:45 PM, Christian Bruel wrote: >> >> >> On 10/07/2015 12:18 PM, Bernd Schmidt wrote: >>> On 10/07/2015 09:04 AM, Christian Bruel wrote: >>>> + /* Similarly, relayout function's alignment if not forced. */ >>>> + if (!DECL_USER_ALIGN (fndecl) >>>> + && (TREE_CODE (fntype) != METHOD_TYPE >>>> + || TARGET_PTRMEMFUNC_VBIT_LOCATION != >>>> ptrmemfunc_vbit_in_pfn)) >>>> + DECL_ALIGN (fndecl) = FUNCTION_BOUNDARY; >>>> } >>> >>> That's a very odd-looking condition. Why the vbit location test? >> >> This is for member functions to make sure that the lsb address is >> reserved for the virtual function bit. >> >> see cp/decl.c: >> >> /* If pointers to member functions use the least significant bit to >> indicate whether a function is virtual, ensure a pointer >> to this function will have that bit clear. */ >> if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn >> && TREE_CODE (type) == METHOD_TYPE >> && DECL_ALIGN (decl) < 2 * BITS_PER_UNIT) >> DECL_ALIGN (decl) = 2 * BITS_PER_UNIT; >> >> This happens for instance on i386 that aligns member functions on 2 >> bytes, bigger than the one byte required boundary. So we cannot >> re-layout bellow that. > > Hmm, at least that would need to be spelled out in a comment, but I > think this would better be abstracted as > > #define MINIMUM_FUNCTION_BOUNDARY(FN) \ > .... > in a header file, and used in both places. > OK, Similar pattern occurs at many other places, that changed also in the attached proposal. Not fully tested (in particular the java part) and no ChangeLog. Just to make sure that we agree on the interface first. Thanks Christian > > Bernd > --------------010901040203050906010104 Content-Type: text/x-patch; name="align5.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="align5.patch" Content-length: 3010 Index: gcc/builtins.c =================================================================== 304a305,306 > tree fntype = exp ? TREE_TYPE (exp) : NULL_TREE; > 309,310c311,312 < if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn) < align = 2 * BITS_PER_UNIT; --- > align = MAX (minimum_function_boundary (exp), align); > Index: gcc/cp/decl.c =================================================================== 7855,7858c7855 < if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn < && TREE_CODE (type) == METHOD_TYPE < && DECL_ALIGN (decl) < 2 * BITS_PER_UNIT) < DECL_ALIGN (decl) = 2 * BITS_PER_UNIT; --- > DECL_ALIGN (decl) = MAX (minimum_function_boundary (decl), DECL_ALIGN (decl)); Index: gcc/cp/lambda.c =================================================================== 1010,1012c1010,1011 < if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn < && DECL_ALIGN (fn) < 2 * BITS_PER_UNIT) < DECL_ALIGN (fn) = 2 * BITS_PER_UNIT; --- > extern int need_vptr_align (void); > DECL_ALIGN (fn) = MAX (minimum_function_boundary (fn), DECL_ALIGN (fn)); 1045,1047c1044 < if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn < && DECL_ALIGN (fn) < 2 * BITS_PER_UNIT) < DECL_ALIGN (fn) = 2 * BITS_PER_UNIT; --- > DECL_ALIGN (fn) = MAX (minimum_function_boundary (fn), DECL_ALIGN (fn)); Index: gcc/cp/method.c =================================================================== 1852c1852 < --- > 1856,1858c1856 < if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn < && DECL_ALIGN (fn) < 2 * BITS_PER_UNIT) < DECL_ALIGN (fn) = 2 * BITS_PER_UNIT; --- > DECL_ALIGN (fn) = MAX (minimum_function_boundary (fn), DECL_ALIGN (fn)); Index: gcc/function.c =================================================================== 4791a4792,4806 > unsigned int > minimum_function_boundary (tree fndecl) > { > gcc_assert (fndecl); > > tree fntype = TREE_TYPE (fndecl); > > if (TREE_CODE (fntype) == METHOD_TYPE > && TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn > && FUNCTION_BOUNDARY < 2 * BITS_PER_UNIT) > return 2 * BITS_PER_UNIT; > > return FUNCTION_BOUNDARY; > } > 4842a4858,4861 > > /* Similarly, relayout function's alignment if not forced. */ > if (!DECL_USER_ALIGN (fndecl)) > DECL_ALIGN (fndecl) = minimum_function_boundary (fndecl); Index: gcc/function.h =================================================================== 601c601 < --- > extern unsigned int minimum_function_boundary (tree); Index: gcc/java/class.c =================================================================== 785,788c785,786 < if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn < && !(access_flags & ACC_STATIC) < && DECL_ALIGN (fndecl) < 2 * BITS_PER_UNIT) < DECL_ALIGN (fndecl) = 2 * BITS_PER_UNIT; --- > if !(access_flags & ACC_STATIC) > DECL_ALIGN (fndecl) = MAX (minimum_function_boundary (decl), DECL_ALIGN (fndecl)); --------------010901040203050906010104--