Compilers insert inline calls to various functions implementing more complicated math operations or memory manipulation. These functions are implemented in the standard library normally shipped with compilers, but as we use a free-standing environment, these are not available on some architectures, causing various build issues with code using such instructions. An intrinsic library should be added to MdePkg with the most common intrinsics required. There is already an CompilerIntrinsicsLib in ArmPkg that implements all of these, but for ARM only. IntelUndiPkg (in edk2-staging) currently uses: - Multiplying: (U)INT64 * (U)INT64 (_allmul) - Dividing INT64 / INT64 (_alldiv) - Dividing UINT64 / UINT64 (_aulldiv) - Left Shift INT64 (_allshl) - Right Shift UINT64 (_aullshr) - Memory ops (memcpy, memset) CryptoPkg (in edk2) currently uses: - Left Shift INT64 (_allshl) - Right Shift UINT64 (_aullshr) - Memory ops (memcpy, memset, memcmp, strcmp)
There are also some intrinsics implemented in StdLib/LibC/CRT and wrappers around EDK2 functions in EmbeddedPkg/Include/libfdt_env.h.
Hi Ryszard, BaseLib already provides APIs for these operations explicitly, and the edk2 coding style requires programmers to explicitly use them (or to avoid the intrinsics by staying away from otherwise valid C constructs). Please refer to "MdePkg/Include/Library/BaseLib.h": - multiplication, division, and bit shifting expression, up to and including the UINTN / INTN types, can be written with the normal C operators. For INT64/UINT64, the functions are: - MultU64x32 - MultU64x64 - MultS64x64 - DivU64x32 - ModU64x32 - DivU64x32Remainder - DivU64x64Remainder - DivS64x64Remainder - LShiftU64 - RShiftU64 - ARShiftU64 - (rotation services and other bit manipulation skipped for now) - memcpy and memset intrinsics are supposed to be avoided by: - avoiding initialization of aggregate objects (structures, arrays) that have automatic storage duration ("locals") - avoiding structure assignment - explicitly calling CopyMem), SetMem*, ZeroMem from BaseMemoryLib instead [MdePkg/Include/Library/BaseMemoryLib.h]. - for comparison, - BaseMemoryLib offers CompareMem, CompareGuid. - BaseLib offers StrCmp, StrnCmp, AsciiStrCmp, AsciiStriCmp, AsciiStrnCmp. The only reason for having implemented some compiler intrinsics for ARM is that the corresponding compilers simply couldn't be convinced to stop generating calls to said intrinsics. It's not a feature, but a workaround. Please see the following commits: - e0c2cc6f8975 ("Fix Xcode, clang, and ARM build and link issues.", 2011-04-07) - 8e01b449de91 ("Patch from open source community for CryptoPkg to allow it to build for ARM using the RVCT toolchain.", 2011-04-19) - 11c20f4e06d2 ("Arm Packages: Fixed coding style/Line endings to follow EDK2 coding convention", 2011-09-22) - 262b7a311cbd ("FmpDevicePkg: Add DSC file to build all package components", 2018-08-02) - dd178f0f9391 ("IntelFrameworkModulePkg: fix build for AARCH64/ARM", 2019-02-04) - 2f0a1e6542fd ("SecurityPkg: fix package build on ARM", 2019-02-04) - 3b6c73f13eac ("SignedCapsulePkg: enable package build for AARCH64/ARM", 2019-02-04) They all add a NULL class resolution with the comment > # It is not possible to prevent the ARM compiler for generic intrinsic functions. > # This library provides the instrinsic functions generate by a given compiler. > # And NULL mean link this library into all ARM images. If IntelUndiPkg was genuinely written for edk2, in my opinion it should be updated to conform to the edk2 coding style. CryptoPkg has an excuse for "CryptoPkg/Library/IntrinsicLib" -- CryptoPkg wraps OpenSSL, which is an external project. I think we shouldn't add any intrinsics unless we are absolutely forced to. I do agree however that, for those intrinsics that we cannot at all avoid reimplementing, we should at least collect them in a common library. (In theory, I can also imagine reimplementing all possible intrinsics *if* the edk2 coding style spec / requirements are updated in parallel, permitting all new code to universally rely on the intrinsics, rather than the BaseLib / BaseMemoryLib functions.) Thanks!
Hi Laszlo, I've explained this on the mailing list, but basically IntelUndiPkg shares a large part of its code with other *nix drivers. They liberally use memcpy/memset, assign structures, use 64-bit math ops and so on - we can't convince them to wrap such operations, as everything they do is valid C (as you've noted, too). Right now a workaround is to simply ship a small library with needed intrinsics alongside IntelUndiPkg, but there's one here, one in CryptoPkg, also one needed for ARM builds... Simply having standard intrinsics + having coding standards allow these, without having to work around the lack of those would be much nicer, I think. Especially that this would simplify porting and sharing code with non-EDK2 components as well whenever needed. See related messages on edk2-devel: - https://lists.01.org/pipermail/edk2-devel/2019-January/036027.html - https://lists.01.org/pipermail/edk2-devel/2019-January/036032.html - https://lists.01.org/pipermail/edk2-devel/2019-February/036366.html
Thanks for the explanation. I think we should follow one of two paths. One path is that edk2 embeds an independent copy of the external code. In this case it should be adapted to the edk2 coding style. Repeated updates (code drops) from the external code base would be difficult, which is why such updates (code drops) should not be done in the first place. Maintenance would be manual (individual ports of the external changes). The other path is that edk2 should consume the external project as-is, via another git submodule. In this case stuff that gets "plugged" into the external code is acceptable IMO, although in some cases the external project would have to be willing to take patches -- not necessarily the kind of wrapping that we discussed above, but some kind of refactoring should be possible, so that edk2 can consume that project more easily. This has been successful with OpenSSL; over time we've dropped basically all of the edk2-specific customization (patches) for the upstream code itself. What I think is sub-optimal is a half-solution in the middle. Repeated upstream code drops (deep copies in the edk2 tree) combined with an ever changing plugin/wrapper library. I'm acutely aware that the drivers incorporated / wrapped by IntelUndiPkg are not the first such example in edk2. We have the LZMA SDK, Brotli, libfdt, Oniguruma, the list goes on and on. We should consume all those external projects via git submodules. Anyway, it's not my intent to block this work.
I understand the intent is to land with a clean solution, but one might not be easily possible at least in case of IntelUndiPkg. Let's say an external, not publicly accessible project accepts some patches, but still requires preprocessing/very ugly patching to build under EDK2. A submodule might be fine, but the result is that code drops would occur to a separate patched repository in parallel with regular commits to EDK2, which basically just adds a bit more complexity package maintainers have to work with, while not really providing any benefits. But then this is the result of some of our constraints, not what inclusion of most open libraries would have to deal with. Having submodules for external libs where possible is absolutely desired, and having a sensible intrinsics library would make including them easier, too. Although I'm not the best person to decide or have opinions on this, given the amount of my contributions in EDK2, hovering around 0 :) Feel free to pick any solution you think is the best for this.
discuss this one in the design meeting.
