svn commit: r321284 - in head/sys: amd64/include sys

[Bruce Evans]

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 at 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 at 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