[PATCH] Makefile: Disable PLATFORM_LIBGCC for LLVM toolchain

[PATCH] Makefile: Disable PLATFORM_LIBGCC for LLVM toolchain

[Alistair Delva]

The LLVM toolchain does not have or need libgcc, so do not require
it to exist on the library path. Even if "-print-libgcc-file-name"
returned the empty string, -lgcc would be specified.

This leaves CONFIG_USE_PRIVATE_LIBGCC alone because I did not have
a target/toolchain combination available for testing.

Signed-off-by: Alistair Delva <adelva@google.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 8af67ebd63..af06b7aa19 100644
--- a/Makefile
+++ b/Makefile
@@ -874,8 +874,10 @@ u-boot-main := $(libs-y)
 ifeq ($(CONFIG_USE_PRIVATE_LIBGCC),y)
 PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
 else
+ifneq ($(cc-name),clang)
 PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
 endif
+endif
 PLATFORM_LIBS += $(PLATFORM_LIBGCC)
 
 ifdef CONFIG_CC_COVERAGE
-- 
2.30.2

Re: [PATCH] Makefile: Disable PLATFORM_LIBGCC for LLVM toolchain

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

On Mon, Sep 26, 2022 at 08:47:47PM +0000, Alistair Delva wrote:

> The LLVM toolchain does not have or need libgcc, so do not require
> it to exist on the library path. Even if "-print-libgcc-file-name"
> returned the empty string, -lgcc would be specified.
> 
> This leaves CONFIG_USE_PRIVATE_LIBGCC alone because I did not have
> a target/toolchain combination available for testing.
> 
> Signed-off-by: Alistair Delva <adelva@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 8af67ebd63..af06b7aa19 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -874,8 +874,10 @@ u-boot-main := $(libs-y)
>  ifeq ($(CONFIG_USE_PRIVATE_LIBGCC),y)
>  PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
>  else
> +ifneq ($(cc-name),clang)
>  PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
>  endif
> +endif
>  PLATFORM_LIBS += $(PLATFORM_LIBGCC)
>  
>  ifdef CONFIG_CC_COVERAGE

So this one isn't quite right, and will result in some platforms /
architectures just failing to build as the handful of functions we get
provided by CONFIG_USE_PRIVATE_LIBGCC end up being missing.

It's also the case that this is a badly named option, and something we
really should figure out how to remove and then always provide the
required functions for. What configuration did you end up having this
problem on exactly?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] Makefile: Disable PLATFORM_LIBGCC for LLVM toolchain

[Alistair Delva]

Hi Tom,

On Tue, Sep 27, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Sep 26, 2022 at 08:47:47PM +0000, Alistair Delva wrote:
>
> > The LLVM toolchain does not have or need libgcc, so do not require
> > it to exist on the library path. Even if "-print-libgcc-file-name"
> > returned the empty string, -lgcc would be specified.
> >
> > This leaves CONFIG_USE_PRIVATE_LIBGCC alone because I did not have
> > a target/toolchain combination available for testing.
> >
> > Signed-off-by: Alistair Delva <adelva@google.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  Makefile | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 8af67ebd63..af06b7aa19 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -874,8 +874,10 @@ u-boot-main := $(libs-y)
> >  ifeq ($(CONFIG_USE_PRIVATE_LIBGCC),y)
> >  PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
> >  else
> > +ifneq ($(cc-name),clang)
> >  PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
> >  endif
> > +endif
> >  PLATFORM_LIBS += $(PLATFORM_LIBGCC)
> >
> >  ifdef CONFIG_CC_COVERAGE
>
> So this one isn't quite right, and will result in some platforms /
> architectures just failing to build as the handful of functions we get
> provided by CONFIG_USE_PRIVATE_LIBGCC end up being missing.
>
> It's also the case that this is a badly named option, and something we
> really should figure out how to remove and then always provide the
> required functions for. What configuration did you end up having this
> problem on exactly?

