edk2
edk2 copied to clipboard
MdePkg: Move CompilerIntrinsicsLib and ArmSoftFloatLib from ArmPkg
Description
As per the emailed RFC in https://edk2.groups.io/g/devel/topic/rfc_move/107675828, this patch moves CompilerIntrinsicsLib and ArmSoftFloatLib from ArmPkg to MdePkg as these libraries provide industry standard compiler intrinsics and ARM floating point functionality, respectively.
This aligns with the goal of integrating ArmPkg into existing packages: https://bugzilla.tianocore.org/show_bug.cgi?id=4121.
The newly placed libs are added to MdeLibs.dsc.inc as every DSC that builds ARM/AARCH64 needs CompilerIntrinsicsLib and every module that builds pieces of OpenSSL for ARM needs SoftFloatLib. The old location is removed from every DSC in edk2 in this commit also to not break bisectability with minimal hoop jumping.
Continuous-integration-options: PatchCheck.ignore-multi-package
- [x] Breaking change?
- Breaking change - Does this PR cause a break in build or boot behavior?
- Examples: Does it add a new library class or move a module to a different repo.
- [ ] Impacts security?
- Security - Does this PR have a direct security impact?
- Examples: Crypto algorithm change or buffer overflow fix.
- [ ] Includes tests?
- Tests - Does this PR include any explicit test code?
- Examples: Unit tests or integration tests.
How This Was Tested
No functional change.
Integration Instructions
Any DSCs referencing ArmSoftFloatLib or CompilerIntrinsicsLib should remove those lines and use the version specified in MdeLibs.dsc.inc.
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @javagedes
- @Zclarkwilliams
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
@mdkinney @leiflindholm @ardbiesheuvel following up from the RFC email I have put up this PR, please review when you have a chance. I did all of the moves in single commits for simplicity and not breaking bisectability. If you feel there is a better approach, please let me know.
When adding a compiler intrinsics library to MdePkg (which makes sense to me) we might move over the x86 bits in CryptoPkg/Library/IntrinsicLib too.
When adding a compiler intrinsics library to MdePkg (which makes sense to me) we might move over the x86 bits in CryptoPkg/Library/IntrinsicLib too.
That makes sense to me, too, although we may not want to make those intrinsics widely available. My understanding of the CryptoPkg InstrinsicLib is that is just to support OpenSSL, which obviously can't control the use of patterns that result in compiler intrinsics. For ARM/AARCH64, we do not have any way to avoid the compiler adding intrinsics, hence making this widely available. I'll let @mdkinney, @leiflindholm, and @ardbiesheuvel speak to that, though. If we do move over IntrinsicLib, I would propose to do that in a follow on patch, to not make this more complicated.
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @javagedes
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
PR can not be merged due to conflict. Please rebase and resubmit
@ardbiesheuvel @leiflindholm @mdkinney , I have dropped moving ArmSoftFloatLib in this PR (and updated the title and description) as Ard is removing the submodule entirely. Please review when you have a chance.
@lgao4 , I made your requested change, can you please re-review?
@mergify requeue
requeue