Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Jul 2017 19:19:36 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Ryan Libby <rlibby@gmail.com>
Cc:        Bruce Evans <brde@optusnet.com.au>, kib@freebsd.org,  Ryan Libby <rlibby@freebsd.org>, src-committers@freebsd.org,  svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r321284 - in head/sys: amd64/include sys
Message-ID:  <20170720190955.U1634@besplex.bde.org>
In-Reply-To: <CAHgpiFxW7JzurYeYuN5WaN0Z%2BjcpPLSjtHL34iCgmdJUz7bSyg@mail.gmail.com>
References:  <201707200647.v6K6l7Hq076554@repo.freebsd.org> <20170720172157.W1152@besplex.bde.org> <CAHgpiFxW7JzurYeYuN5WaN0Z%2BjcpPLSjtHL34iCgmdJUz7bSyg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 20 Jul 2017, Ryan Libby wrote:

> On Thu, Jul 20, 2017 at 1:01 AM, Bruce Evans <brde@optusnet.com.au> wrote:
>> On Thu, 20 Jul 2017, Ryan Libby wrote:
>>> Modified: head/sys/amd64/include/efi.h
>>>
>>> ==============================================================================
>>> --- head/sys/amd64/include/efi.h        Thu Jul 20 05:43:48 2017
>>> (r321283)
>>> +++ head/sys/amd64/include/efi.h        Thu Jul 20 06:47:06 2017
>>> (r321284)
>>> @@ -36,8 +36,14 @@
>>>  * XXX: from gcc 6.2 manual:
>>>  * Note, the ms_abi attribute for Microsoft Windows 64-bit targets
>>>  * currently requires the -maccumulate-outgoing-args option.
>>> + *
>>> + * Avoid EFIABI_ATTR declarations for compilers that don't support it.
>>> + * GCC support began in version 4.4.
>>>  */
>>> +#if defined(__clang__) || defined(__GNUC__) && \
>>> +    (__GNUC__ > 4 || __GNUC__ == 4 && __GNUC_MINOR__ >= 4)
>>> #define EFIABI_ATTR     __attribute__((ms_abi))
>>
>>
>> This is still broken.  ms_abi is in the application namespace.  Thus
>> my hack of defining it to nothing is valid, and so is defining it to
>> 'syntax error', but the latter shows the brokenness of EFIABI_ATTR.
>
> To be clear, you're referring to how ms_abi is spelled here without
> underscores?  I can prepare a follow-up to spell it as
> __attribute__((__ms_abi__))

Yes.
>> ...
>> 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?)

Maybe it can go in x86 with a different ifdef for clang.  Or if it just
doesn't work on i386, don't define or use EFIABI_ATTR.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170720190955.U1634>