Re: [PATCH 1/1] ArmPkg/CompilerIntrinsicsLib: Add lasr assembly source for gcc/arm

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Tue, May 07, 2019 at 04:57:26PM +0100, Pete Batard wrote:
> While compiling drivers or applications on Debian 9.8, using the native
> gcc 6.3.0 ARM compiler, one may run into the following build failure:
> 
>   "undefined reference to `__aeabi_lasr'"
> 
> This patch adds the missing definition, through a port of the existing
> RVCT lasr.asm source.

Ugh. Here's a module that could do with a bit of love.
ArmPkg/Library/CompilerIntrinsicsLib/Arm/switch.asm,
ArmPkg/Library/CompilerIntrinsicsLib/Arm/uread.asm and
ArmPkg/Library/CompilerIntrinsicsLib/Arm/uwrite.asm
also all miss .S versions.

And then there is an ominous comment in the .inf
"# Move .c to .s to work around LLVM issues"
and it appears all of the .c files are actually unused.

I'm not going to try to make you fix those too, but if you're feeling
charitable...

One comment below.

> CC: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> CC: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>  ArmPkg/Library/CompilerIntrinsicsLib/Arm/lasr.S                | 22 ++++++++++++++++++++
>  ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf |  1 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/lasr.S b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/lasr.S
> new file mode 100644
> index 000000000000..9ded045b8bc4
> --- /dev/null
> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/lasr.S
> @@ -0,0 +1,22 @@
> +#------------------------------------------------------------------------------
> +#
> +# Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#------------------------------------------------------------------------------
> +
> +#include <AsmMacroIoLib.h>
> +

Could you add a "prototype" here?
The one in the original is nonsense, but
something like

#
#INT64
#EFIAPI
#__aeabi_lasr (
#  IN INT64 Value,
#  IN INT32 Shift
#  );
#

?

/
    Leif

> +ASM_FUNC(__aeabi_lasr)
> +  subs     r3,r2,#0x20
> +  bpl      L_Test
> +  rsb      r3,r2,#0x20
> +  lsr      r0,r0,r2
> +  orr      r0,r0,r1,LSL r3
> +  asr      r1,r1,r2
> +  bx       lr
> +L_Test:
> +  asr      r0,r1,r3
> +  asr      r1,r1,#31
> +  bx       lr
> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf b/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> index 0d5634959993..7cb8f194f210 100644
> --- a/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> @@ -90,6 +90,7 @@ [Sources.ARM]
>    Arm/uldiv.S      | GCC
>    Arm/ldivmod.S    | GCC
>  
> +  Arm/lasr.S       | GCC
>    Arm/llsr.S       | GCC
>    Arm/llsl.S       | GCC
>  
> -- 
> 2.21.0.windows.1
>