From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Pete Batard <pete@akeo.ie>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Gao, Liming" <liming.gao@intel.com>
Subject: Re: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
Date: Tue, 12 Dec 2017 19:59:49 +0000 [thread overview]
Message-ID: <CAKv+Gu9qu-JzffSXLOVVXmkmYR5HFQ6DKAo7MDkUxhnLp_asZg@mail.gmail.com> (raw)
In-Reply-To: <20171208140631.4252-5-pete@akeo.ie>
Hi Pete,
On 8 December 2017 at 14:06, Pete Batard <pete@akeo.ie> 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 <pete@akeo.ie>
> ---
> 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.<BR>
> +// Based on generated assembly of ReactOS' sdk/lib/crt/math/arm/__rt_###div.c,
> +// Copyright (c) Timo Kreuzer. All rights reserved.<BR>
> +//
> +// 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.<BR>
> +//
> +// 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.<BR>
> +//
> +// 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.<BR>
> +//
> +// 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.
next prev parent reply other threads:[~2017-12-12 19:55 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-08 14:06 [PATCH v3 0/6] Add ARM support for VS2017 Pete Batard
2017-12-08 14:06 ` [PATCH v3 1/6] MdePkg: Disable some Level 4 warnings for VS2017/ARM Pete Batard
2017-12-08 14:06 ` [PATCH v3 2/6] MdePkg/Library/BaseStackCheckLib: Add Null handler " Pete Batard
2017-12-08 14:06 ` [PATCH v3 3/6] MdePkg/Library/BaseLib: Enable VS2017/ARM builds Pete Batard
2017-12-08 14:06 ` [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: " Pete Batard
2017-12-10 13:30 ` Gao, Liming
2017-12-12 19:59 ` Ard Biesheuvel [this message]
2017-12-13 13:56 ` Pete Batard
2017-12-14 14:05 ` Ard Biesheuvel
2017-12-08 14:06 ` [PATCH v3 5/6] MdePkg/Include: Add VA list support for VS2017/ARM Pete Batard
2017-12-08 14:06 ` [PATCH v3 6/6] BaseTools/Conf: Add VS2017/ARM support Pete Batard
2017-12-08 14:13 ` [PATCH v3 0/6] Add ARM support for VS2017 Gao, Liming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu9qu-JzffSXLOVVXmkmYR5HFQ6DKAo7MDkUxhnLp_asZg@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox