From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 8/8] x86/efi: Generate uefi_call_wrapper() when compiling with clang Date: Wed, 10 Feb 2016 19:11:18 +0000 Message-ID: <56BB8B56.4060703@citrix.com> References: <1455048108-5045-1-git-send-email-andrew.cooper3@citrix.com> <1455048108-5045-9-git-send-email-andrew.cooper3@citrix.com> <56BB49CA02000078000D090C@prv-mh.provo.novell.com> <56BB3DEF.9040801@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56BB3DEF.9040801@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Xen-devel List-Id: xen-devel@lists.xenproject.org On 10/02/16 13:41, Andrew Cooper wrote: > On 10/02/16 13:31, Jan Beulich wrote: >>>>> On 09.02.16 at 21:01, wrote: >>> Signed-off-by: Andrew Cooper >>> --- >>> CC: Jan Beulich >>> >>> What is the GCC version check supposed to be achieving here? GCC has >>> supported this syntax since 3.0 >> This is best answered by looking at where we've got these headers >> from - the gnu-efi package. It has ... >> >>> --- a/xen/include/asm-x86/x86_64/efibind.h >>> +++ b/xen/include/asm-x86/x86_64/efibind.h >>> @@ -274,7 +274,7 @@ typedef uint64_t UINTN; >>> #endif >>> #endif >>> >>> -#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4) >>> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4) || __clang__ >>> #define uefi_call_wrapper(func, va_num, ...) func(__VA_ARGS__) >>> #else >>> /* for x86_64, EFI_FUNCTION_WRAPPER must be defined */ >> /* for x86_64, EFI_FUNCTION_WRAPPER must be defined */ >> #if defined(HAVE_USE_MS_ABI) >> #define uefi_call_wrapper(func, va_num, ...) func(__VA_ARGS__) >> #else >> UINTN uefi_call_wrapper(void *func, unsigned long va_num, ...); >> #endif >> #define EFI_FUNCTION __attribute__((ms_abi)) >> >> I think this makes clear that the needed feature here is the >> attribute, which certainly wasn't available in older gcc. >> >> With that the question is whether the Clang case won't also need >> a version check, since I can't imagine them having supported this >> prior to gcc introducing it. > Clang has an substantially more sane way than GCC of checking for > individual features. I will introduce and use the > __has_{attribute,feature}() infrastructure to tests like this. > > Experimentally, Clang 3.5 does have ms_abi support Looking at it further, this entire block is unsed. Nothing in tree refers to uefi_call_wrapper() or EFI_FUNCTION_WRAPPER other than this small bit; all declarations use EFIABI directly. i.e. this: diff --git a/xen/include/asm-x86/x86_64/efibind.h b/xen/include/asm-x86/x86_64/efibind.h index af5f424..b013db1 100644 --- a/xen/include/asm-x86/x86_64/efibind.h +++ b/xen/include/asm-x86/x86_64/efibind.h @@ -274,17 +274,6 @@ typedef uint64_t UINTN; #endif #endif -#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4) -#define uefi_call_wrapper(func, va_num, ...) func(__VA_ARGS__) -#else -/* for x86_64, EFI_FUNCTION_WRAPPER must be defined */ -#ifdef EFI_FUNCTION_WRAPPER -UINTN uefi_call_wrapper(void *func, unsigned long va_num, ...); -#else -#error "EFI_FUNCTION_WRAPPER must be defined for x86_64 architecture" -#endif -#endif - #ifdef _MSC_EXTENSIONS #pragma warning ( disable : 4731 ) // Suppress warnings about modification of EBP #endif works correctly for GCC and clang. Would dropping this completely be acceptable? ~Andrew