[PATCH] ArmPkg/ArmSoftFloatLib GCC4x: fix build failure

[PATCH] ArmPkg/ArmSoftFloatLib GCC4x: fix build failure

[Ard Biesheuvel]

The upstream SoftFloat code that was recently incorporated into
ArmSoftFloatLib uses some parameterization to tweak the inlining
and optimization behavior for different compilers.

The custom platform.h file that sets these parameters is based on
the upstream version for Linux/ARM, but was updated to include the
'always_inline' GCC attribute into the INLINE macro, to ensure that
all definitions that are marked as inline are not only inlined into
their callers, but also to ensure that no version of the function
is ever emitted into the object file.

This works fine on recent GCC and Clang, but the latter part turns
out to break on GCC 4.x, resulting in duplicate definition linker
errors. Fortunately, the synticatically more appriopriate 'static
inline' works fine on both the recent and the older compilers, so
let's switch to that instead.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: "Gao, Liming" <liming.gao@intel.com>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/ArmSoftFloatLib/platform.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmSoftFloatLib/platform.h b/ArmPkg/Library/ArmSoftFloatLib/platform.h
index 31e843463a38..07800a9d5b79 100644
--- a/ArmPkg/Library/ArmSoftFloatLib/platform.h
+++ b/ArmPkg/Library/ArmSoftFloatLib/platform.h
@@ -5,7 +5,7 @@
  */
 
 #define LITTLEENDIAN 1
-#define INLINE inline __attribute__((always_inline))
+#define INLINE static inline
 #define SOFTFLOAT_BUILTIN_CLZ 1
 #define SOFTFLOAT_FAST_INT64
 #include "opts-GCC.h"
-- 
2.20.1

Re: [PATCH] ArmPkg/ArmSoftFloatLib GCC4x: fix build failure

[Leif Lindholm]

On Fri, May 31, 2019 at 11:01:15PM +0200, Ard Biesheuvel wrote:
> The upstream SoftFloat code that was recently incorporated into
> ArmSoftFloatLib uses some parameterization to tweak the inlining
> and optimization behavior for different compilers.
> 
> The custom platform.h file that sets these parameters is based on
> the upstream version for Linux/ARM, but was updated to include the
> 'always_inline' GCC attribute into the INLINE macro, to ensure that
> all definitions that are marked as inline are not only inlined into
> their callers, but also to ensure that no version of the function
> is ever emitted into the object file.
> 
> This works fine on recent GCC and Clang, but the latter part turns
> out to break on GCC 4.x, resulting in duplicate definition linker
> errors. Fortunately, the synticatically more appriopriate 'static
> inline' works fine on both the recent and the older compilers, so
> let's switch to that instead.

Oh yeah, I knew that, and completely missed it when reviewing.

This behaviour hasn't actually changed substantially in later versions
of GCC. I bet all that's saving us there is LTO :)

> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: "Gao, Liming" <liming.gao@intel.com>
> Cc: "Wang, Jian J" <jian.j.wang@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/Library/ArmSoftFloatLib/platform.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ArmPkg/Library/ArmSoftFloatLib/platform.h b/ArmPkg/Library/ArmSoftFloatLib/platform.h
> index 31e843463a38..07800a9d5b79 100644
> --- a/ArmPkg/Library/ArmSoftFloatLib/platform.h
> +++ b/ArmPkg/Library/ArmSoftFloatLib/platform.h
> @@ -5,7 +5,7 @@
>   */
>  
>  #define LITTLEENDIAN 1
> -#define INLINE inline __attribute__((always_inline))
> +#define INLINE static inline
>  #define SOFTFLOAT_BUILTIN_CLZ 1
>  #define SOFTFLOAT_FAST_INT64
>  #include "opts-GCC.h"
> -- 
> 2.20.1
> 

Re: [PATCH] ArmPkg/ArmSoftFloatLib GCC4x: fix build failure

[Ard Biesheuvel]

On Fri, 31 May 2019 at 23:12, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, May 31, 2019 at 11:01:15PM +0200, Ard Biesheuvel wrote:
> > The upstream SoftFloat code that was recently incorporated into
> > ArmSoftFloatLib uses some parameterization to tweak the inlining
> > and optimization behavior for different compilers.
> >
> > The custom platform.h file that sets these parameters is based on
> > the upstream version for Linux/ARM, but was updated to include the
> > 'always_inline' GCC attribute into the INLINE macro, to ensure that
> > all definitions that are marked as inline are not only inlined into
> > their callers, but also to ensure that no version of the function
> > is ever emitted into the object file.
> >
> > This works fine on recent GCC and Clang, but the latter part turns
> > out to break on GCC 4.x, resulting in duplicate definition linker
> > errors. Fortunately, the synticatically more appriopriate 'static
> > inline' works fine on both the recent and the older compilers, so
> > let's switch to that instead.
>
> Oh yeah, I knew that, and completely missed it when reviewing.
>
> This behaviour hasn't actually changed substantially in later versions
> of GCC. I bet all that's saving us there is LTO :)
>

Not sure. LTO is disabled entirely for this library, since it
interoperates poorly with intrinsics.

> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: "Gao, Liming" <liming.gao@intel.com>
> > Cc: "Wang, Jian J" <jian.j.wang@intel.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks

Pushed as 8594c2073cdb..371e7001e8d5


> > ---
> >  ArmPkg/Library/ArmSoftFloatLib/platform.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ArmPkg/Library/ArmSoftFloatLib/platform.h b/ArmPkg/Library/ArmSoftFloatLib/platform.h
> > index 31e843463a38..07800a9d5b79 100644
> > --- a/ArmPkg/Library/ArmSoftFloatLib/platform.h
> > +++ b/ArmPkg/Library/ArmSoftFloatLib/platform.h
> > @@ -5,7 +5,7 @@
> >   */
> >
> >  #define LITTLEENDIAN 1
> > -#define INLINE inline __attribute__((always_inline))
> > +#define INLINE static inline
> >  #define SOFTFLOAT_BUILTIN_CLZ 1
> >  #define SOFTFLOAT_FAST_INT64
> >  #include "opts-GCC.h"
> > --
> > 2.20.1
> >