[edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Oliver Smith-Denny]

CompilerIntrinsicsLib and ArmSoftFloatLib add ARM/AARCH64 compiler
intrinsics and floating point functions required by OpenSSL,
respectively. CompilerIntrinsicsLib is used almost in every DSC that
builds ARM/AARCH64 and ArmSoftFloatLib is used by every DSC that builds
logic from OpenSSL. Together these make almost every DSC have a
dependency on ArmPkg, which is odd and for a handful, MdeModulePkg,
EmbeddedPkg, and ShellPkg, namely, it is a circular dependency.

There have been previous mailing list suggestions to move
CompilerIntrinsicsLib to MdePkg (possibly combining with other arch
intrinsics). I am not sure the end status of those conversations. By
moving these two libraries to MdePkg package, we accomplish a few
things:

   - Removing the circular dependency from MdeModulePkg and ShellPkg
     (EmbeddedPkg has other ArmPkg dependencies)

   - Aligning MdePkg as the base package where baseline build and spec
     dependencies are found with other industry standard behavior

   - Detangling ArmPkg and making ARM/AARCH64 more of a first class
     citizen in edk2 instead of bolted onto the side

There is no functional change here and the amount of work is light, but
I think it moves edk2 in the direction it wants to go, so I'm happy to
put up a PR for this, but I wanted to get feedback before I did so. This
aligns with similar efforts, such as moving more ARM/AARCH64 chipset
definitions to MdePkg from ArmPkg [1][2][3]. This also aligns to the
overall goal of deleting ArmPkg:
https://bugzilla.tianocore.org/show_bug.cgi?id=4121

Thanks,
Oliver

[1] 
https://github.com/tianocore/edk2/commit/f2b9d5417dccf763bcbb68cd0effed0e25890aab
[2] 
https://github.com/tianocore/edk2/commit/cf323e2839ce260fde43487baae205527dee1b2f
[3] 
https://github.com/tianocore/edk2/commit/c68fb69dfefa7a76ebad33674a49632c4f8c6926


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120197): https://edk2.groups.io/g/devel/message/120197
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Michael D Kinney]

In order to make this a compatible change, look into
updating MdeLibs.dsc.inc too so the lib mappings 
to the new locations are invisible to DSC files that
include MdeLibs.dsc.inc.

Mike

> -----Original Message-----
> From: Oliver Smith-Denny <osde@linux.microsoft.com>
> Sent: Thursday, August 1, 2024 3:39 PM
> To: devel@edk2.groups.io; Ard Biesheuvel <ardb@kernel.org>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Sami Mujawar <sami.mujawar@arm.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> Subject: [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg
> 
> CompilerIntrinsicsLib and ArmSoftFloatLib add ARM/AARCH64 compiler
> intrinsics and floating point functions required by OpenSSL,
> respectively. CompilerIntrinsicsLib is used almost in every DSC that
> builds ARM/AARCH64 and ArmSoftFloatLib is used by every DSC that builds
> logic from OpenSSL. Together these make almost every DSC have a
> dependency on ArmPkg, which is odd and for a handful, MdeModulePkg,
> EmbeddedPkg, and ShellPkg, namely, it is a circular dependency.
> 
> There have been previous mailing list suggestions to move
> CompilerIntrinsicsLib to MdePkg (possibly combining with other arch
> intrinsics). I am not sure the end status of those conversations. By
> moving these two libraries to MdePkg package, we accomplish a few
> things:
> 
>    - Removing the circular dependency from MdeModulePkg and ShellPkg
>      (EmbeddedPkg has other ArmPkg dependencies)
> 
>    - Aligning MdePkg as the base package where baseline build and spec
>      dependencies are found with other industry standard behavior
> 
>    - Detangling ArmPkg and making ARM/AARCH64 more of a first class
>      citizen in edk2 instead of bolted onto the side
> 
> There is no functional change here and the amount of work is light, but
> I think it moves edk2 in the direction it wants to go, so I'm happy to
> put up a PR for this, but I wanted to get feedback before I did so. This
> aligns with similar efforts, such as moving more ARM/AARCH64 chipset
> definitions to MdePkg from ArmPkg [1][2][3]. This also aligns to the
> overall goal of deleting ArmPkg:
> https://bugzilla.tianocore.org/show_bug.cgi?id=4121
> 
> Thanks,
> Oliver
> 
> [1]
> https://github.com/tianocore/edk2/commit/f2b9d5417dccf763bcbb68cd0effed0e25
> 890aab
> [2]
> https://github.com/tianocore/edk2/commit/cf323e2839ce260fde43487baae205527d
> ee1b2f
> [3]
> https://github.com/tianocore/edk2/commit/c68fb69dfefa7a76ebad33674a49632c4f
> 8c6926


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120198): https://edk2.groups.io/g/devel/message/120198
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Oliver Smith-Denny]

