public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Add ARM support for VS2017
@ 2017-12-08 14:06 Pete Batard
  2017-12-08 14:06 ` [PATCH v3 1/6] MdePkg: Disable some Level 4 warnings for VS2017/ARM Pete Batard
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Pete Batard @ 2017-12-08 14:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: liming.gao

This v3 is the same as v2, except for the following comment added in
tools_def for VS2017:
# Building of XIP firmware images for ARM is not currently supported (only applications).
# /FILEALIGN:4096 and other changes are needed for ARM firmware builds.

The following series adds ARM compilation support for the VS2017 toolchain.
* PATCH 1 targets the disabling of VS Level 4 warnings. The disabled warnings
  for ARM are now aligned with IA32 and X64.
* PATCH 2 adds a NULL handler for the base stack check, since this is a GCC
  functionality.
* PATCH 3 updates MdePkg/Library/BaseLib so that the RVCT assembly sources
  are also used for MSFT.
* PATCH 4 adds the required compiler intrinsics replacements for division,
  shift and memset/memcpy.
* PATCH 5 adds variable argument handlers for print output. Note that this
  is done without relying on any external headers, with the VA_ARG macro
  having been reverse engineered from MSFT ARM assembly output.
* PATCH 6 enables the selection of ARM in the conf templates.

With these patches, VS2017 toolchain users should be able to compile
regular UEFI ARM applications using EDK2. Note that, unlike ARM64 support,
ARM support does not require a specific update of Visual Studio 2017, as
the ARM toolchain has been available from the very first release.

Additional notes:

We tested compiling and running the full UEFI Shell with this series, as
well as a small set of applications and drivers, and found no issues.
With an additional patch [1], it is also possible to use this proposal to
compile a complete QEMU ARM firmware. As the patch shows, the changes that
need to be applied to the EDK2 sources to achieve this are actually very
minimal.

However, the generated firmware does not currently boot, possibly because
of the following warnings being generated by the MS compiler:
- ArmCpuDxe.dll : warning LNK4072: section count 118 exceeds max (96); image may not run
- UiApp.dll : warning LNK4072: section count 113 exceeds max (96); image may not run

As far as I could see, the section count max is hardcoded so a workaround
would be needed to address those.

Also, because the VS2017 ARM compiler forces a section alignment of 4096
bytes (which in turn forces use to use /FILEALIGN:4096 as a linker option
for the firmware generation), the generated firmware exceeds 2MB and we
had to double its size to 4MB.

At this stage, since the goal of this series is to allow users to compile
regular ARM UEFI applications using the VS2017 toolchain, I have non plans
to spend more time on the QEMU firmware issues, especially as I suspect 
that reducing the firmware size back to 2 MB may not be achievable without
Microsoft altering their compiler. I am however hopeful that ARM
specialists can take this matter over eventually...

Regards,

/Pete

[1] https://github.com/pbatard/edk2/commit/c4ce41094a46f4f3dc7ccc64a90604813f037b13

Pete Batard (6):
  MdePkg: Disable some Level 4 warnings for VS2017/ARM
  MdePkg/Library/BaseStackCheckLib: Add Null handler for VS2017/ARM
  MdePkg/Library/BaseLib: Enable VS2017/ARM builds
  ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
  MdePkg/Include: Add VA list support for VS2017/ARM
  BaseTools/Conf: Add VS2017/ARM support

 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 +++
 BaseTools/Conf/build_rule.template                             |  31 ++-
 BaseTools/Conf/tools_def.template                              |  31 +++
 MdePkg/Include/Arm/ProcessorBind.h                             |  96 ++++++--
 MdePkg/Include/Base.h                                          |  13 +
 MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm                   |   5 +-
 MdePkg/Library/BaseLib/BaseLib.inf                             |  16 +-
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf         |   5 +-
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c          |  18 ++
 13 files changed, 565 insertions(+), 30 deletions(-)
 create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm
 create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm
 create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c
 create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c
 create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c

-- 
2.9.3.windows.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/6] MdePkg: Disable some Level 4 warnings for VS2017/ARM
  2017-12-08 14:06 [PATCH v3 0/6] Add ARM support for VS2017 Pete Batard
@ 2017-12-08 14:06 ` Pete Batard
  2017-12-08 14:06 ` [PATCH v3 2/6] MdePkg/Library/BaseStackCheckLib: Add Null handler " Pete Batard
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2017-12-08 14:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: liming.gao

We disable the exact same warnings as IA32 and X64.

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 <pete@akeo.ie>
---
 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..0700e4a4c50a 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.<BR>
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
   Portions copyright (c) 2008 - 2009, Apple Inc. 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                                            
+  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 some level 4 compilation warnings (same as IA32 and X64)
 //
-#ifdef __ARMCC_VERSION
+
+//
+// Disabling bitfield type checking warnings.
+//
+#pragma warning ( disable : 4214 )
+
+//
+// Disabling the unreferenced formal parameter warnings.
+//
+#pragma warning ( disable : 4100 )
+
+//
+// Disable slightly different base types warning as CHAR8 * can not be set
+// to a constant string.
+//
+#pragma warning ( disable : 4057 )
+
+//
+// ASSERT(FALSE) or while (TRUE) are legal constructs so suppress this warning
+//
+#pragma warning ( disable : 4127 )
+
+//
+// This warning is caused by functions defined but not used. For precompiled header only.
+//
+#pragma warning ( disable : 4505 )
+
+//
+// This warning is caused by empty (after preprocessing) source file. For precompiled header only.
+//
+#pragma warning ( disable : 4206 )
+
+//
+// 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



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/6] MdePkg/Library/BaseStackCheckLib: Add Null handler for VS2017/ARM
  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 ` Pete Batard
  2017-12-08 14:06 ` [PATCH v3 3/6] MdePkg/Library/BaseLib: Enable VS2017/ARM builds Pete Batard
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2017-12-08 14:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: liming.gao

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard <pete@akeo.ie>
---
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf |  5 +++--
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c  | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
index d02d97107b08..e280651b1199 100644
--- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
@@ -30,8 +30,9 @@ [Defines]
 #
 
 [Sources]
-  BaseStackCheckGcc.c | GCC
-  BaseStackCheckGcc.c | RVCT
+  BaseStackCheckGcc.c  | GCC
+  BaseStackCheckGcc.c  | RVCT
+  BaseStackCheckNull.c | MSFT
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
new file mode 100644
index 000000000000..fb2f65929d3e
--- /dev/null
+++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
@@ -0,0 +1,18 @@
+/*++
+
+ Copyright (c) 2017, Intel Corporation. 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.
+
+Abstract:
+
+  This file is purely empty as a work around for BaseStackCheck to pass MSVC build.
+
+**/
+
+extern int __BaseStackCheckNull;
-- 
2.9.3.windows.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/6] MdePkg/Library/BaseLib: Enable VS2017/ARM builds
  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 ` Pete Batard
  2017-12-08 14:06 ` [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: " Pete Batard
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2017-12-08 14:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: liming.gao

Most of the RVCT assembly can be reused as is for MSFT except
for CpuBreakpoint.asm, which we need to force to Arm mode.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard <pete@akeo.ie>
---
 MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm |  5 ++++-
 MdePkg/Library/BaseLib/BaseLib.inf           | 16 +++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm b/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm
index 8a8065159bf2..2e508d6f1ad8 100644
--- a/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm
+++ b/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm
@@ -16,7 +16,10 @@
 
     EXPORT CpuBreakpoint
 
-  AREA Cpu_Breakpoint, CODE, READONLY
+; Force ARM mode for this section, as MSFT assembler defaults to THUMB
+  AREA Cpu_Breakpoint, CODE, READONLY, ARM
+
+    ARM
 
 ;/**
 ;  Generates a breakpoint on the CPU.
diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index 320ac457ea3d..a81d56f61421 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -821,8 +821,9 @@ [Sources.EBC]
 [Sources.ARM]
   Arm/InternalSwitchStack.c
   Arm/Unaligned.c
-  Math64.c                   | RVCT 
-    
+  Math64.c                   | RVCT
+  Math64.c                   | MSFT
+
   Arm/SwitchStack.asm        | RVCT
   Arm/SetJumpLongJump.asm    | RVCT
   Arm/DisableInterrupts.asm  | RVCT
@@ -831,7 +832,16 @@ [Sources.ARM]
   Arm/CpuPause.asm           | RVCT
   Arm/CpuBreakpoint.asm      | RVCT
   Arm/MemoryFence.asm        | RVCT
- 
+
+  Arm/SwitchStack.asm        | MSFT
+  Arm/SetJumpLongJump.asm    | MSFT
+  Arm/DisableInterrupts.asm  | MSFT
+  Arm/EnableInterrupts.asm   | MSFT
+  Arm/GetInterruptsState.asm | MSFT
+  Arm/CpuPause.asm           | MSFT
+  Arm/CpuBreakpoint.asm      | MSFT
+  Arm/MemoryFence.asm        | MSFT
+
   Arm/Math64.S                  | GCC
   Arm/SwitchStack.S             | GCC
   Arm/EnableInterrupts.S        | GCC
-- 
2.9.3.windows.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
  2017-12-08 14:06 [PATCH v3 0/6] Add ARM support for VS2017 Pete Batard
                   ` (2 preceding siblings ...)
  2017-12-08 14:06 ` [PATCH v3 3/6] MdePkg/Library/BaseLib: Enable VS2017/ARM builds Pete Batard
@ 2017-12-08 14:06 ` Pete Batard
  2017-12-10 13:30   ` Gao, Liming
  2017-12-12 19:59   ` Ard Biesheuvel
  2017-12-08 14:06 ` [PATCH v3 5/6] MdePkg/Include: Add VA list support for VS2017/ARM Pete Batard
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 12+ messages in thread
From: Pete Batard @ 2017-12-08 14:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: liming.gao

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
+
+  EXPORT __rt_sdiv
+  EXPORT __rt_udiv
+  EXPORT __rt_udiv64
+  EXPORT __rt_sdiv64
+
+  AREA  Math, CODE, READONLY
+
+_fltused
+    dcd 0x9875
+
+__brkdiv0
+    udf #249
+
+//
+// uint64_t __rt_udiv(uint32_t divisor, uint32_t dividend)
+//
+
+__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)
+//
+
+__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);
+//
+
+__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;
+}
-- 
2.9.3.windows.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 5/6] MdePkg/Include: Add VA list support for VS2017/ARM
  2017-12-08 14:06 [PATCH v3 0/6] Add ARM support for VS2017 Pete Batard
                   ` (3 preceding siblings ...)
  2017-12-08 14:06 ` [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: " Pete Batard
@ 2017-12-08 14:06 ` 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
  6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2017-12-08 14:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: liming.gao