We have CI set up for qemu_arm64_defconfig, qemu-x86_64_defconfig,
am57xx_evm_defconfig and rock-pi-4-rk3399_defconfig. I think all were
affected, since they don't use CONFIG_USE_PRIVATE_LIBGCC, so they all
hit this fallback.

AFAIK, for a long time, Android used an LLVM toolchain built with some
workarounds such as knowledge of a corresponding GCC-based toolchain,
so -print-libgcc-file-name could provide a path to libgcc.a. However,
our newer toolchains no longer do this and the symbols are either
automatically provided or provided by compiler-rt. In U-Boot's case, I
don't think our targets ever required libgcc but because the
fall-through expands to "-L . -lgcc" when -print-libgcc-file-name
returns the empty string, the link will fail because it will look for
libgcc but not find it.

Alistair.

Re: [PATCH] Makefile: Disable PLATFORM_LIBGCC for LLVM toolchain

[Nick Desaulniers]

On Tue, Sep 27, 2022 at 9:30 AM Alistair Delva <adelva@google.com> wrote:
>
> Hi Tom,
>
> On Tue, Sep 27, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Sep 26, 2022 at 08:47:47PM +0000, Alistair Delva wrote:
> >
> > > The LLVM toolchain does not have or need libgcc, so do not require
> > > it to exist on the library path. Even if "-print-libgcc-file-name"
> > > returned the empty string, -lgcc would be specified.
> > >
> > > This leaves CONFIG_USE_PRIVATE_LIBGCC alone because I did not have
> > > a target/toolchain combination available for testing.
> > >
> > > Signed-off-by: Alistair Delva <adelva@google.com>
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Cc: Tom Rini <trini@konsulko.com>
> > > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  Makefile | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 8af67ebd63..af06b7aa19 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -874,8 +874,10 @@ u-boot-main := $(libs-y)
> > >  ifeq ($(CONFIG_USE_PRIVATE_LIBGCC),y)
> > >  PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
> > >  else
> > > +ifneq ($(cc-name),clang)
> > >  PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
> > >  endif
> > > +endif
> > >  PLATFORM_LIBS += $(PLATFORM_LIBGCC)
> > >
> > >  ifdef CONFIG_CC_COVERAGE
> >
> > So this one isn't quite right, and will result in some platforms /
> > architectures just failing to build as the handful of functions we get
> > provided by CONFIG_USE_PRIVATE_LIBGCC end up being missing.

So sounds like based on the configs we build for Android, we don't
need the compiler runtime for ARCH=arm64 and ARCH=x86_64 (possibly
ARCH=arm).

