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::22d; helo=mail-wm0-x22d.google.com; envelope-from=pete@akeo.ie; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) (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 9055C2205B926 for ; Thu, 11 Jan 2018 08:26:36 -0800 (PST) Received: by mail-wm0-x22d.google.com with SMTP id i11so6538642wmf.4 for ; Thu, 11 Jan 2018 08:31:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=FHEbIybzmAfdL5VadI//MSMgY7LNjIhG4DgNBqZBDHc=; b=dt+IEp9Ftq06oBGjWaX+94Aj5DFZFAY6c0V0rs9T1NUgKVlRy0EI55Np9RJhkP46mN N/f2lp6h9zXQ1XjlwuTlUX+RTIhLStj6/HfXiLbkgVFDimSVlrrb5FZWDdbSkD1vX1X6 UhK7dXxhZ2iItIpB6cm33Y45GK1BhgTkgjTvoBZgwCphz1U08HmihA23AyoBUcxqZTSB ckaBSmVYrd/he1a9fSsoETVCOKh/gnlB4+9hov4aKLXuXRHb+mSOa3cbPOLHiKqi1JU3 6b/23cRJPdq6wy7KwG+aftsV8+uVNfVncV639Todm4K0cG/uIO7HpoAE4cKRSZN2ax/B BLMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=FHEbIybzmAfdL5VadI//MSMgY7LNjIhG4DgNBqZBDHc=; b=UTWCR2LyQYpVdvs3m+W1D8oWZk1+xBhgzLndJbZJD5rOEvnaJrRS0lyYlpepNRn1Wk vY+6LXvcyaZUBw+/HWunippE8j2Mr2C4+VhyZU7wRq2s/viPNoAiM8EcokF72teu119G TD8yIwxKpq5c68H5e+d8KelHUfWS4SPWncAtrorEIHSInXP2vk338U5Yne/tD2rU2Pj7 OO8voIVTGoaZ8OzlRsYS5E1/n/NBHz6lLIqGOcKusvOPumH4mj5Dg+MZp+hHcloAFkm+ t+23pBabUHCiEJAYUB4Ed8m15OLoNudMznF5RtbDTsDSDL0Q/VHHULQ6rGUxh/hivO4M FzEg== X-Gm-Message-State: AKGB3mI0lUZJrOrTCeFzfZvZsqTlvHYcOBjaUL3yCBd9ZCNJ1Tj4I3qW aM52Omr/xmdxBJxKhXS0LbSZxw== X-Google-Smtp-Source: ACJfBovgr7mSFTcacvvkin1I8gxjKtDVNOhLfWItW85AYZ/1hF73Qh+cScCv/T+bAr2VqP8ACtsD2Q== X-Received: by 10.80.144.241 with SMTP id d46mr31146878eda.263.1515688308288; Thu, 11 Jan 2018 08:31:48 -0800 (PST) Received: from [10.0.0.101] ([84.203.57.149]) by smtp.googlemail.com with ESMTPSA id y5sm11659289edj.5.2018.01.11.08.31.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Jan 2018 08:31:47 -0800 (PST) To: "Cohen, Eugene" , Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , "Gao, Liming" References: <20180110162644.11208-1-pete@akeo.ie> <20180110162644.11208-5-pete@akeo.ie> From: Pete Batard Message-ID: <5ccd6f32-ee75-57e2-75f0-1fe14cba6b05@akeo.ie> Date: Thu, 11 Jan 2018 16:31:46 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: Subject: Re: [PATCH v4 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Jan 2018 16:26:37 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Eugene, How about I modify the patch to use "AREA s_" instead of "AREA Math" as per the current proposal? I believe this should achieve the same result as the one you want, without having to rely on introducing conditional differences between MSFT and RVCT, which are going to make the code harder to maintain. In the places we modified, there was exactly 1 use of the macro per file, which means that, we should be able to "unroll" the macro and preserve the existing behaviour, while keeping MSFT happy without having to define separate macros and conditional stuff. Also, you'll notice that the current div.asm [1], which is used by RVCT does *not* rely on the macro, so, unless this is intentional, there already seems to exist inconsistencies with regards to using the RVCT_ASM_EXPORT macro to ensure the removal of dead code... So, to summarise, I would much prefer if we could keep most of the current patch, and simply use the following where needed: AREA s___aeabi_ldivmod, CODE, READONLY, ARM AREA s___aeabi_llsr, CODE, READONLY, ARM AREA s___aeabi_uldivmod, CODE, READONLY, ARM What do you think? Regards, /Pete [1] https://github.com/tianocore/edk2/blob/master/ArmPkg/Library/CompilerIntrinsicsLib/Arm/div.asm On 2018.01.11 15:30, Cohen, Eugene wrote: > >>> Introduce CRT assembly replacements for __rt_sdiv, __rt_udiv, >>> __rt_udiv64, __rt_sdiv64, __rt_srsh (by reusing the RVCT code) as well >>> as memcpy and memset. >>> For MSFT compatibility, some of the code needs to be explicitly forced >>> to ARM, and the /oldit assembly flag needs to be added. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Pete Batard >> >> This looks fine to me but I haven't been able to test it. >> >> Reviewed-by: Ard Biesheuvel >> >> Eugene: any comments regarding the changes to RVCT code? > > I've been away in Linux land for a while, it's nice that the day I catch up on my edk2-devel backlog I see my name! :) > > One concern for RVCT is that every function is in its own section to enable proper dead code removal. This is addressed with this macro: > > MACRO > RVCT_ASM_EXPORT $func > EXPORT $func > AREA s_$func, CODE, READONLY > $func > MEND > > note the AREA directive - this makes a section per function. > > Pete's patchset removes these macros which breaks dead code removal. > > Please restore these macros for at least RVCT and please verify that the VS2017 path does appropriate dead code removal for these assembly functions. > > Thanks! > > Eugene > >> >> >>> --- >>> ArmPkg/Library/CompilerIntrinsicsLib/Arm/div.asm | 43 >> +++++++++++++++++--- >>> ArmPkg/Library/CompilerIntrinsicsLib/Arm/ldivmod.asm | 40 >> +++++++++++++----- >>> ArmPkg/Library/CompilerIntrinsicsLib/Arm/llsr.asm | 22 +++++---- >> - >>> ArmPkg/Library/CompilerIntrinsicsLib/Arm/uldiv.asm | 29 >> +++++++++++-- >>> ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf | 16 >> +++++++- >>> ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c | 34 >> ++++++++++++++++ >>> ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c | 33 >> +++++++++++++++ >>> 7 files changed, 185 insertions(+), 32 deletions(-) >>> >>> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/div.asm >>> b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/div.asm >>> index b539e516892d..f9e0107395f2 100644 >>> --- a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/div.asm >>> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/div.asm >>> @@ -1,6 +1,7 @@ >>> >>> //-------------------------------------------------------------------- >>> ---------- >>> // >>> // Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>>> +// Copyright (c) 2018, Pete Batard. All rights reserved.
>>> // >>> // This program and the accompanying materials // are licensed and >>> made available under the terms and conditions of the BSD License @@ >>> -17,18 +18,19 @@ >>> EXPORT __aeabi_uidivmod >>> EXPORT __aeabi_idiv >>> EXPORT __aeabi_idivmod >>> + EXPORT __rt_udiv >>> + EXPORT __rt_sdiv >>> >>> AREA Math, CODE, READONLY >>> >>> ; >>> ;UINT32 >>> ;EFIAPI >>> -;__aeabi_uidivmode ( >>> -; IN UINT32 Dividen >>> +;__aeabi_uidivmod ( >>> +; IN UINT32 Dividend >>> ; IN UINT32 Divisor >>> ; ); >>> ; >>> - >>> __aeabi_uidiv >>> __aeabi_uidivmod >>> RSBS r12, r1, r0, LSR #4 >>> @@ -40,10 +42,40 @@ __aeabi_uidivmod >>> B __arm_div_large >>> >>> ; >>> +;UINT64 >>> +;EFIAPI >>> +;__rt_udiv ( >>> +; IN UINT32 Divisor, >>> +; IN UINT32 Dividend >>> +; ); >>> +; >>> +__rt_udiv >>> + ; Swap R0 and R1 >>> + MOV r12, r0 >>> + MOV r0, r1 >>> + MOV r1, r12 >>> + B __aeabi_uidivmod >>> + >>> +; >>> +;UINT64 >>> +;EFIAPI >>> +;__rt_sdiv ( >>> +; IN INT32 Divisor, >>> +; IN INT32 Dividend >>> +; ); >>> +; >>> +__rt_sdiv >>> + ; Swap R0 and R1 >>> + MOV r12, r0 >>> + MOV r0, r1 >>> + MOV r1, r12 >>> + B __aeabi_idivmod >>> + >>> +; >>> ;INT32 >>> ;EFIAPI >>> -;__aeabi_idivmode ( >>> -; IN INT32 Dividen >>> +;__aeabi_idivmod ( >>> +; IN INT32 Dividend >>> ; IN INT32 Divisor >>> ; ); >>> ; >>> @@ -152,4 +184,3 @@ __aeabi_idiv0 >>> BX r14 >>> >>> END >>> - >>> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/ldivmod.asm >>> b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/ldivmod.asm >>> index c71bd59e4520..3794cac35eed 100644 >>> --- a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/ldivmod.asm >>> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/ldivmod.asm >>> @@ -1,6 +1,7 @@ >>> >>> //-------------------------------------------------------------------- >>> ---------- >>> // >>> // Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>>> +// Copyright (c) 2018, Pete Batard. All rights reserved.
>>> // >>> // This program and the accompanying materials // are licensed and >>> made available under the terms and conditions of the BSD License @@ >>> -13,20 +14,41 @@ >>> >>> //-------------------------------------------------------------------- >>> ---------- >>> >>> >>> - EXTERN __aeabi_uldivmod >>> + IMPORT __aeabi_uldivmod >>> + EXPORT __aeabi_ldivmod >>> + EXPORT __rt_sdiv64 >>> >>> - INCLUDE AsmMacroExport.inc >>> + AREA Math, CODE, READONLY, ARM >>> + >>> + ARM >>> >>> ; >>> -;UINT32 >>> +;INT64 >>> ;EFIAPI >>> -;__aeabi_uidivmode ( >>> -; IN UINT32 Dividen >>> -; IN UINT32 Divisor >>> +;__rt_sdiv64 ( >>> +; IN INT64 Divisor >>> +; IN INT64 Dividend >>> ; ); >>> ; >>> +__rt_sdiv64 >>> + ; Swap r0-r1 and r2-r3 >>> + MOV r12, r0 >>> + MOV r0, r2 >>> + MOV r2, r12 >>> + MOV r12, r1 >>> + MOV r1, r3 >>> + MOV r3, r12 >>> + B __aeabi_ldivmod >>> >>> - RVCT_ASM_EXPORT __aeabi_ldivmod >>> +; >>> +;INT64 >>> +;EFIAPI >>> +;__aeabi_ldivmod ( >>> +; IN INT64 Dividend >>> +; IN INT64 Divisor >>> +; ); >>> +; >>> +__aeabi_ldivmod >>> PUSH {r4,lr} >>> ASRS r4,r1,#1 >>> EOR r4,r4,r3,LSR #1 >>> @@ -39,7 +61,7 @@ L_Test1 >>> RSBS r2,r2,#0 >>> RSC r3,r3,#0 >>> L_Test2 >>> - BL __aeabi_uldivmod ; >>> + BL __aeabi_uldivmod >>> TST r4,#0x40000000 >>> BEQ L_Test3 >>> RSBS r0,r0,#0 >>> @@ -53,5 +75,3 @@ L_Exit >>> POP {r4,pc} >>> >>> END >>> - >>> - >>> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/llsr.asm >>> b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/llsr.asm >>> index 881db106d9d7..db2fd5057832 100644 >>> --- a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/llsr.asm >>> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/llsr.asm >>> @@ -1,6 +1,7 @@ >>> >>> //-------------------------------------------------------------------- >>> ---------- >>> // >>> // Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>>> +// Copyright (c) 2018, Pete Batard. All rights reserved.
>>> // >>> // This program and the accompanying materials // are licensed and >>> made available under the terms and conditions of the BSD License @@ >>> -12,32 +13,33 @@ // >>> >>> //-------------------------------------------------------------------- >>> ---------- >>> >>> + EXPORT __aeabi_llsr >>> + EXPORT __rt_srsh >>> >>> + AREA Math, CODE, READONLY, ARM >>> >>> - INCLUDE AsmMacroExport.inc >>> + ARM >>> >>> ; >>> ;VOID >>> ;EFIAPI >>> ;__aeabi_llsr ( >>> -; IN VOID *Destination, >>> -; IN VOID *Source, >>> -; IN UINT32 Size >>> -; ); >>> +; IN UINT64 Value, >>> +; IN UINT32 Shift >>> +;) >>> ; >>> - RVCT_ASM_EXPORT __aeabi_llsr >>> +__aeabi_llsr >>> +__rt_srsh >>> SUBS r3,r2,#0x20 >>> - BPL {pc} + 0x18 ; 0x1c >>> + BPL __aeabi_llsr_label1 >>> RSB r3,r2,#0x20 >>> LSR r0,r0,r2 >>> ORR r0,r0,r1,LSL r3 >>> LSR r1,r1,r2 >>> BX lr >>> +__aeabi_llsr_label1 >>> LSR r0,r1,r3 >>> MOV r1,#0 >>> BX lr >>> >>> END >>> - >>> - >>> - >>> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/uldiv.asm >>> b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/uldiv.asm >>> index 6b6184ebd3fc..c5632b4e95e7 100644 >>> --- a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/uldiv.asm >>> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/uldiv.asm >>> @@ -1,6 +1,7 @@ >>> >>> //-------------------------------------------------------------------- >>> ---------- >>> // >>> // Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>>> +// Copyright (c) 2018, Pete Batard. All rights reserved.
>>> // >>> // This program and the accompanying materials // are licensed and >>> made available under the terms and conditions of the BSD License @@ >>> -13,9 +14,30 @@ >>> >>> //-------------------------------------------------------------------- >>> ---------- >>> >>> >>> + EXPORT __aeabi_uldivmod >>> + EXPORT __rt_udiv64 >>> >>> + AREA Math, CODE, READONLY, ARM >>> >>> - INCLUDE AsmMacroExport.inc >>> + ARM >>> + >>> +; >>> +;UINT64 >>> +;EFIAPI >>> +;__rt_udiv64 ( >>> +; IN UINT64 Divisor >>> +; IN UINT64 Dividend >>> +; ) >>> +; >>> +__rt_udiv64 >>> + ; Swap r0-r1 and r2-r3 >>> + mov r12, r0 >>> + mov r0, r2 >>> + mov r2, r12 >>> + mov r12, r1 >>> + mov r1, r3 >>> + mov r3, r12 >>> + b __aeabi_uldivmod >>> >>> ; >>> ;UINT64 >>> @@ -25,7 +47,7 @@ >>> ; IN UINT64 Divisor >>> ; ) >>> ; >>> - RVCT_ASM_EXPORT __aeabi_uldivmod >>> +__aeabi_uldivmod >>> stmdb sp!, {r4, r5, r6, lr} >>> mov r4, r1 >>> mov r5, r0 >>> @@ -261,7 +283,6 @@ _ll_div0 >>> b __aeabi_ldiv0 >>> >>> __aeabi_ldiv0 >>> - BX r14 >>> + bx r14 >>> >>> END >>> - >>> diff --git >>> a/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf >>> b/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf >>> index 44333141a70a..14e88da7ce06 100644 >>> --- a/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf >>> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf >>> @@ -23,8 +23,12 @@ [Defines] >>> LIBRARY_CLASS = CompilerIntrinsicsLib >>> >>> [Sources] >>> - memcpy.c >>> - memset.c >>> + memcpy.c | RVCT >>> + memcpy.c | GCC >>> + memcpy_ms.c | MSFT >>> + memset.c | RVCT >>> + memset.c | GCC >>> + memset_ms.c | MSFT >>> >>> [Sources.ARM] >>> Arm/mullu.asm | RVCT >>> @@ -94,6 +98,10 @@ [Sources.ARM] >>> Arm/llsr.S | GCC >>> Arm/llsl.S | GCC >>> >>> + Arm/div.asm | MSFT >>> + Arm/uldiv.asm | MSFT >>> + Arm/ldivmod.asm | MSFT >>> + Arm/llsr.asm | MSFT >>> >>> [Packages] >>> MdePkg/MdePkg.dec >>> @@ -101,3 +109,7 @@ [Packages] >>> >>> [LibraryClasses] >>> >>> +[BuildOptions] >>> + MSFT:*_*_ARM_CC_FLAGS = /GL- >>> + MSFT:*_*_ARM_ASM_FLAGS = /oldit >>> + MSFT:*_*_AARCH64_CC_FLAGS = /GL- >>> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c >>> b/ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c >>> new file mode 100644 >>> index 000000000000..90bbbb930d31 >>> --- /dev/null >>> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c >>> @@ -0,0 +1,34 @@ >>> +//------------------------------------------------------------------- >>> +----------- >>> +// >>> +// Copyright (c) 2017, Pete Batard. 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 >>> +// >>> +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS >> IS" >>> +BASIS, // WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, >> EITHER >>> +EXPRESS OR // IMPLIED. >>> +// >>> +//------------------------------------------------------------------- >>> +----------- >>> + >>> +#if defined(_M_ARM64) >>> +typedef unsigned __int64 size_t; >>> +#else >>> +typedef unsigned __int32 size_t; >>> +#endif >>> + >>> +void* memcpy(void *, const void *, size_t); #pragma intrinsic(memcpy) >>> +#pragma function(memcpy) >>> +void* memcpy(void *dest, const void *src, size_t n) { >>> + unsigned char *d = dest; >>> + unsigned char const *s = src; >>> + >>> + while (n--) >>> + *d++ = *s++; >>> + >>> + return dest; >>> +} >>> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c >>> b/ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c >>> new file mode 100644 >>> index 000000000000..64205e5d1012 >>> --- /dev/null >>> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c >>> @@ -0,0 +1,33 @@ >>> +//------------------------------------------------------------------- >>> +----------- >>> +// >>> +// Copyright (c) 2017, Pete Batard. 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 >>> +// >>> +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS >> IS" >>> +BASIS, // WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, >> EITHER >>> +EXPRESS OR // IMPLIED. >>> +// >>> +//------------------------------------------------------------------- >>> +----------- >>> + >>> +#if defined(_M_ARM64) >>> +typedef unsigned __int64 size_t; >>> +#else >>> +typedef unsigned __int32 size_t; >>> +#endif >>> + >>> +void* memset(void *, int, size_t); >>> +#pragma intrinsic(memset) >>> +#pragma function(memset) >>> +void *memset(void *s, int c, size_t n) { >>> + unsigned char *d = s; >>> + >>> + while (n--) >>> + *d++ = (unsigned char)c; >>> + >>> + return s; >>> +} >>> -- >>> 2.9.3.windows.2 >>>