VA_START, VA_END and VA_COPY are the same as the generic macros.
VA_ARG was reverse engineered from MS ARM assembly output.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard <pete@akeo.ie>
---
 MdePkg/Include/Base.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 02140a5ac2ee..f3467ecdc22f 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -631,6 +631,19 @@ struct _LIST_ENTRY {
 
 #define VA_COPY(Dest, Start)          __va_copy (Dest, Start)
 
+#elif defined(_M_ARM)
+//
+// MSFT ARM variable argument list support.
+// Same as the generic macros below, except for VA_ARG that needs extra adjustment.
+//
+
+typedef char* VA_LIST;
+
+#define VA_START(Marker, Parameter)     (Marker = (VA_LIST) ((UINTN) & (Parameter) + _INT_SIZE_OF(Parameter)))
+#define VA_ARG(Marker, TYPE)            (*(TYPE *) ((Marker += _INT_SIZE_OF(TYPE) + ((-(INTN)Marker) & (sizeof(TYPE) - 1))) - _INT_SIZE_OF (TYPE)))
+#define VA_END(Marker)                  (Marker = (VA_LIST) 0)
+#define VA_COPY(Dest, Start)            ((void)((Dest) = (Start)))
+
 #elif defined(__GNUC__)
 
 #if defined(MDE_CPU_X64) && !defined(NO_MSABI_VA_FUNCS)
-- 
2.9.3.windows.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 6/6] BaseTools/Conf: Add VS2017/ARM support
  2017-12-08 14:06 [PATCH v3 0/6] Add ARM support for VS2017 Pete Batard
                   ` (4 preceding siblings ...)
  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 ` Pete Batard
  2017-12-08 14:13 ` [PATCH v3 0/6] Add ARM support for VS2017 Gao, Liming
  6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2017-12-08 14:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: liming.gao

We duplicate the Assembly-Code-File section from build_rule.template
because --convert-hex cannot be used with the MSFT ARM assembler.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard <pete@akeo.ie>
---
 BaseTools/Conf/build_rule.template | 31 +++++++++++++++++++-
 BaseTools/Conf/tools_def.template  | 31 ++++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template
index 3e6aa8ff0f34..6ea14fb7aa02 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -182,7 +182,6 @@
 
     <Command>
 
-
 [Assembly-Code-File.COMMON.COMMON]
     <InputFile.MSFT, InputFile.INTEL, InputFile.RVCT>
         ?.asm, ?.Asm, ?.ASM
@@ -207,6 +206,36 @@
         # For RVCTCYGWIN ASM_FLAGS must be first to work around pathing issues
         "$(ASM)" $(ASM_FLAGS) -o ${dst} $(INC) ${d_path}(+)${s_base}.iii
 
+[Assembly-Code-File.COMMON.ARM]
+    # Remove --convert-hex for ARM as it breaks MSFT assemblers
+    <InputFile.MSFT, InputFile.INTEL, InputFile.RVCT>
+        ?.asm, ?.Asm, ?.ASM
+
+    <InputFile.GCC, InputFile.GCCLD>
+        ?.S, ?.s
+
+    <ExtraDependency>
+        $(MAKE_FILE)
+
+    <OutputFile>
+        $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
+
+    <Command.INTEL>
+        "$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
+        Trim --source-code --convert-hex --trim-long -o ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
+        "$(ASM)" /Fo${dst} $(ASM_FLAGS) /I${s_path} $(INC) ${d_path}(+)${s_base}.iii
+
+    <Command.MSFT>
+        "$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
+        Trim --source-code --trim-long -o ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
+        "$(ASM)" /Fo${dst} $(ASM_FLAGS) /I${s_path} $(INC) ${d_path}(+)${s_base}.iii
+
+    <Command.GCC, Command.GCCLD, Command.RVCT>
+        "$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
+        Trim --trim-long --source-code -o ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
+        # For RVCTCYGWIN ASM_FLAGS must be first to work around pathing issues
+        "$(ASM)" $(ASM_FLAGS) -o ${dst} $(INC) ${d_path}(+)${s_base}.iii
+
 [Nasm-Assembly-Code-File.COMMON.COMMON]
     <InputFile>
         ?.nasm
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 91b135c2e569..0bf30f05756f 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -79,6 +79,7 @@ DEFINE VS2017_HOST        = x86
 DEFINE VS2017_BIN_HOST    = DEF(VS2017_BIN)\HostDEF(VS2017_HOST)\DEF(VS2017_HOST)
 DEFINE VS2017_BIN_IA32    = DEF(VS2017_BIN)\HostDEF(VS2017_HOST)\x86
 DEFINE VS2017_BIN_X64     = DEF(VS2017_BIN)\HostDEF(VS2017_HOST)\x64
+DEFINE VS2017_BIN_ARM     = DEF(VS2017_BIN)\HostDEF(VS2017_HOST)\arm
 
 DEFINE WINSDK_BIN       = ENV(WINSDK_PREFIX)
 DEFINE WINSDKx86_BIN    = ENV(WINSDKx86_PREFIX)
@@ -335,6 +336,9 @@ DEFINE DTC_BIN                 = ENV(DTC_PREFIX)dtc
 #                             Required to build platforms or ACPI tables:
 #                               Intel(r) ACPI Compiler (iasl.exe) from
 #                               https://acpica.org/downloads
+#                        Note:
+#                             Building of XIP firmware images for ARM is not currently supported (only applications).
+#                             /FILEALIGN:4096 and other changes are needed for ARM firmware builds.
 #   DDK3790     -win32-  Requires:
 #                             Microsoft Windows Server 2003 Driver Development Kit (Microsoft WINDDK) version 3790.1830
 #                        Optional:
@@ -4169,6 +4173,33 @@ NOOPT_VS2017_X64_NASM_FLAGS     = -O0 -f win64 -g
 RELEASE_VS2017_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /IGNORE:4254 /OPT:REF /OPT:ICF=10 /MAP /ALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER /MERGE:.rdata=.data
 NOOPT_VS2017_X64_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /MAP /ALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER /DEBUG
 
+#################
+# ARM definitions
+#################
+*_VS2017_ARM_CC_PATH              = DEF(VS2017_BIN_ARM)\cl.exe
+*_VS2017_ARM_VFRPP_PATH           = DEF(VS2017_BIN_ARM)\cl.exe
+*_VS2017_ARM_SLINK_PATH           = DEF(VS2017_BIN_ARM)\lib.exe
+*_VS2017_ARM_DLINK_PATH           = DEF(VS2017_BIN_ARM)\link.exe
+*_VS2017_ARM_APP_PATH             = DEF(VS2017_BIN_ARM)\cl.exe
+*_VS2017_ARM_PP_PATH              = DEF(VS2017_BIN_ARM)\cl.exe
+*_VS2017_ARM_ASM_PATH             = DEF(VS2017_BIN_ARM)\armasm.exe
+*_VS2017_ARM_ASLCC_PATH           = DEF(VS2017_BIN_ARM)\cl.exe
+*_VS2017_ARM_ASLPP_PATH           = DEF(VS2017_BIN_ARM)\cl.exe
+*_VS2017_ARM_ASLDLINK_PATH        = DEF(VS2017_BIN_ARM)\link.exe
+
+      *_VS2017_ARM_MAKE_FLAGS     = /nologo
+  DEBUG_VS2017_ARM_CC_FLAGS       = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Gw /Oi-
+RELEASE_VS2017_ARM_CC_FLAGS       = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw /Oi-
+NOOPT_VS2017_ARM_CC_FLAGS         = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od /Oi-
+
+  DEBUG_VS2017_ARM_ASM_FLAGS      = /nologo /g
+RELEASE_VS2017_ARM_ASM_FLAGS      = /nologo
+NOOPT_VS2017_ARM_ASM_FLAGS        = /nologo
+
+  DEBUG_VS2017_ARM_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /MAP /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:ARM /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER /DEBUG
+RELEASE_VS2017_ARM_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /IGNORE:4254 /OPT:REF /OPT:ICF=10 /MAP /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:ARM /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER /MERGE:.rdata=.data
+NOOPT_VS2017_ARM_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /MAP /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:ARM /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER /DEBUG
+
 ##################
 # EBC definitions
 ##################
-- 
2.9.3.windows.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/6] Add ARM support for VS2017
  2017-12-08 14:06 [PATCH v3 0/6] Add ARM support for VS2017 Pete Batard
                   ` (5 preceding siblings ...)
  2017-12-08 14:06 ` [PATCH v3 6/6] BaseTools/Conf: Add VS2017/ARM support Pete Batard
@ 2017-12-08 14:13 ` Gao, Liming
  6 siblings, 0 replies; 12+ messages in thread
