[PATCH u-boot 16/39] build: use thin archives instead of incremental linking

[Marek Behun]

On Tue, 9 Mar 2021 11:42:08 +0800
Bin Meng <bmeng.cn at gmail.com> wrote:

> Hi Marek,
> 
> On Tue, Mar 9, 2021 at 9:24 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Marek,
> >
> > On Mon, Mar 8, 2021 at 11:22 PM Marek Behún <marek.behun at nic.cz> wrote:  
> > >
> > > On Mon, 8 Mar 2021 22:30:17 +0800
> > > Bin Meng <bmeng.cn at gmail.com> wrote:
> > >  
> > > > Hi Marek,
> > > >
> > > > On Mon, Mar 8, 2021 at 9:24 PM Marek Behún <marek.behun at nic.cz> wrote:  
> > > > >
> > > > > On Mon, 8 Mar 2021 19:32:10 +0800
> > > > > Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > >  
> > > > > > On Mon, Mar 8, 2021 at 7:18 PM Marek Behun <marek.behun at nic.cz>
> > > > > > wrote:  
> > > > > > >
> > > > > > > On Mon, 8 Mar 2021 18:44:58 +0800
> > > > > > > Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > >  
> > > > > > > > Could you investigate why?  
> > > > > > >
> > > > > > > I could, but I don't understand why exactly I should
> > > > > > > - Linux is also using --whole-archive
> > > > > > > - it is working
> > > > > > > - I have other things I would like to work on
> > > > > > >
> > > > > > > Maybe you could look into this? :)  
> > > > > >
> > > > > > Yes, I can look into this. I wonder if you already knew this which
> > > > > > could save some time as this is a normal review process, asking
> > > > > > for clarifications if something isn't clear.  
> > > > >
> > > > > Bin, CI is failing without the --whole-archive option.
> > > > >
> > > > > What is interesting is that the binaries build successfully, but
> > > > > testing them fails :)
> > > > >
> > > > > You can try this (with and without the --whole-archive options)
> > > > > (note that this is without LTO)
> > > > >   make qemu_arm_defconfig
> > > > >   CROSS_COMPILE=arm-compiler- make -j8
> > > > >   qemu-system-arm -M virt -nographic \
> > > > >     -netdev user,id=net0,tftp=$(pwd) \
> > > > >     -device e1000,netdev=net0 -device virtio-rng-pci \
> > > > >     -bios u-boot.bin -serial mon:stdio
> > > > >
> > > > > With --whole-archive option this boots successfully into U-Boot,
> > > > > without --whole-archive it just hangs.  
> > > >
> > > > Thanks for reporting. My initnial build testing on qemu_arm_defconfig
> > > > with this series succeeded but there are some warnings:
> > > >
> > > > /opt/armv7-linux/bin/arm-linux-ld.bfd:
> > > > lib/efi_selftest/efi_selftest_miniapp_exception.o: plugin needed to
> > > > handle lto object
> > > > /opt/armv7-linux/bin/arm-linux-ld.bfd:
> > > > examples/standalone/hello_world.o: plugin needed to handle lto object
> > > > /opt/armv7-linux/bin/arm-linux-ld.bfd: examples/standalone/libstubs.o:
> > > > plugin needed to handle lto object
> > > > /opt/armv7-linux/bin/arm-linux-ld.bfd: warning: cannot find entry
> > > > symbol hello_world; defaulting to 000000000c100000
> > > >
> > > > It looks we should update the make rules to remove "-flto" for these
> > > > targets.
> > > >
> > > > Applying the following diff to remove --whole-archive, I got a build
> > > > error:
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 0f538270d7..127630e060 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1780,7 +1780,7 @@ quiet_cmd_u-boot__ ?= LTO     $@
> > > >                 $(LTO_FINAL_CFLAGS) $(c_flags)
> > > >          \
> > > >                 $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o
> > > > $@       \
> > > >                 -T u-boot.lds $(u-boot-init)
> > > >          \
> > > > -               -Wl,--start-group -Wl,--whole-archive
> > > >          \
> > > > +               -Wl,--start-group
> > > >          \
> > > >                         $(u-boot-main)
> > > >          \
> > > >                         $(PLATFORM_LIBS)
> > > >          \
> > > >                 -Wl,--no-whole-archive -Wl,--end-group
> > > >          \
> > > > @@ -1790,9 +1790,9 @@ else
> > > >  quiet_cmd_u-boot__ ?= LD      $@
> > > >        cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@
> > > >          \
> > > >                 -T u-boot.lds $(u-boot-init)
> > > >          \
> > > > -               --start-group --whole-archive
> > > >          \
> > > > +               --start-group
> > > >          \
> > > >                         $(u-boot-main)
> > > >          \
> > > > -               --no-whole-archive --end-group
> > > >          \
> > > > +               --end-group
> > > >          \
> > > >                 $(PLATFORM_LIBS) -Map u-boot.map;
> > > >          \
> > > >                 $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK)
> > > > $@, true) endif
> > > >
> > > >   LTO     u-boot
> > > > /opt/armv7-linux/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.3.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld:
> > > > arch/arm/lib/lib.a(lib1funcs.o): in function `Ldiv0':
> > > > arch/arm/lib/lib1funcs.S:360: undefined reference to `__div0'
> > > >
> > > > I am using a pre-built armv7 toolchain from
> > > > https://toolchains.bootlin.com/  
> > >
> > > The
> > >   plugin needed to handle lto object
> > > need another change in Makefiles, I will look into this.
> > >
> > > As for the __div0 availability, that happens when compiling with LTO
> > > and not using the --whole-archive flag.  
> >
> > Yes, that's something we need to investigate. Using "--whole-archive"
> > just masks the issue.  
> 
> Please ignore this. I think "--whole-archive" is correct after
> experimenting with the compiler.

So do you agree with this patch? Can I consider having your
Reviewed-by tag? :)