From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2a00:1450:400c:c09::241; helo=mail-wm0-x241.google.com; envelope-from=pete@akeo.ie; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3B971221523A1 for ; Mon, 4 Dec 2017 09:38:03 -0800 (PST) Received: by mail-wm0-x241.google.com with SMTP id b76so15869919wmg.1 for ; Mon, 04 Dec 2017 09:42:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=5KDo2w6MpTFMC1zYpntcj46D+NzZSJBLg8s3ry4Ydl0=; b=RKhdzfWSDtjMqyzOKMvFQg0HHLqsPzTmmgS1oE3Dy9B9P/3xHuGEGJOnu1xvR9EOf5 3U8i57qDGGG6xeN3M40suJLVytJWRRC9gPS7TOTjJ6J0R7IFZQmGDEFBI5GxVXChtjS9 w42qV53XjHN3xSfujK+3CHHcR8xbQYtyauIzuTSlGQvq0D6L7HhgE/poFYEuwG45TEvR 7H+6W4IHLwUbU91iXoM9xTa8uGeECFZwDO7szR4JEM4HW5x+WqlXp/vgJ5u0OIrUJvLA +pEmOmQ9uNMd3BM38o3XUQBU/pKTFCqe6sgqX8ql8ggqlSF6M9bGHIuDkeUVGCBg3TYt Eu6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=5KDo2w6MpTFMC1zYpntcj46D+NzZSJBLg8s3ry4Ydl0=; b=nPRW5ckn4NCGP8oGklaOUZmkUVkO9uGHFIG2PywYvbw/whuLiyzQFD7i2tj3luUGWC YuyRQpFOv/hlqg6EbsY0+5DzMf9fWo+DPEv0DtHXc/QhsvnY2vTsjAHaDP3IzY92ZPBI JtniDHElSHnkmSbC9xD2BlbW5b3Y1jUHA4BDG7OFP9wPThbcVqK1e0O0bW7vFRRYK7Nv qNHgJR7kSAlZP690/wELPDiHyfFUihoy5IayW2JRDv5lLOducemmpzIFO5U2D80WEdqM AQKYUU1VEpP51DpSlI01D/362o7B2bWs2iNaeTJ1qm2tATJNl6xf/Uu7wBrKczp6I4qI pnEw== X-Gm-Message-State: AJaThX430Yv+6Qv2gvcmBxnMJtPnCDHwkVmE3wxb18RGCfx/BJWABG2m OXBGZIBDOzV/tkn2DbYHwypLBws14JI= X-Google-Smtp-Source: AGs4zMYSSI2nYq87SEs1awJYkR221lEec3IzSX0sc8zBw4Deq67s2B9eoaXuwpUDEthhKwPk9+YvBA== X-Received: by 10.80.170.24 with SMTP id o24mr32533189edc.40.1512409351810; Mon, 04 Dec 2017 09:42:31 -0800 (PST) Received: from [10.0.0.101] ([84.203.42.156]) by smtp.googlemail.com with ESMTPSA id l9sm6807627edi.58.2017.12.04.09.42.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Dec 2017 09:42:31 -0800 (PST) To: "edk2-devel@lists.01.org" , "Gao, Liming" References: <20171204131205.11304-1-pete@akeo.ie> <20171204131205.11304-2-pete@akeo.ie> <4A89E2EF3DFEDB4C8BFDE51014F606A14E189CE6@SHSMSX104.ccr.corp.intel.com> From: Pete Batard Message-ID: <132bb69c-5a04-a20b-0d4c-71e1eeeba53f@akeo.ie> Date: Mon, 4 Dec 2017 17:42:30 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E189CE6@SHSMSX104.ccr.corp.intel.com> Subject: Re: [PATCH 1/6] MdePkg: Disable some Level 4 warnings for VS2017/ARM X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Dec 2017 17:38:03 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Hi Liming, Thanks for reviewing these patches. As explained in the git comment, I chose to disable warnings that I saw being generated more than 5 times during QEMU compilation, as I suspect we may inherit things from gcc that we may not want to spend too much time changing, just to keep MSVC happy. Considering that we are introducing a new target for MS users, it may be worth being a bit more lenient with warnings for the time being, so that people don't have to spend too much time addressing issues that they wouldn't get with other compilers (gcc, Clang), if they decide to try VS2017/ARM, as it may drive them off otherwise. Especially, if we try to align ARM warnings with IA32/X64, then we're going to have to address at least 78 instances of C4244 warnings ("conversion from 'type1' to 'type2', possible loss of data") in our ARM codebase eventually, which is probably not in our best interest. For reference, here is the list of Level 4 compiler warnings I got, during QEMU compilation, along with the number of times they occurred: C4018: 7 C4101: 6 C4132: 1 C4146: 3 C4244: 78 C4366: 1 C4389: 1 C4456: 10 C4701: 50 C4703: 48 This being said, I suspect most of the warnings above may not be generated when compiling regular UEFI ARM applications or drivers, as I only started to see those when trying to compile QEMU. So, if you think that's preferable, we can proceed as you suggest, and align ARM disabled warnings with IA32 and X64. Then we can wait and see if users of VS2017/ARM start to complain that more warnings need to be disabled... Regards, /Pete On 2017.12.04 15:36, Gao, Liming wrote: > Pete: > I suggest to align the disabled warning list to IA32/X64 arch. I find someone are not listed in IA32/X64 arch. Could you give the more detail on why disable them by default? > > Thanks > Liming >> -----Original Message----- >> From: Pete Batard [mailto:pete@akeo.ie] >> Sent: Monday, December 4, 2017 9:12 PM >> To: edk2-devel@lists.01.org >> Cc: Gao, Liming >> Subject: [PATCH 1/6] MdePkg: Disable some Level 4 warnings for VS2017/ARM >> >> Warnings 4018, 4100, 4101, 4127, 4214, 4244, 4456, 4701 and 4703 are >> disabled as they were found to occur more than 5 times during QEMU >> firmware compilation. >> >> Also create a dummy macro for PRESERVE8, as this is not supported by >> the Microsoft ARM assembler. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Pete Batard >> --- >> MdePkg/Include/Arm/ProcessorBind.h | 96 +++++++++++++++----- >> 1 file changed, 75 insertions(+), 21 deletions(-) >> >> diff --git a/MdePkg/Include/Arm/ProcessorBind.h b/MdePkg/Include/Arm/ProcessorBind.h >> index dde1fd1152ba..00de80bafe0a 100644 >> --- a/MdePkg/Include/Arm/ProcessorBind.h >> +++ b/MdePkg/Include/Arm/ProcessorBind.h >> @@ -1,15 +1,15 @@ >> /** @file >> Processor or Compiler specific defines and types for ARM. >> >> - Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
>> + Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
>> Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>> - This program and the accompanying materials >> - are licensed and made available under the terms and conditions of the BSD License >> - which accompanies this distribution. The full text of the license may be found at >> - http://opensource.org/licenses/bsd-license.php >> + This program and the accompanying materials >> + are licensed and made available under the terms and conditions of the BSD License >> + which accompanies this distribution. The full text of the license may be found at >> + http://opensource.org/licenses/bsd-license.php >> >> - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> >> **/ >> >> @@ -28,14 +28,63 @@ >> #pragma pack() >> #endif >> >> +#if defined(_MSC_EXTENSIONS) >> + >> // >> -// RVCT does not support the __builtin_unreachable() macro >> +// Disable 'signed/unsigned mismatch' warnings. >> // >> -#ifdef __ARMCC_VERSION >> +#pragma warning ( disable : 4018 ) >> + >> +// >> +// Disable 'unreferenced formal parameter' warnings. >> +// >> +#pragma warning ( disable : 4100 ) >> + >> +// >> +// Disable 'unreferenced local variable' warnings. >> +// >> +#pragma warning ( disable : 4101 ) >> + >> +// >> +// Suppress warnings for ASSERT(FALSE) or while(TRUE). >> +// >> +#pragma warning ( disable : 4127 ) >> + >> +// >> +// Disable bitfield type check warnings. >> +// >> +#pragma warning ( disable : 4214 ) >> + >> +// >> +// Disable 'conversion from X to Y, possible loss of data' warnings >> +// >> +#pragma warning ( disable : 4244 ) >> + >> +// >> +// Disable 'declaration of X hides previous local declaration' warnings >> +// >> +#pragma warning ( disable : 4456 ) >> + >> +// >> +// Disable 'potentially uninitialized local variable X used' warnings >> +// >> +#pragma warning ( disable : 4701 ) >> + >> +// >> +// Disable 'potentially uninitialized local pointer variable X used' warnings >> +// >> +#pragma warning ( disable : 4703 ) >> + >> +#endif >> + >> +// >> +// RVCT and MSFT don't support the __builtin_unreachable() macro >> +// >> +#if defined(__ARMCC_VERSION) || defined(_MSC_EXTENSIONS) >> #define UNREACHABLE() >> #endif >> >> -#if _MSC_EXTENSIONS >> +#if defined(_MSC_EXTENSIONS) >> // >> // use Microsoft* C compiler dependent integer width types >> // >> @@ -52,7 +101,7 @@ >> typedef signed char INT8; >> #else >> // >> - // Assume standard ARM alignment. >> + // Assume standard ARM alignment. >> // Need to check portability of long long >> // >> typedef unsigned long long UINT64; >> @@ -121,7 +170,7 @@ typedef INT32 INTN; >> // use the correct C calling convention. All protocol member functions and >> // EFI intrinsics are required to modify their member functions with EFIAPI. >> // >> -#define EFIAPI >> +#define EFIAPI >> >> // When compiling with Clang, we still use GNU as for the assembler, so we still >> // need to define the GCC_ASM* macros. >> @@ -142,34 +191,39 @@ typedef INT32 INTN; >> >> #define GCC_ASM_EXPORT(func__) \ >> .global _CONCATENATE (__USER_LABEL_PREFIX__, func__) ;\ >> - .type ASM_PFX(func__), %function >> + .type ASM_PFX(func__), %function >> >> #define GCC_ASM_IMPORT(func__) \ >> .extern _CONCATENATE (__USER_LABEL_PREFIX__, func__) >> - >> + >> #else >> // >> - // .type not supported by Apple Xcode tools >> + // .type not supported by Apple Xcode tools >> // >> - #define INTERWORK_FUNC(func__) >> + #define INTERWORK_FUNC(func__) >> >> #define GCC_ASM_EXPORT(func__) \ >> .globl _CONCATENATE (__USER_LABEL_PREFIX__, func__) \ >> - >> - #define GCC_ASM_IMPORT(name) >> + >> + #define GCC_ASM_IMPORT(name) >> >> #endif >> +#elif defined(_MSC_EXTENSIONS) >> + // >> + // PRESERVE8 is not supported by the MSFT assembler. >> + // >> + #define PRESERVE8 >> #endif >> >> /** >> Return the pointer to the first instruction of a function given a function pointer. >> - On ARM CPU architectures, these two pointer values are the same, >> + On ARM CPU architectures, these two pointer values are the same, >> so the implementation of this macro is very simple. >> - >> + >> @param FunctionPointer A pointer to a function. >> >> @return The pointer to the first instruction of a function given a function pointer. >> - >> + >> **/ >> #define FUNCTION_ENTRY_POINT(FunctionPointer) (VOID *)(UINTN)(FunctionPointer) >> >> -- >> 2.9.3.windows.2 >