LLVM's equivalent is called compiler-rt; `$ clang
--rtlib=compiler-rt`, which might need to be passed explicitly
depending on whether clang was configured at build time of clang
itself to use libgcc or compiler-rt as the implicit default --rtlib.

FWIW, the Linux kernel is a bit of a mess when it comes to compiler
runtime.  Some architecture ports require a compiler runtime, some
provide their own impl, but only for some symbols which makes
targeting them with any given compiler...interesting.

If these flags are going to the compiler as the driver, I wonder if
`--rtlib=libgcc` vs `--rtlib=none` might be more descriptive.

> >
> > It's also the case that this is a badly named option, and something we
> > really should figure out how to remove and then always provide the
> > required functions for. What configuration did you end up having this
> > problem on exactly?
>
> We have CI set up for qemu_arm64_defconfig, qemu-x86_64_defconfig,
> am57xx_evm_defconfig and rock-pi-4-rk3399_defconfig. I think all were
> affected, since they don't use CONFIG_USE_PRIVATE_LIBGCC, so they all
> hit this fallback.
>
> AFAIK, for a long time, Android used an LLVM toolchain built with some
> workarounds such as knowledge of a corresponding GCC-based toolchain,
> so -print-libgcc-file-name could provide a path to libgcc.a. However,
> our newer toolchains no longer do this and the symbols are either
> automatically provided or provided by compiler-rt. In U-Boot's case, I
> don't think our targets ever required libgcc but because the
> fall-through expands to "-L . -lgcc" when -print-libgcc-file-name
> returns the empty string, the link will fail because it will look for
> libgcc but not find it.
>
> Alistair.



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] Makefile: Disable PLATFORM_LIBGCC for LLVM toolchain

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3497 bytes --]

On Tue, Sep 27, 2022 at 09:29:51AM -0700, Alistair Delva wrote:
> Hi Tom,
> 
> On Tue, Sep 27, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Sep 26, 2022 at 08:47:47PM +0000, Alistair Delva wrote:
> >
> > > The LLVM toolchain does not have or need libgcc, so do not require
> > > it to exist on the library path. Even if "-print-libgcc-file-name"
> > > returned the empty string, -lgcc would be specified.
> > >
> > > This leaves CONFIG_USE_PRIVATE_LIBGCC alone because I did not have
> > > a target/toolchain combination available for testing.
> > >
> > > Signed-off-by: Alistair Delva <adelva@google.com>
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Cc: Tom Rini <trini@konsulko.com>
> > > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  Makefile | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 8af67ebd63..af06b7aa19 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -874,8 +874,10 @@ u-boot-main := $(libs-y)
> > >  ifeq ($(CONFIG_USE_PRIVATE_LIBGCC),y)
> > >  PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
> > >  else
> > > +ifneq ($(cc-name),clang)
> > >  PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
> > >  endif
> > > +endif
> > >  PLATFORM_LIBS += $(PLATFORM_LIBGCC)
> > >
> > >  ifdef CONFIG_CC_COVERAGE
> >
> > So this one isn't quite right, and will result in some platforms /
> > architectures just failing to build as the handful of functions we get
> > provided by CONFIG_USE_PRIVATE_LIBGCC end up being missing.
> >
> > It's also the case that this is a badly named option, and something we
> > really should figure out how to remove and then always provide the
> > required functions for. What configuration did you end up having this
> > problem on exactly?
> 
> We have CI set up for qemu_arm64_defconfig, qemu-x86_64_defconfig,
> am57xx_evm_defconfig and rock-pi-4-rk3399_defconfig. I think all were
> affected, since they don't use CONFIG_USE_PRIVATE_LIBGCC, so they all
> hit this fallback.
> 
> AFAIK, for a long time, Android used an LLVM toolchain built with some
> workarounds such as knowledge of a corresponding GCC-based toolchain,
> so -print-libgcc-file-name could provide a path to libgcc.a. However,
> our newer toolchains no longer do this and the symbols are either
> automatically provided or provided by compiler-rt. In U-Boot's case, I
> don't think our targets ever required libgcc but because the
> fall-through expands to "-L . -lgcc" when -print-libgcc-file-name
> returns the empty string, the link will fail because it will look for
> libgcc but not find it.

OK, that sounds familiar now that you say it, and part of why rpi_arm64
+ LLVM isn't part of my local testing loop. I believe I had been
thinking the best path forward was to port ashldi3.S and friends from
arch/arm64 in the Linux kernel over, so that USE_PRIVATE_LIBGCC (which
is a bad name!) could also be select'd on CONFIG_ARM64. That would just
leave x86, which just needs a div64 implementation for 64bit? I'm not
entirely sure.

All of that said, it might also be the case we don't need the fallback
to using libgcc at all, in most cases? If you're able to build and test
those platforms with it disabled, it might well be the case that we
just don't need the fallback anymore, and a world build with that just
removed would help figure out if that's true.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] Makefile: Disable PLATFORM_LIBGCC for LLVM toolchain

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4285 bytes --]

On Tue, Sep 27, 2022 at 12:41:15PM -0400, Tom Rini wrote:
> On Tue, Sep 27, 2022 at 09:29:51AM -0700, Alistair Delva wrote:
> > Hi Tom,
> > 
> > On Tue, Sep 27, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Sep 26, 2022 at 08:47:47PM +0000, Alistair Delva wrote:
> > >
> > > > The LLVM toolchain does not have or need libgcc, so do not require
> > > > it to exist on the library path. Even if "-print-libgcc-file-name"
> > > > returned the empty string, -lgcc would be specified.
> > > >
> > > > This leaves CONFIG_USE_PRIVATE_LIBGCC alone because I did not have
> > > > a target/toolchain combination available for testing.
> > > >
> > > > Signed-off-by: Alistair Delva <adelva@google.com>
> > > > Cc: Simon Glass <sjg@chromium.org>
> > > > Cc: Tom Rini <trini@konsulko.com>
> > > > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > > > ---
> > > >  Makefile | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 8af67ebd63..af06b7aa19 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -874,8 +874,10 @@ u-boot-main := $(libs-y)
> > > >  ifeq ($(CONFIG_USE_PRIVATE_LIBGCC),y)
> > > >  PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
> > > >  else
> > > > +ifneq ($(cc-name),clang)
> > > >  PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
> > > >  endif
> > > > +endif
> > > >  PLATFORM_LIBS += $(PLATFORM_LIBGCC)
> > > >
> > > >  ifdef CONFIG_CC_COVERAGE
> > >
> > > So this one isn't quite right, and will result in some platforms /
> > > architectures just failing to build as the handful of functions we get
> > > provided by CONFIG_USE_PRIVATE_LIBGCC end up being missing.
> > >
> > > It's also the case that this is a badly named option, and something we
> > > really should figure out how to remove and then always provide the
> > > required functions for. What configuration did you end up having this
> > > problem on exactly?
> > 
> > We have CI set up for qemu_arm64_defconfig, qemu-x86_64_defconfig,
> > am57xx_evm_defconfig and rock-pi-4-rk3399_defconfig. I think all were
> > affected, since they don't use CONFIG_USE_PRIVATE_LIBGCC, so they all
> > hit this fallback.
> > 
> > AFAIK, for a long time, Android used an LLVM toolchain built with some
> > workarounds such as knowledge of a corresponding GCC-based toolchain,
> > so -print-libgcc-file-name could provide a path to libgcc.a. However,
> > our newer toolchains no longer do this and the symbols are either
> > automatically provided or provided by compiler-rt. In U-Boot's case, I
> > don't think our targets ever required libgcc but because the
> > fall-through expands to "-L . -lgcc" when -print-libgcc-file-name
> > returns the empty string, the link will fail because it will look for
> > libgcc but not find it.
> 
> OK, that sounds familiar now that you say it, and part of why rpi_arm64
> + LLVM isn't part of my local testing loop. I believe I had been
> thinking the best path forward was to port ashldi3.S and friends from
> arch/arm64 in the Linux kernel over, so that USE_PRIVATE_LIBGCC (which
> is a bad name!) could also be select'd on CONFIG_ARM64. That would just
> leave x86, which just needs a div64 implementation for 64bit? I'm not
> entirely sure.
> 
> All of that said, it might also be the case we don't need the fallback
> to using libgcc at all, in most cases? If you're able to build and test
> those platforms with it disabled, it might well be the case that we
> just don't need the fallback anymore, and a world build with that just
> removed would help figure out if that's true.

OK, another new test build, a look at the kernel, and I see a better
path forward than it used to be at least. It would be best to:
- Port the kernel's current GENERIC_LIB_*DI3 functions
- Rename USE_PRIVATE_LIBGCC in U-Boot / drop it, depending on the
  location (arch/arm/lib/Makefile should just include ashldi3.o and
  friends on !ARM64).
- Port the kernel's arch/powerpc/boot/crtsavres.S for PowerPC which is
  fairly straight forward looking, even if you aren't a PowerPC person

This would be the way forward to drop trying to use libgcc directly, I
believe.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]