From: Gao, Liming @ 2017-12-08 14:13 UTC (permalink / raw)
  To: Pete Batard, edk2-devel@lists.01.org

Pete:
  Thanks for your update. I have no other comment on BaseTools and MdePkg part. Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Pete Batard [mailto:pete@akeo.ie]
> Sent: Friday, December 8, 2017 10:06 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH v3 0/6] Add ARM support for VS2017
> 
> This v3 is the same as v2, except for the following comment added in
> tools_def for VS2017:
> # Building of XIP firmware images for ARM is not currently supported (only applications).
> # /FILEALIGN:4096 and other changes are needed for ARM firmware builds.
> 
> The following series adds ARM compilation support for the VS2017 toolchain.
> * PATCH 1 targets the disabling of VS Level 4 warnings. The disabled warnings
>   for ARM are now aligned with IA32 and X64.
> * PATCH 2 adds a NULL handler for the base stack check, since this is a GCC
>   functionality.
> * PATCH 3 updates MdePkg/Library/BaseLib so that the RVCT assembly sources
>   are also used for MSFT.
> * PATCH 4 adds the required compiler intrinsics replacements for division,
>   shift and memset/memcpy.
> * PATCH 5 adds variable argument handlers for print output. Note that this
>   is done without relying on any external headers, with the VA_ARG macro
>   having been reverse engineered from MSFT ARM assembly output.
> * PATCH 6 enables the selection of ARM in the conf templates.
> 
> With these patches, VS2017 toolchain users should be able to compile
> regular UEFI ARM applications using EDK2. Note that, unlike ARM64 support,
> ARM support does not require a specific update of Visual Studio 2017, as
> the ARM toolchain has been available from the very first release.
> 
> Additional notes:
> 
> We tested compiling and running the full UEFI Shell with this series, as
> well as a small set of applications and drivers, and found no issues.
> With an additional patch [1], it is also possible to use this proposal to
> compile a complete QEMU ARM firmware. As the patch shows, the changes that
> need to be applied to the EDK2 sources to achieve this are actually very
> minimal.
> 
> However, the generated firmware does not currently boot, possibly because
> of the following warnings being generated by the MS compiler:
> - ArmCpuDxe.dll : warning LNK4072: section count 118 exceeds max (96); image may not run
> - UiApp.dll : warning LNK4072: section count 113 exceeds max (96); image may not run
> 
> As far as I could see, the section count max is hardcoded so a workaround
> would be needed to address those.
> 
> Also, because the VS2017 ARM compiler forces a section alignment of 4096
> bytes (which in turn forces use to use /FILEALIGN:4096 as a linker option
> for the firmware generation), the generated firmware exceeds 2MB and we
> had to double its size to 4MB.
> 
> At this stage, since the goal of this series is to allow users to compile
> regular ARM UEFI applications using the VS2017 toolchain, I have non plans
> to spend more time on the QEMU firmware issues, especially as I suspect
> that reducing the firmware size back to 2 MB may not be achievable without
> Microsoft altering their compiler. I am however hopeful that ARM
> specialists can take this matter over eventually...
> 
> Regards,
> 
> /Pete
> 
> [1] https://github.com/pbatard/edk2/commit/c4ce41094a46f4f3dc7ccc64a90604813f037b13
> 
> Pete Batard (6):
>   MdePkg: Disable some Level 4 warnings for VS2017/ARM
>   MdePkg/Library/BaseStackCheckLib: Add Null handler for VS2017/ARM
>   MdePkg/Library/BaseLib: Enable VS2017/ARM builds
>   ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
>   MdePkg/Include: Add VA list support for VS2017/ARM
>   BaseTools/Conf: Add VS2017/ARM support
> 
>  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 +++
>  BaseTools/Conf/build_rule.template                             |  31 ++-
>  BaseTools/Conf/tools_def.template                              |  31 +++
>  MdePkg/Include/Arm/ProcessorBind.h                             |  96 ++++++--
>  MdePkg/Include/Base.h                                          |  13 +
>  MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm                   |   5 +-
>  MdePkg/Library/BaseLib/BaseLib.inf                             |  16 +-
>  MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf         |   5 +-
>  MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c          |  18 ++
>  13 files changed, 565 insertions(+), 30 deletions(-)
>  create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm
>  create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm
>  create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c
>  create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c
>  create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
> 
> --
> 2.9.3.windows.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
  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
  1 sibling, 0 replies; 12+ messages in thread
