Date: Sun, 23 Jul 2017 05:49:26 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Ryan Libby <rlibby@gmail.com>, Bruce Evans <brde@optusnet.com.au>, 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 Message-ID: <20170723052316.W1061@besplex.bde.org> In-Reply-To: <20170721061552.GK1935@kib.kiev.ua> References: <201707200647.v6K6l7Hq076554@repo.freebsd.org> <20170720172157.W1152@besplex.bde.org> <CAHgpiFxW7JzurYeYuN5WaN0Z%2BjcpPLSjtHL34iCgmdJUz7bSyg@mail.gmail.com> <20170720103323.GG1935@kib.kiev.ua> <CAHgpiFxZxdW30P3VDqe%2BcTihEf92c3AaJy7e9cJ1uGUdWGFu6Q@mail.gmail.com> <20170721061552.GK1935@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >> <kostikbel@gmail.com> 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 <brde@optusnet.com.au> wrote: >> [...] >>>>> This bug is not very common. There seem to be no instances of it in >>>>> <sys> (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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170723052316.W1061>