Date
1 - 7 of 7
[PATCH 0/3] [RFC] consolidate compiler intrinsics
|
Gerd Hoffmann
This is an attept to start cleaning up the messy compiler intrinsics
situation. Today we don't have a core intrinsics library, resulting in everybody creating their own. ArmPkg has one, CryptoPkg has one. I'm sure there are many more. This doesn't make sense. Given we can't avoid compiler intrinsics (as proven by the existence of those libraries) we should better have them as core library so we have to maintain a single version only. Given we already have BaseIoLibIntrinsic in MdePkg we can place the compiler intrinsics there too. This little patch series does just that: It moves over the existing ArmPkg intrinsics, fixes them to build on non-arm too, and adds additional bits from the CryptoPkg intrinsics. take care, Gerd Gerd Hoffmann (3): MdePkg: promote CompilerIntrinsicsLib from ArmPkg to MdePkg MdePkg/CompilerIntrinsicsLib: fix msft sources for x64 MdePkg/CompilerIntrinsicsLib: move ia32 intrinsics and strcmp ArmVirtPkg/ArmVirt.dsc.inc | 4 +- .../UnitTestFrameworkPkgTarget.dsc.inc | 2 +- ArmPkg/ArmPkg.dsc | 3 +- .../ArmCrashDumpDxe/ArmCrashDumpDxe.dsc | 2 +- ArmPlatformPkg/ArmPlatformPkg.dsc | 2 +- CryptoPkg/CryptoPkg.dsc | 5 +- DynamicTablesPkg/DynamicTablesPkg.dsc | 2 +- EmbeddedPkg/EmbeddedPkg.dsc | 2 +- EmulatorPkg/EmulatorPkg.dsc | 2 +- FatPkg/FatPkg.dsc | 2 +- FmpDevicePkg/FmpDevicePkg.dsc | 4 +- MdeModulePkg/MdeModulePkg.dsc | 2 +- MdePkg/MdePkg.dsc | 1 + NetworkPkg/NetworkPkg.dsc | 4 +- OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +- OvmfPkg/Bhyve/BhyveX64.dsc | 2 +- OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +- OvmfPkg/Microvm/MicrovmX64.dsc | 2 +- OvmfPkg/OvmfPkgIa32.dsc | 2 +- OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- OvmfPkg/OvmfPkgX64.dsc | 2 +- OvmfPkg/OvmfXen.dsc | 2 +- RedfishPkg/RedfishPkg.dsc | 2 +- SecurityPkg/SecurityPkg.dsc | 12 +-- ShellPkg/ShellPkg.dsc | 2 +- SignedCapsulePkg/SignedCapsulePkg.dsc | 14 ++-- StandaloneMmPkg/StandaloneMmPkg.dsc | 2 +- UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +- .../Library/IntrinsicLib/IntrinsicLib.inf | 67 ---------------- .../CompilerIntrinsicsLib.inf | 15 +++- {ArmPkg => MdePkg}/Include/AsmMacroIoLib.h | 0 CryptoPkg/Library/IntrinsicLib/CopyMem.c | 47 ----------- .../Library/IntrinsicLib/MemoryIntrinsics.c | 74 ------------------ .../CompilerIntrinsicsLib}/Ia32/MathFtol.c | 0 .../Ia32/MathLShiftS64.c | 0 .../Ia32/MathRShiftU64.c | 0 .../Library/CompilerIntrinsicsLib/memcmp_ms.c | 2 +- .../Library/CompilerIntrinsicsLib/memcpy.c | 0 .../Library/CompilerIntrinsicsLib/memcpy_ms.c | 2 +- .../CompilerIntrinsicsLib/memmove_ms.c | 2 +- .../Library/CompilerIntrinsicsLib/memset.c | 0 .../Library/CompilerIntrinsicsLib/memset_ms.c | 2 +- MdePkg/Library/CompilerIntrinsicsLib/strcmp.c | 33 ++++++++ ArmPkg/ArmPkg.ci.yaml | 1 - .../DEBUG_XCODE31/CompilerIntrinsicsLib.lib | Bin 36072 -> 36072 bytes .../DEBUG_XCODE32/CompilerIntrinsicsLib.lib | Bin 36072 -> 36072 bytes .../Library/IntrinsicLib/BaseIntrinsicLib.uni | 16 ---- .../CompilerIntrinsicsLib/AArch64/Atomics.S | 0 .../CompilerIntrinsicsLib/Arm/ashldi3.S | 0 .../CompilerIntrinsicsLib/Arm/ashrdi3.S | 0 .../CompilerIntrinsicsLib/Arm/clzsi2.S | 0 .../CompilerIntrinsicsLib/Arm/ctzsi2.S | 0 .../Library/CompilerIntrinsicsLib/Arm/div.S | 0 .../Library/CompilerIntrinsicsLib/Arm/div.asm | 0 .../CompilerIntrinsicsLib/Arm/divdi3.S | 0 .../CompilerIntrinsicsLib/Arm/divsi3.S | 0 .../Library/CompilerIntrinsicsLib/Arm/lasr.S | 0 .../CompilerIntrinsicsLib/Arm/lasr.asm | 0 .../CompilerIntrinsicsLib/Arm/ldivmod.S | 0 .../CompilerIntrinsicsLib/Arm/ldivmod.asm | 0 .../Library/CompilerIntrinsicsLib/Arm/llsl.S | 0 .../CompilerIntrinsicsLib/Arm/llsl.asm | 0 .../Library/CompilerIntrinsicsLib/Arm/llsr.S | 0 .../CompilerIntrinsicsLib/Arm/llsr.asm | 0 .../CompilerIntrinsicsLib/Arm/lshrdi3.S | 0 .../CompilerIntrinsicsLib/Arm/memmove.S | 0 .../CompilerIntrinsicsLib/Arm/memmove.asm | 0 .../CompilerIntrinsicsLib/Arm/moddi3.S | 0 .../CompilerIntrinsicsLib/Arm/modsi3.S | 0 .../CompilerIntrinsicsLib/Arm/muldi3.S | 0 .../Library/CompilerIntrinsicsLib/Arm/mullu.S | 0 .../CompilerIntrinsicsLib/Arm/mullu.asm | 0 .../CompilerIntrinsicsLib/Arm/sourcery.S | 0 .../CompilerIntrinsicsLib/Arm/switch.asm | 0 .../CompilerIntrinsicsLib/Arm/switch16.S | 0 .../CompilerIntrinsicsLib/Arm/switch32.S | 0 .../CompilerIntrinsicsLib/Arm/switch8.S | 0 .../CompilerIntrinsicsLib/Arm/switchu8.S | 0 .../CompilerIntrinsicsLib/Arm/ucmpdi2.S | 0 .../CompilerIntrinsicsLib/Arm/udivdi3.S | 0 .../CompilerIntrinsicsLib/Arm/udivmoddi4.S | 0 .../CompilerIntrinsicsLib/Arm/udivsi3.S | 0 .../Library/CompilerIntrinsicsLib/Arm/uldiv.S | 0 .../CompilerIntrinsicsLib/Arm/uldiv.asm | 0 .../CompilerIntrinsicsLib/Arm/umoddi3.S | 0 .../CompilerIntrinsicsLib/Arm/umodsi3.S | 0 .../Library/CompilerIntrinsicsLib/Arm/uread.S | 0 .../CompilerIntrinsicsLib/Arm/uread.asm | 0 .../CompilerIntrinsicsLib/Arm/uwrite.S | 0 .../CompilerIntrinsicsLib/Arm/uwrite.asm | 0 .../Ia32/MathLShiftS64.nasm | 0 .../Ia32/MathRShiftU64.nasm | 0 MdePkg/MdePkg.ci.yaml | 1 + 93 files changed, 95 insertions(+), 254 deletions(-) delete mode 100644 CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf (84%) rename {ArmPkg => MdePkg}/Include/AsmMacroIoLib.h (100%) delete mode 100644 CryptoPkg/Library/IntrinsicLib/CopyMem.c delete mode 100644 CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c rename {CryptoPkg/Library/IntrinsicLib => MdePkg/Library/CompilerIntrinsicsLib}/Ia32/MathFtol.c (100%) rename {CryptoPkg/Library/IntrinsicLib => MdePkg/Library/CompilerIntrinsicsLib}/Ia32/MathLShiftS64.c (100%) rename {CryptoPkg/Library/IntrinsicLib => MdePkg/Library/CompilerIntrinsicsLib}/Ia32/MathRShiftU64.c (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/memcmp_ms.c (89%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/memcpy.c (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/memcpy_ms.c (89%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/memmove_ms.c (90%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/memset.c (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/memset_ms.c (89%) create mode 100644 MdePkg/Library/CompilerIntrinsicsLib/strcmp.c delete mode 100644 CryptoPkg/Library/IntrinsicLib/BaseIntrinsicLib.uni rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/AArch64/Atomics.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/ashldi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/ashrdi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/clzsi2.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/ctzsi2.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/div.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/div.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/divdi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/divsi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/lasr.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/lasr.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/ldivmod.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/ldivmod.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/llsl.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/llsl.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/llsr.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/llsr.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/lshrdi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/memmove.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/memmove.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/moddi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/modsi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/muldi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/mullu.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/mullu.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/sourcery.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/switch.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/switch16.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/switch32.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/switch8.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/switchu8.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/ucmpdi2.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/udivdi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/udivmoddi4.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/udivsi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/uldiv.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/uldiv.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/umoddi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/umodsi3.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/uread.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/uread.asm (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/uwrite.S (100%) rename {ArmPkg => MdePkg}/Library/CompilerIntrinsicsLib/Arm/uwrite.asm (100%) rename {CryptoPkg/Library/IntrinsicLib => MdePkg/Library/CompilerIntrinsicsLib}/Ia32/MathLShiftS64.nasm (100%) rename {CryptoPkg/Library/IntrinsicLib => MdePkg/Library/CompilerIntrinsicsLib}/Ia32/MathRShiftU64.nasm (100%) -- 2.35.1
|
|
|
|
Ard Biesheuvel
On Tue, 1 Mar 2022 at 08:18, Gerd Hoffmann <kraxel@...> wrote:
For the series, Acked-by: Ard Biesheuvel <ardb@...> ArmVirtPkg/ArmVirt.dsc.inc | 4 +-
|
|
|
|
Gerd Hoffmann
On Tue, Mar 01, 2022 at 08:18:17AM +0100, Gerd Hoffmann wrote:
This is an attept to start cleaning up the messy compiler intrinsicsPing. How go forward with this? I've posted it as RFC because I expected a more controversial discussion. But seems everybody agrees this is a step into the right direction. So just merge this as-is? Yes, it passed CI (https://github.com/tianocore/edk2/pull/2542). take care, Gerd
|
|
|
|
Michael D Kinney
Hi Gerd,
toggle quoted messageShow quoted text
Thanks for working on this. I raised this topic in the new TianoCore build and CI meeting yesterday because the intrinsic libs are closely tied to the toolchains supported by TianoCore. I agree with the concept of moving the compiler intrinsics to a library in the MdePkg. + Sean Brogan Sean mentioned that he has some addition work on intrinsic libs for Visual Studio toolchains. It would be good to see if this set of changes is aligned with those efforts. I see a few items that might be good to clean up at some point * fltused should go into its own file. Not related to the strcmp() API * I see size_t defined in multiple places using different mappings. Would be good to define that in a single location. Best regards, Mike
-----Original Message-----
|
|
|
|
Pedro Falcato
Gerd, Have you considered just pulling in LLVM's compiler-rt builtins? It implements essentially everything we'll ever need for a GNU-like toolchain, for all the architectures we support, and also has unit tests (https://github.com/llvm/llvm-project/tree/main/compiler-rt/test/builtins/Unit). I think it would be a good idea considering it's guaranteed to be correct and always implements whatever clang requires (and AFAIK clang always requires what gcc requires). Best regards, Pedro
On Tue, Mar 15, 2022 at 12:23 PM Gerd Hoffmann <kraxel@...> wrote: On Tue, Mar 01, 2022 at 08:18:17AM +0100, Gerd Hoffmann wrote: --
Pedro Falcato
|
|
|
|
Gerd Hoffmann
On Tue, Mar 15, 2022 at 03:27:58PM +0000, Michael D Kinney wrote:
Hi Gerd,Having a single place where all compiler intrinsics are located should simplify this kind of changes. I see a few items that might be good to clean up at some pointYes, sure. Both are valid points. I see this series as first step, consolidating the existing code in a singe place with (almost) no actual code changes. There surely will be incremental improvements and cleanups later on. I will need a few more intrinsic functions when moving edk2 to openssl 3.0 for example. take care, Gerd
|
|
|
|
Gerd Hoffmann
On Tue, Mar 15, 2022 at 05:08:55PM +0000, Pedro Falcato wrote:
Gerd,This is beyond the scope of this patch series, I want have a single intrinsics lib for everybody as first step. Using llvm's intrinsics certainly is an option for future improvements, assuming it is ok license-wise (edk2 is bsd, llvm compiler-rt is apache, as far I know they are compatible). take care, Gerd
|
|
|