From: Gao, Liming @ 2017-12-10 13:30 UTC (permalink / raw)
  To: Pete Batard, edk2-devel@lists.01.org
  Cc: leif.lindholm@linaro.org, ard.biesheuvel@linaro.org

Include ArmPkg maintainers for review. 

> -----Original Message-----
> From: Pete Batard [mailto:pete@akeo.ie]
> Sent: Friday, December 8, 2017 10:06 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
> 
> 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
> +
> +  EXPORT __rt_sdiv
> +  EXPORT __rt_udiv
> +  EXPORT __rt_udiv64
> +  EXPORT __rt_sdiv64
> +
> +  AREA  Math, CODE, READONLY
> +
> +_fltused
> +    dcd 0x9875
> +
> +__brkdiv0
> +    udf #249
> +
> +//
> +// uint64_t __rt_udiv(uint32_t divisor, uint32_t dividend)
> +//
> +
> +__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)
> +//
> +
> +__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);
> +//
> +
> +__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;
> +}
> --
> 2.9.3.windows.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
  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
  2017-12-13 13:56     ` Pete Batard
  1 sibling, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 19:59 UTC (permalink / raw)
  To: Pete Batard; +Cc: edk2-devel@lists.01.org, Gao, Liming

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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
  2017-12-12 19:59   ` Ard Biesheuvel