On 8/1/2024 3:49 PM, Michael D Kinney wrote:
> In order to make this a compatible change, look into
> updating MdeLibs.dsc.inc too so the lib mappings
> to the new locations are invisible to DSC files that
> include MdeLibs.dsc.inc.
> 

Sure, that is a good call out, I can do so.

Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120199): https://edk2.groups.io/g/devel/message/120199
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Ard Biesheuvel]

On Fri, 2 Aug 2024 at 00:39, Oliver Smith-Denny
<osde@linux.microsoft.com> wrote:
>
> CompilerIntrinsicsLib and ArmSoftFloatLib add ARM/AARCH64 compiler
> intrinsics and floating point functions required by OpenSSL,
> respectively. CompilerIntrinsicsLib is used almost in every DSC that
> builds ARM/AARCH64 and ArmSoftFloatLib is used by every DSC that builds
> logic from OpenSSL.

What's even worse: OpenSSL doesn't really use floating point at all -
the only reason we need this is because there is some global double_t
somewhere that records entropy or somesuch, but IIRC that code is not
actually used.

So perhaps we could add a hack in OpensslLib to work around this in
one way or another for ARM only, and get rid of SoftFloatLib
altogether.

> Together these make almost every DSC have a
> dependency on ArmPkg, which is odd and for a handful, MdeModulePkg,
> EmbeddedPkg, and ShellPkg, namely, it is a circular dependency.
>
> There have been previous mailing list suggestions to move
> CompilerIntrinsicsLib to MdePkg (possibly combining with other arch
> intrinsics). I am not sure the end status of those conversations. By
> moving these two libraries to MdePkg package, we accomplish a few
> things:
>
>    - Removing the circular dependency from MdeModulePkg and ShellPkg
>      (EmbeddedPkg has other ArmPkg dependencies)
>
>    - Aligning MdePkg as the base package where baseline build and spec
>      dependencies are found with other industry standard behavior
>
>    - Detangling ArmPkg and making ARM/AARCH64 more of a first class
>      citizen in edk2 instead of bolted onto the side
>
> There is no functional change here and the amount of work is light, but
> I think it moves edk2 in the direction it wants to go, so I'm happy to
> put up a PR for this, but I wanted to get feedback before I did so. This
> aligns with similar efforts, such as moving more ARM/AARCH64 chipset
> definitions to MdePkg from ArmPkg [1][2][3]. This also aligns to the
> overall goal of deleting ArmPkg:
> https://bugzilla.tianocore.org/show_bug.cgi?id=4121
>

