Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

["Ni, Ray" <ray.ni@intel.com>]

UEFI spec allows using float operation so asking module developers avoid using
float may not make sense. Even UefiCpuPkg\Library\BaseUefiCpuLib\ provides
routine to initialize float support for X86.

Given ARM already uses NULL library class mechanism to supply the compiler stub,
I prefer X86 aligns to the ARM style.

The only unsure thing is the size impact when a module is not using float. We expect
there is no size impact when a module is not using float.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D
> Kinney
> Sent: Thursday, April 2, 2020 12:38 AM
> To: devel@edk2.groups.io; ard.biesheuvel@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: lersek@redhat.com; macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for
> float.
> 
> Hi Ard,
> 
> I think adding a dependency in the module entry point
> libs is also good way to guarantee an intrinsic lib
> is available.  I agree that it can provide module type
> and arch specific mappings.  The NULL lib instance can
> do the same if the NULL instance is listed in the
> module/arch specific library classes sections. a few
> example section names.
> 
> [LibraryClasses.ARM.PEIM, LibraryClasses.AARCH64.PEIM]
> 
> [LibraryClasses.IA32.UEFI_DRIVER]
> 
> [LibraryClasses.X64.DXE_DRIVER]
> 
> So either way, the DSC files need to provide the
> library mapping.  The only difference between these
> 2 approaches is that adding a dependency to the
> module entry point libs uses a formally defined
> library class name for the intrinsic functions vs.
> the un-named NULL library instance.  A formally
> defined library class name is better supported for
> things like unit tests from the UnitTestFrameworkPkg.
> 
> The fltused issue would not go away of we removed the
> float usage for OpenSSL.  One or more other libs or the
> module could use float/double types and this issue
> would pop up again.  I do agree it would be cleaner
> if we could use OpenSSL without floating point.
> 
> I think adding an intrinsic lib for IA32/X64 for VS20xx
> generation of intrinsic functions would address fltused
> and other common C implementation styles that generate
> intrinsic functions (e.g. 64-bit int math on 32-bit,
> structure variable assignments, and loops that fill a buffer
> with a const value) by VS20xx compilers.  An intrinsic
> lib for GCC would also help with these same common C
> implementation styles that generate intrinsic functions.
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> > Behalf Of Ard Biesheuvel
> > Sent: Tuesday, March 31, 2020 11:43 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: devel@edk2.groups.io; lersek@redhat.com;
> > macarl@microsoft.com
> > Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> > Add FltUsedLib for float.
> >
> > On Tue, 31 Mar 2020 at 16:36, Kinney, Michael D
> > <michael.d.kinney@intel.com> wrote:
> > >
> > > ARM and AARCH64 have a compiler intrinsic lib that is
> > linked against all modules.
> > >
> > > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >   #
> > >   # It is not possible to prevent ARM compiler calls to
> > generic intrinsic functions.
> > >   # This library provides the instrinsic functions
> > generated by a given compiler.
> > >   # [LibraryClasses.ARM] and NULL mean link this
> > library into all ARM images.
> > >   #
> > >
> > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrins
> > icsLib.inf
> > >
> > > Can we use this same technique for IA32/X64 VS builds?
> > >
> >
> > In my opinion, having these intrinsics libraries added
> > via NULL
> > library class resolution was a mistake to begin with.
> >
> > Every component that we build incorporates some kind of
> > startup code
> > library that defines the _ModuleEntryPoint symbol, and it
> > would be
> > much better to make those libraries include an
> > IntrinsicsLibrary
> > dependency that can be satisfied by arch specific
> > versions that also
> > encapsulate the toolchain dependencies (such as this
> > _fltused symbol,
> > or memcpy/memset on ARM/GCC).
> >
> > On another note, it is still deeply disappointing that we
> > need to jump
> > through all of these hoops because the *only* single use
> > of floating
> > point in OpenSSL is the entropy estimate of an RNG, which
> > is in the
> > range of 0..1023 to begin with [IIRC). Remember that we
> > also have a
> > softfloat submodule for 32-bit ARM, for the same stupid
> > reason. If we
> > could stop pulling in that part of the code (or replace
> > it with an
> > UINT16 when building for the UEFI target), the whole
> > floating point
> > issue would mostly go away AFAICT.
> >
> >
> 
> 
>