@ 2017-12-13 13:56     ` Pete Batard
  2017-12-14 14:05       ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Pete Batard @ 2017-12-13 13:56 UTC (permalink / raw)
  To: edk2-devel@lists.01.org; +Cc: Ard Biesheuvel, Gao, Liming

Hi Ard. Thanks for reviewing this.

On 2017.12.12 19:59, Ard Biesheuvel wrote:
>> +  EXPORT _fltused
>> +  EXPORT __brkdiv0
>> +
> 
> Why are these being exported? Are these part of the CRT ABI?

Good point. I exported them because I was encountering undefined symbols 
for those when I started working on adding ARM support. But I now 
suspect they may have come from an export statement from VS headers 
(which we no longer include), as re-test shows that the exports seem to 
be superfluous.

I will test further to confirm that this is the case, and remove the 
unneeded exports.

>> +__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.

Yes. I mostly copied what ReactOS was doing here, since it should be 
similar to what Microsoft does internally.

> Perhaps branch to a C function
> here that calls ASSERT() and CpuDeadLoop ?

I don't mind giving it a try, if we keep the separate .asm files for MSVC.

On the other hand, if I am to try to reuse existing assembly, as you 
suggest below, then I don't think I want to introduce anything that will 
differ from how divisions by zero are currently handled there. As far as 
I can tell (for RVCT, which is the only assembly that might be suitable 
for reuse with MSFT) the current way of handling a zero divisor is to 
set result and remainder to 0 and return (as per uldiv.asm). For gcc, we 
also have the following comment in div.S:

   "@ What to do about division by zero?  For now, just return."

So, while I agree that calling ASSERT and CpuDeadLoop is probably the 
better approach, I'd rather have the introduction of this behaviour be 
the subject of a separate ARM harmonization patch, that applies to all 
the toolchains, rather than something that's specific to Visual Studio 
usage.

> There are many different division implementations already in the same
> directory. Do we really need a new one?

I'll look into that. My goal with this series was to make sure that the 
changes being introduced would not break existing toolchains, which is 
why I saw it preferable to keep the MSVC intrinsics separate.

Now, gcc assembly is not something that can be reused easily with the MS 
toolchain, so the only possibility are the RVCT .asm files.

There's a fair bit of work involved into trying to figure out if and how 
we can reuse that code, but I'll see what I can do. It'll probably be a 
few weeks before I get back to this list on that however.

Regards,

/Pete


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds
  2017-12-13 13:56     ` Pete Batard