Intrinsics are tightly coupled with the toolchain, and so it would
make sense for the basetools to provide them. Intrinsics are also
-essentially- build once, run everywhere objects so I don't think we
need the typical handling for them either, and we could just build
them once when we build the basetools.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120210): https://edk2.groups.io/g/devel/message/120210
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Michael D Kinney]

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Friday, August 2, 2024 1:46 AM
> To: Oliver Smith-Denny <osde@linux.microsoft.com>
> Cc: devel@edk2.groups.io; Leif Lindholm <quic_llindhol@quicinc.com>; Sami
> Mujawar <sami.mujawar@arm.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>
> Subject: Re: [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg
> 
> On Fri, 2 Aug 2024 at 00:39, Oliver Smith-Denny
> <osde@linux.microsoft.com> wrote:
> >
> > CompilerIntrinsicsLib and ArmSoftFloatLib add ARM/AARCH64 compiler
> > intrinsics and floating point functions required by OpenSSL,
> > respectively. CompilerIntrinsicsLib is used almost in every DSC that
> > builds ARM/AARCH64 and ArmSoftFloatLib is used by every DSC that builds
> > logic from OpenSSL.
> 
> What's even worse: OpenSSL doesn't really use floating point at all -
> the only reason we need this is because there is some global double_t
> somewhere that records entropy or somesuch, but IIRC that code is not
> actually used.
> 
> So perhaps we could add a hack in OpensslLib to work around this in
> one way or another for ARM only, and get rid of SoftFloatLib
> altogether.
> 
> > Together these make almost every DSC have a
> > dependency on ArmPkg, which is odd and for a handful, MdeModulePkg,
> > EmbeddedPkg, and ShellPkg, namely, it is a circular dependency.
> >
> > There have been previous mailing list suggestions to move
> > CompilerIntrinsicsLib to MdePkg (possibly combining with other arch
> > intrinsics). I am not sure the end status of those conversations. By
> > moving these two libraries to MdePkg package, we accomplish a few
> > things:
> >
> >    - Removing the circular dependency from MdeModulePkg and ShellPkg
> >      (EmbeddedPkg has other ArmPkg dependencies)
> >
> >    - Aligning MdePkg as the base package where baseline build and spec
> >      dependencies are found with other industry standard behavior
> >
> >    - Detangling ArmPkg and making ARM/AARCH64 more of a first class
> >      citizen in edk2 instead of bolted onto the side
> >
> > There is no functional change here and the amount of work is light, but
> > I think it moves edk2 in the direction it wants to go, so I'm happy to
> > put up a PR for this, but I wanted to get feedback before I did so. This
> > aligns with similar efforts, such as moving more ARM/AARCH64 chipset
> > definitions to MdePkg from ArmPkg [1][2][3]. This also aligns to the
> > overall goal of deleting ArmPkg:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=4121
> >
> 
> Intrinsics are tightly coupled with the toolchain, and so it would
> make sense for the basetools to provide them. Intrinsics are also
> -essentially- build once, run everywhere objects so I don't think we
> need the typical handling for them either, and we could just build
> them once when we build the basetools.

I do not think this is possible

* BaseTools are built with the host compiler.
* FW is built with the target compiler which may be different than
  the host compiler.  And multiple target compilers or target compiler
  CPU target may be used from a single host build environment.  An
  intrinsic lib instance is needed for each unique target used.






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120212): https://edk2.groups.io/g/devel/message/120212
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Oliver Smith-Denny]

On 8/2/2024 1:46 AM, Ard Biesheuvel wrote:
> On Fri, 2 Aug 2024 at 00:39, Oliver Smith-Denny
> <osde@linux.microsoft.com> wrote:
>>
>> CompilerIntrinsicsLib and ArmSoftFloatLib add ARM/AARCH64 compiler
>> intrinsics and floating point functions required by OpenSSL,
>> respectively. CompilerIntrinsicsLib is used almost in every DSC that
>> builds ARM/AARCH64 and ArmSoftFloatLib is used by every DSC that builds
>> logic from OpenSSL.
> 
> What's even worse: OpenSSL doesn't really use floating point at all -
> the only reason we need this is because there is some global double_t
> somewhere that records entropy or somesuch, but IIRC that code is not
> actually used.
> 
> So perhaps we could add a hack in OpensslLib to work around this in
> one way or another for ARM only, and get rid of SoftFloatLib
> altogether.

I took a quick look at this. Unfortunately, I don't think this is
trivial. There are some uses of doubles within OpenSSL and it
doesn't look like we can just not build part of it, because
the first error I hit is that crypto/params.c uses fp operations:
https://github.com/openssl/openssl/blob/de90e54bbe82e5be4fb9608b6f5c308bb837d355/crypto/params.c#L762
and this provides a lot of definitions for other OpenSSL code. A
quick search turns up a number of other instances, so for now I will
go with moving the submodule, unless you want to take a look at
OpenSSL.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120237): https://edk2.groups.io/g/devel/message/120237
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Leif Lindholm]

Hi Oliver,

On 2024-08-01 23:39, Oliver Smith-Denny wrote:
> CompilerIntrinsicsLib and ArmSoftFloatLib add ARM/AARCH64 compiler
> intrinsics and floating point functions required by OpenSSL,
> respectively. CompilerIntrinsicsLib is used almost in every DSC that
> builds ARM/AARCH64 and ArmSoftFloatLib is used by every DSC that builds
> logic from OpenSSL. Together these make almost every DSC have a
> dependency on ArmPkg, which is odd and for a handful, MdeModulePkg,
> EmbeddedPkg, and ShellPkg, namely, it is a circular dependency.
> 
> There have been previous mailing list suggestions to move
> CompilerIntrinsicsLib to MdePkg (possibly combining with other arch
> intrinsics). I am not sure the end status of those conversations. By
> moving these two libraries to MdePkg package, we accomplish a few
> things:
> 
>    - Removing the circular dependency from MdeModulePkg and ShellPkg
>      (EmbeddedPkg has other ArmPkg dependencies)
> 
>    - Aligning MdePkg as the base package where baseline build and spec
>      dependencies are found with other industry standard behavior
> 
>    - Detangling ArmPkg and making ARM/AARCH64 more of a first class
>      citizen in edk2 instead of bolted onto the side
> 
> There is no functional change here and the amount of work is light, but
> I think it moves edk2 in the direction it wants to go, so I'm happy to
> put up a PR for this, but I wanted to get feedback before I did so. This
> aligns with similar efforts, such as moving more ARM/AARCH64 chipset
> definitions to MdePkg from ArmPkg [1][2][3]. This also aligns to the
> overall goal of deleting ArmPkg:
> https://bugzilla.tianocore.org/show_bug.cgi?id=4121

As you might expect, I'm a big fan of this.

Of course, there is still bikeshedding to be done.

For example, if we move ArmSoftFloatLib out of ArmPkg, Should it become 
SoftFloatLib? It's a pretty thin wrapper on the berkley softfloat library.

So yes, please, create the PR :)

Regards,

Leif

> Thanks,
> Oliver
> 
> [1] 
> https://github.com/tianocore/edk2/commit/f2b9d5417dccf763bcbb68cd0effed0e25890aab
> [2] 
> https://github.com/tianocore/edk2/commit/cf323e2839ce260fde43487baae205527dee1b2f
> [3] 
> https://github.com/tianocore/edk2/commit/c68fb69dfefa7a76ebad33674a49632c4f8c6926



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120222): https://edk2.groups.io/g/devel/message/120222
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Move CompilerIntrinsicsLib and ArmSoftFloatLib to MdePkg

[Oliver Smith-Denny]

On 8/5/2024 3:13 AM, Leif Lindholm wrote:
> 
> As you might expect, I'm a big fan of this.
> 
> Of course, there is still bikeshedding to be done.
> 
> For example, if we move ArmSoftFloatLib out of ArmPkg, Should it become 
> SoftFloatLib? It's a pretty thin wrapper on the berkley softfloat library.
> 
> So yes, please, create the PR :)
> 

Thanks, I'll put up the PR! I have no problems with renaming this
SoftFloatLib, so I'll start with that and then let the bikeshedding
go from there :).


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120238): https://edk2.groups.io/g/devel/message/120238
Mute This Topic: https://groups.io/mt/107675828/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-