From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::242; helo=mail-io0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::242]) (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 5ABF8220EE115 for ; Tue, 12 Dec 2017 11:55:12 -0800 (PST) Received: by mail-io0-x242.google.com with SMTP id u42so277620ioi.9 for ; Tue, 12 Dec 2017 11:59:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=wxI94MjyDnAJMr9r54MZQ9k3D2GeHInHAYwhPHP1ltY=; b=iJokUtpG7UDmuULK6UFOeJt1WzE6m8Q4IqlUQOzlSHyv5YI8yxhJK7RP85ueKhHQpG M11TTlcNYMBZcVAluf8OQAJRbz5cQG6zNnJjjDfiBCNQhG+P10gp2NFoWCw2u65TFwSr mXYsJAopWPhNPd6KSNWQdlGIZBxMTfo5HlSac= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=wxI94MjyDnAJMr9r54MZQ9k3D2GeHInHAYwhPHP1ltY=; b=bOVABXptRPYen1M5p0wjTZY6JXrbsCCx1q7chhrZ7wSL9Y3X6dod9SpEtXSqA4ciPL h1nBAXbnItWPLrs0Hm7kgJRDBOBGt3F32nrjshFgJI6vMW7BS3Ir9VnbekRA4/sEClQb /U1s7lrXruuLkcpFJlcwTUC+UXoQDK33ciWf6GpY0Rpi2sbaoiYEEF21pqjBoCeUaCvV KdKDzK+xRSZvY6G0ywcHKT/LFblER4SAAlTt+eYrpp2zxhPY3TFimvL7lLSUvOSc2VK9 pPsNPlrTO+U2D8q3Xdkndj9Cz544r5YhaW8eDmXVNFq0v+MN/AlEjcZ5kvXqFUQITW7W MV7g== X-Gm-Message-State: AKGB3mLohyj5q231ULcoEy8b8OHeFjf8dXMBAP8B18S/1TnJBEl6rqc6 eovxTx/o3fQIUwnOdM7s89nMSozQDlAff39MVMCBXzzs X-Google-Smtp-Source: ACJfBov/lK4gG5eYJ/AtWezGIWjMisxXkCZHzTWuJvwkkrqY/oPKExhZBPGZFlQp/QeD8EoNkzibN/UAdSl8kYP6Iw8= X-Received: by 10.107.170.90 with SMTP id t87mr5910ioe.253.1513108790367; Tue, 12 Dec 2017 11:59:50 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Tue, 12 Dec 2017 11:59:49 -0800 (PST) In-Reply-To: <20171208140631.4252-5-pete@akeo.ie> References: <20171208140631.4252-1-pete@akeo.ie> <20171208140631.4252-5-pete@akeo.ie> From: Ard Biesheuvel Date: Tue, 12 Dec 2017 19:59:49 +0000 Message-ID: To: Pete Batard Cc: "edk2-devel@lists.01.org" , "Gao, Liming" Subject: Re: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds 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: Tue, 12 Dec 2017 19:55:12 -0000 Content-Type: text/plain; charset="UTF-8" Hi Pete, On 8 December 2017 at 14:06, Pete Batard wrote: > Introduce CRT assembly replacements for __rt_sdiv, __rt_udiv, > __rt_udiv64, __rt_sdiv64, __rt_srsh, memcpy and memset. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Pete Batard > --- > ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm | 255 ++++++++++++++++++++ > ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm | 45 ++++ > ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf | 13 +- > ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c | 34 +++ > ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c | 33 +++ > 5 files changed, 378 insertions(+), 2 deletions(-) > > diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm > new file mode 100644 > index 000000000000..096dc6317318 > --- /dev/null > +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm > @@ -0,0 +1,255 @@ > +///** @file > +// > +// This code provides replacement for MSVC CRT division functions > +// > +// Copyright (c) 2017, Pete Batard. All rights reserved.
> +// Based on generated assembly of ReactOS' sdk/lib/crt/math/arm/__rt_###div.c, > +// Copyright (c) Timo Kreuzer. 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. > +// > +//**/ > + > + EXPORT _fltused > + EXPORT __brkdiv0 > + Why are these being exported? Are these part of the CRT ABI? > + EXPORT __rt_sdiv > + EXPORT __rt_udiv > + EXPORT __rt_udiv64 > + EXPORT __rt_sdiv64 > + > + AREA Math, CODE, READONLY > + > +_fltused > + dcd 0x9875 > + > +__brkdiv0 > + udf #249 > + This will cause a crash when invoked, in a way that gives very little to go on to figure out what happens. Perhaps branch to a C function here that calls ASSERT() and CpuDeadLoop ? > +// > +// uint64_t __rt_udiv(uint32_t divisor, uint32_t dividend) > +// There are many different division implementations already in the same directory. Do we really need a new one? > + > +__rt_udiv > + cmp r0, #0 > + beq __brkdiv0 > + push {r3-r5,lr} > + mov r5,r0 > + mov r4,r1 > + cmp r5,r4 > + it hi > + movhi r0,#0 > + bhi __rt_udiv_label3 > + clz r2,r5 > + clz r3,r4 > + subs r3,r2,r3 > + movs r1,#1 > + lsl r2,r5,r3 > + lsl r3,r1,r3 > + movs r0,#0 > +__rt_udiv_label1 > + cmp r4,r2 > + bcc __rt_udiv_label2 > + orrs r0,r0,r3 > + subs r4,r4,r2 > +__rt_udiv_label2 > + lsrs r2,r2,#1 > + lsrs r3,r3,#1 > + bne __rt_udiv_label1 > +__rt_udiv_label3 > + mov r1,r4 > + pop {r3-r5,pc} > + > +// > +// uint64_t __rt_sdiv(int32_t divisor, int32_t dividend) > +// > + Does signed division return an unsigned result? And do we really need yet another implementation? > +__rt_sdiv > + cmp r0, #0 > + beq __brkdiv0 > + push {r4-r6,lr} > + mov r4,r1 > + ands r6,r0,#0x80000000 > + it ne > + rsbne r4,r4,#0 > + mov r5,r0 > + rsbs r5,r5,#0 > + cmp r5,r4 > + it hi > + movhi r0,#0 > + bhi __rt_sdiv_label3 > + clz r2,r5 > + clz r3,r4 > + subs r3,r2,r3 > + movs r1,#1 > + lsl r2,r5,r3 > + lsl r3,r1,r3 > + movs r0,#0 > +__rt_sdiv_label1 > + cmp r4,r2 > + bcc __rt_sdiv_label2 > + orrs r0,r0,r3 > + subs r4,r4,r2 > +__rt_sdiv_label2 > + lsrs r2,r2,#1 > + lsrs r3,r3,#1 > + bne __rt_sdiv_label1 > +__rt_sdiv_label3 > + cbz r6,__rt_sdiv_label4 > + rsbs r4,r4,#0 > +__rt_sdiv_label4 > + mov r1,r4 > + pop {r4-r6,pc} > + > +// > +// typedef struct { > +// uint64_t quotient; > +// uint64_t modulus; > +// } udiv64_result_t; > +// > +// void __rt_udiv64_internal(udiv64_result_t *result, uint64_t divisor, uint64_t dividend) > +// > + > +__rt_udiv64_internal > + orrs r1,r2,r3 > + beq __brkdiv0 > + push {r4-r8,lr} > + mov r7,r3 > + mov r6,r2 > + mov r4,r0 > + ldrd r0,r5,[sp,#0x18] > + cmp r7,r5 > + bcc __rt_udiv64_internal_label2 > + bhi __rt_udiv64_internal_label1 > + cmp r6,r0 > + bls __rt_udiv64_internal_label2 > +__rt_udiv64_internal_label1 > + movs r3,#0 > + strd r3,r3,[r4] > + b __rt_udiv64_internal_label8 > +__rt_udiv64_internal_label2 > + clz r2,r7 > + cmp r2,#0x20 > + bne __rt_udiv64_internal_label3 > + clz r3,r6 > + add r2,r2,r3 > +__rt_udiv64_internal_label3 > + clz r1,r5 ; > + cmp r1,#0x20 > + bne __rt_udiv64_internal_label4 > + clz r3,r0 > + add r1,r1,r3 > +__rt_udiv64_internal_label4 > + subs r1,r2,r1 > + rsb r3,r1,#0x20 > + lsr r3,r6,r3 > + lsl r2,r7,r1 > + orrs r2,r2,r3 > + sub r3,r1,#0x20 > + lsl r3,r6,r3 > + orrs r2,r2,r3 > + lsl r7,r6,r1 > + sub r3,r1,#0x20 > + movs r6,#1 > + lsls r6,r6,r3 > + movs r3,#1 > + mov lr,#0 > + lsl r1,r3,r1 > + mov r8,lr > +__rt_udiv64_internal_label5 > + cmp r5,r2 > + bcc __rt_udiv64_internal_label7 > + bhi __rt_udiv64_internal_label6 > + cmp r0,r7 > + bcc __rt_udiv64_internal_label7 > +__rt_udiv64_internal_label6 > + subs r0,r0,r7 > + sbcs r5,r5,r2 > + orr lr,lr,r1 > + orr r8,r8,r6 > +__rt_udiv64_internal_label7 > + lsls r3,r2,#0x1F > + orr r7,r3,r7,lsr #1 > + lsls r3,r6,#0x1F > + orr r1,r3,r1,lsr #1 > + lsrs r6,r6,#1 > + lsrs r2,r2,#1 > + orrs r3,r1,r6 > + bne __rt_udiv64_internal_label5 > + strd lr,r8,[r4] > +__rt_udiv64_internal_label8 > + str r5,[r4,#0xC] > + str r0,[r4,#8] > + pop {r4-r8,pc} > + > +// > +// {int64_t, int64_t} __rt_sdiv64(int64_t divisor, int64_t dividend) > +// > + > +__rt_sdiv64 > + push {r4-r6,lr} > + sub sp,sp,#0x18 > + and r6,r1,#0x80000000 > + movs r4,r6 > + mov r5,r0 > + beq __rt_sdiv64_label1 > + movs r0,#0 > + rsbs r2,r2,#0 > + sbc r3,r0,r3 > +__rt_sdiv64_label1 > + movs r4,r6 > + beq __rt_sdiv64_label2 > + movs r0,#0 > + rsbs r5,r5,#0 > + sbc r1,r0,r1 > +__rt_sdiv64_label2 > + str r2,[sp] > + str r3,[sp,#4] > + mov r3,r1 > + mov r2,r5 > + add r0,sp,#8 > + bl __rt_udiv64_internal > + movs r3,r6 > + beq __rt_sdiv64_label3 > + ldrd r3,r2,[sp,#0x10] > + movs r1,#0 > + rsbs r3,r3,#0 > + sbcs r1,r1,r2 > + b __rt_sdiv64_label4 > +__rt_sdiv64_label3 > + ldrd r3,r1,[sp,#0x10] > +__rt_sdiv64_label4 > + mov r2,r3 > + ldr r0,[sp,#8] > + mov r3,r1 > + ldr r1,[sp,#0xC] > + add sp,sp,#0x18 > + pop {r4-r6,pc} > + > +// > +// {uint64_t, uint64_t} __rt_udiv64(uint64_t divisor, uint64_t dividend) > +// > + > +__rt_udiv64 > + push {r4,r5,lr} > + sub sp,sp,#0x1C > + mov r4,r2 > + mov r2,r0 > + mov r5,r3 > + add r0,sp,#8 > + mov r3,r1 > + str r4,[sp] > + str r5,[sp,#4] > + bl __rt_udiv64_internal > + ldrd r2,r3,[sp,#0x10] > + ldrd r0,r1,[sp,#8] > + add sp,sp,#0x1C > + pop {r4,r5,pc} > + > + END > diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm > new file mode 100644 > index 000000000000..2332dda823f2 > --- /dev/null > +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm > @@ -0,0 +1,45 @@ > +///** @file > +// > +// This code provides replacement for MSVC CRT __rt_srsh > +// > +// Copyright (c) Timo Kreuzer. 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. > +// > +//**/ > + > + EXPORT __rt_srsh > + > + AREA Math, CODE, READONLY > + > +// > +// int64_t __rt_srsh(int64_t value, uint32_t shift); > +// > + Same questions here, basically. > +__rt_srsh > + rsbs r3, r2, #32 > + bmi __rt_srsh_label1 > + lsr r0, r0, r2 > + lsl r3, r1, r3 > + orr r0, r0, r3 > + asr r1, r1, r2 > + bx lr > +__rt_srsh_label1 > + cmp r2, 64 > + bhs __rt_srsh_label2 > + sub r3, r2, #32 > + asr r0, r1, r3 > + asr r1, r1, #32 > + bx lr > +__rt_srsh_label2 > + asr r1, r1, #32 > + mov r0, r1 > + bx lr > + > + END > diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf b/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf > index 44333141a70a..0dacc5e5117d 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,8 @@ [Sources.ARM] > Arm/llsr.S | GCC > Arm/llsl.S | GCC > > + Arm/rtdiv.asm | MSFT > + Arm/rtsrsh.asm | MSFT > > [Packages] > MdePkg/MdePkg.dec > @@ -101,3 +107,6 @@ [Packages] > > [LibraryClasses] > > +[BuildOptions] > + MSFT:*_*_ARM_CC_FLAGS = /GL- > + 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; > +} These look fine to me.