@ 2017-12-14 14:05       ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-12-14 14:05 UTC (permalink / raw)
  To: Pete Batard; +Cc: edk2-devel@lists.01.org, Gao, Liming

On 13 December 2017 at 14:56, Pete Batard <pete@akeo.ie> wrote:
> Hi Ard. Thanks for reviewing this.
>
> On 2017.12.12 19:59, Ard Biesheuvel wrote:
>>>
>>> +  EXPORT _fltused
>>> +  EXPORT __brkdiv0
>>> +
>>
>>
>> Why are these being exported? Are these part of the CRT ABI?
>
>
> Good point. I exported them because I was encountering undefined symbols for
> those when I started working on adding ARM support. But I now suspect they
> may have come from an export statement from VS headers (which we no longer
> include), as re-test shows that the exports seem to be superfluous.
>
> I will test further to confirm that this is the case, and remove the
> unneeded exports.
>
>>> +__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.
>
>
> Yes. I mostly copied what ReactOS was doing here, since it should be similar
> to what Microsoft does internally.
>
>> Perhaps branch to a C function
>> here that calls ASSERT() and CpuDeadLoop ?
>
>
> I don't mind giving it a try, if we keep the separate .asm files for MSVC.
>
> On the other hand, if I am to try to reuse existing assembly, as you suggest
> below, then I don't think I want to introduce anything that will differ from
> how divisions by zero are currently handled there. As far as I can tell (for
> RVCT, which is the only assembly that might be suitable for reuse with MSFT)
> the current way of handling a zero divisor is to set result and remainder to
> 0 and return (as per uldiv.asm). For gcc, we also have the following comment
> in div.S:
>
>   "@ What to do about division by zero?  For now, just return."
>
> So, while I agree that calling ASSERT and CpuDeadLoop is probably the better
> approach, I'd rather have the introduction of this behaviour be the subject
> of a separate ARM harmonization patch, that applies to all the toolchains,
> rather than something that's specific to Visual Studio usage.
>

I agree. Reusing code is more important than having one flavour of
division that will spot div-by-0. But we really don't want the UDF,
because it will crash unconditionally without a hint of what happened.

>> There are many different division implementations already in the same
>> directory. Do we really need a new one?
>
>
> I'll look into that. My goal with this series was to make sure that the
> changes being introduced would not break existing toolchains, which is why I
> saw it preferable to keep the MSVC intrinsics separate.
>
> Now, gcc assembly is not something that can be reused easily with the MS
> toolchain, so the only possibility are the RVCT .asm files.
>
> There's a fair bit of work involved into trying to figure out if and how we
> can reuse that code, but I'll see what I can do. It'll probably be a few
> weeks before I get back to this list on that however.
>

I'm prepared to be pragmatic here. I would prefer to have all
platforms run the same low level division code, but I understand that
it is tightly coupled to the compiler, given that they are intrinsics.

So yes, please try to reuse the RVCT code if feasible without a ton of
rework, but it seems like we will need to live with some MSFT specific
hooks anyway (if I understand the prototypes correctly)


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-12-14 14:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox