From owner-svn-src-head@freebsd.org Sat Jul 22 20:10:55 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A0C48DAC20C; Sat, 22 Jul 2017 20:10:55 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 301D863E73; Sat, 22 Jul 2017 20:10:54 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 3B2EF3C7698; Sun, 23 Jul 2017 05:49:26 +1000 (AEST) Date: Sun, 23 Jul 2017 05:49:26 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov cc: Ryan Libby , Bruce Evans , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, imp@freebsd.org Subject: Re: svn commit: r321284 - in head/sys: amd64/include sys In-Reply-To: <20170721061552.GK1935@kib.kiev.ua> Message-ID: <20170723052316.W1061@besplex.bde.org> References: <201707200647.v6K6l7Hq076554@repo.freebsd.org> <20170720172157.W1152@besplex.bde.org> <20170720103323.GG1935@kib.kiev.ua> <20170721061552.GK1935@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=VbSHBBh9 c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=pGLkceISAAAA:8 a=PO7r1zJSAAAA:8 a=H7MLjYanAAAA:8 a=dw0t6H4-AAAA:8 a=QjaddGlQvknW33FtQ14A:9 a=ZAWH0lS2keSws1BU:21 a=73LSAuf79mZQfIRR:21 a=CjuIK1q_8ugA:10 a=6kGIvZw6iX1k4Y-7sg4_:22 a=bgSlMGY6Cwri7Y-TTqhu:22 a=wVJa4CU9-Z26yuRAZDil:22 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 22 Jul 2017 20:10:55 -0000 On Fri, 21 Jul 2017, Konstantin Belousov wrote: > On Thu, Jul 20, 2017 at 04:02:02PM -0700, Ryan Libby wrote: >> On Thu, Jul 20, 2017 at 3:33 AM, Konstantin Belousov >> wrote: >>> On Thu, Jul 20, 2017 at 02:08:30AM -0700, Ryan Libby wrote: >>>> On Thu, Jul 20, 2017 at 1:01 AM, Bruce Evans wrote: >> [...] >>>>> This bug is not very common. There seem to be no instances of it in >>>>> (only sys/cdefs.h uses __attribute__(()), and it seems to use >>>>> underscores for all the attributes). Grepping sys/include/*.h for >>>>> attribute shows the following bugs: >>>>> >>>>> X amd64/include/efi.h:#define EFIABI_ATTR __attribute__((ms_abi)) >>>>> X i386/include/efi.h:#define EFIABI_ATTR /* __attribute__((ms_abi)) */ /* clang fails with this */ >>>>> X ofed/include/rdma/ib_user_mad.h:typedef unsigned long __attribute__((aligned(4))) packed_ulong; >>>>> X ofed/include/rdma/ib_smi.h:} __attribute__ ((packed)); >>>>> X ofed/include/rdma/ib_mad.h:} __attribute__ ((packed)); >>>>> X ofed/include/rdma/ib_mad.h:} __attribute__ ((packed)); >>>>> >>>>> The commented-out ms_abi was only a style bug. Now it is a larger style >>>>> bug -- it is different and worse than amd64. >>>> >>>> I'm not sure what to do about i386 there (again beyond fixing up the >>>> spelling in the comment). Maybe the unsupported architectures should >>>> just not be declaring EFIABI_ATTR at all? (Thoughts, kib?) >>> >>> I think i386 should be treated exactly same as amd64, i.e. EFIABI_ATTR >>> should be not defined if gcc < 4.4. Or I do not understand the scope >>> of the question. It depends on whether to comment is correct. >> After googling around [1] and a quick check of the spec [2], it now >> seems to me that the i386 comment might just be erroneous. I think the Testing shows that it is still correct. Using this attribute generates the warning that it is ignored. >> right solution for sys/i386/include/efi.h may just be to delete the >> comment and leave that EFIAPI_ATTR macro definition as empty (always, no >> compiler version check) in order to use the native calling convention. >> >> [1] http://wiki.osdev.org/UEFI#Calling_Conventions >> [2] http://www.uefi.org/sites/default/files/resources/UEFI_Spec_2_7.pdf > > At very least, the UEFI Spec requires 16-byte alignment of the stack. > This is not guaranteed by our i386 ABI. This UEFI pessimization is guaranteed to be not done by our i386 ABI. We use -mpreferred-stack-boundary=2 to prevent it for gcc, and this 4-byte alignment is the default for clang. This avoids the pessimization of wasting time and space by padding the stack to a 16-byte boundary on (non-null on 3/4 of function calls). __aligned(16) on local variables is broken by this for gcc-4.2.1. gcc-4.2.1 doesn't understand its own option, and doesn't generate the necessary andl of the stack to align it in functions that need it. gcc by default has the pessimization. clang handles stack alignment correctly, except it doesn't support -mpreferred-stack-boundary to control it. It hard-codes 4-byte alignment on i386, and does the necessary andl of the stack in functions that need it. Since clang on i386 also doesn't support ms_abi to change this, the hard-coding seems to be perfect. Bruce