Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Sat, 2019-07-13 at 03:21:06 UTC, Masahiro Yamada wrote:
> The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> in a useful way because it is always overridden by the following code
> in the top Makefile:
>
> # use the deterministic mode of AR if available
> KBUILD_ARFLAGS := $(call ar-option,D)
>
> The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> ("kbuild: Fix build with binutils <= 2.19").
>
> The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> beginning.
>
> Nobody has reported a problem since 43c9127d94d6 ("powerpc: Add option
> to use thin archives"), so this code was unneeded.
>
> Signed-off-by: Masahiro Yamada
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/56347074c5307d7bca5db38eb2c764c64ae57514
cheers
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
Hi, On Fri, Jul 19, 2019 at 12:43 PM Michael Ellerman wrote: > > Segher Boessenkool writes: > > On Thu, Jul 18, 2019 at 11:19:58AM +0900, Masahiro Yamada wrote: > >> On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool > >> wrote: > >> Kbuild always uses thin archives as far as vmlinux is concerned. > >> > >> But, there are some other call-sites. > >> > >> masahiro@pug:~/ref/linux$ git grep '$(AR)' -- :^Documentation :^tools > >> arch/powerpc/boot/Makefile:BOOTAR := $(AR) > >> arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@ > >> arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > > >> $@ > >> lib/raid6/test/Makefile: $(AR) cq $@ $^ > >> scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1) > >> "$$TMP",$(1),$(2)) > >> scripts/Makefile.build: cmd_ar_builtin = rm -f $@; $(AR) > >> rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) > >> scripts/Makefile.lib: cmd_ar = rm -f $@; $(AR) > >> rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) > >> > >> Probably, you are interested in arch/powerpc/boot/Makefile. > > > > That one seems fine actually. The raid6 one I don't know. > > > > > > My original commit message was > > > > Without this, some versions of GNU ar fail to create > > an archive index if the object files it is packing > > together are of a different object format than ar's > > default format (for example, binutils compiled to > > default to 64-bit, with 32-bit objects). > > > > but I cannot reproduce the problem anymore. Shortly after my patch the > > thin archive code happened to binutils, and that overhauled some other > > things, which might have fixed it already? > > > >> > Yes, I know. This isn't about built-in.[oa], it is about *other* > >> > archives we at least *used to* create. If we *know* we do not anymore, > >> > then this workaround can of course be removed (and good riddance). > >> > >> If it is not about built-in.[oa], > >> which archive are you talking about? > >> > >> Can you pin-point the one? > > > > No, not anymore. Lost in the mists of time, I guess? I think we'll > > just have to file it as "it seems to work fine now". > > Yeah I think so. If someone finds a case it breaks we can fix it then. > > > Thank you (and everyone else) for the time looking at this! > > Likewise. > > cheers So, we agreed to apply this patch, right? Please let me know if there is some improvement that should be get done. -- Best Regards Masahiro Yamada
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
Segher Boessenkool writes: > On Thu, Jul 18, 2019 at 11:19:58AM +0900, Masahiro Yamada wrote: >> On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool >> wrote: >> Kbuild always uses thin archives as far as vmlinux is concerned. >> >> But, there are some other call-sites. >> >> masahiro@pug:~/ref/linux$ git grep '$(AR)' -- :^Documentation :^tools >> arch/powerpc/boot/Makefile:BOOTAR := $(AR) >> arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@ >> arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > $@ >> lib/raid6/test/Makefile: $(AR) cq $@ $^ >> scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1) >> "$$TMP",$(1),$(2)) >> scripts/Makefile.build: cmd_ar_builtin = rm -f $@; $(AR) >> rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) >> scripts/Makefile.lib: cmd_ar = rm -f $@; $(AR) >> rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) >> >> Probably, you are interested in arch/powerpc/boot/Makefile. > > That one seems fine actually. The raid6 one I don't know. > > > My original commit message was > > Without this, some versions of GNU ar fail to create > an archive index if the object files it is packing > together are of a different object format than ar's > default format (for example, binutils compiled to > default to 64-bit, with 32-bit objects). > > but I cannot reproduce the problem anymore. Shortly after my patch the > thin archive code happened to binutils, and that overhauled some other > things, which might have fixed it already? > >> > Yes, I know. This isn't about built-in.[oa], it is about *other* >> > archives we at least *used to* create. If we *know* we do not anymore, >> > then this workaround can of course be removed (and good riddance). >> >> If it is not about built-in.[oa], >> which archive are you talking about? >> >> Can you pin-point the one? > > No, not anymore. Lost in the mists of time, I guess? I think we'll > just have to file it as "it seems to work fine now". Yeah I think so. If someone finds a case it breaks we can fix it then. > Thank you (and everyone else) for the time looking at this! Likewise. cheers
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
Hi! On Thu, Jul 18, 2019 at 11:19:58AM +0900, Masahiro Yamada wrote: > On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool > wrote: > Kbuild always uses thin archives as far as vmlinux is concerned. > > But, there are some other call-sites. > > masahiro@pug:~/ref/linux$ git grep '$(AR)' -- :^Documentation :^tools > arch/powerpc/boot/Makefile:BOOTAR := $(AR) > arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@ > arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > $@ > lib/raid6/test/Makefile: $(AR) cq $@ $^ > scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1) > "$$TMP",$(1),$(2)) > scripts/Makefile.build: cmd_ar_builtin = rm -f $@; $(AR) > rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) > scripts/Makefile.lib: cmd_ar = rm -f $@; $(AR) > rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) > > Probably, you are interested in arch/powerpc/boot/Makefile. That one seems fine actually. The raid6 one I don't know. My original commit message was Without this, some versions of GNU ar fail to create an archive index if the object files it is packing together are of a different object format than ar's default format (for example, binutils compiled to default to 64-bit, with 32-bit objects). but I cannot reproduce the problem anymore. Shortly after my patch the thin archive code happened to binutils, and that overhauled some other things, which might have fixed it already? > > Yes, I know. This isn't about built-in.[oa], it is about *other* > > archives we at least *used to* create. If we *know* we do not anymore, > > then this workaround can of course be removed (and good riddance). > > If it is not about built-in.[oa], > which archive are you talking about? > > Can you pin-point the one? No, not anymore. Lost in the mists of time, I guess? I think we'll just have to file it as "it seems to work fine now". Thank you (and everyone else) for the time looking at this! Segher
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool wrote: > > On Thu, Jul 18, 2019 at 12:19:36AM +0900, Masahiro Yamada wrote: > > On Wed, Jul 17, 2019 at 11:38 PM Segher Boessenkool > > wrote: > > > > > > On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote: > > > > Segher Boessenkool writes: > > > > And it's definitely calling ar with no flags, eg: > > > > > > > > rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a > > > > init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o > > > > init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o > > > > init/init_task.o > > > > > > This uses thin archives. Those will work fine. > > > > > > The failing case was empty files IIRC, stuff created from no inputs. > > > > Actually, empty files are created everywhere. > > >cmd_ar_builtin = rm -f $@; $(AR) rcSTP$(KBUILD_ARFLAGS) $@ > > $(real-prereqs) > > You use thin archives. > > Does every config use thin archives always nowadays? Kbuild always uses thin archives as far as vmlinux is concerned. But, there are some other call-sites. masahiro@pug:~/ref/linux$ git grep '$(AR)' -- :^Documentation :^tools arch/powerpc/boot/Makefile:BOOTAR := $(AR) arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@ arch/unicore32/lib/Makefile:$(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > $@ lib/raid6/test/Makefile: $(AR) cq $@ $^ scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1) "$$TMP",$(1),$(2)) scripts/Makefile.build: cmd_ar_builtin = rm -f $@; $(AR) rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) scripts/Makefile.lib: cmd_ar = rm -f $@; $(AR) rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) Probably, you are interested in arch/powerpc/boot/Makefile. This does not seem a thin archive. > > BTW, your commit 8995ac8702737147115e1c75879a1a2d75627b9e > > dates back to 2008. > > > > At that time, thin archive was not used. > > Yes, I know. This isn't about built-in.[oa], it is about *other* > archives we at least *used to* create. If we *know* we do not anymore, > then this workaround can of course be removed (and good riddance). If it is not about built-in.[oa], which archive are you talking about? Can you pin-point the one? masahiro@pug:~/ref/linux$ git log --oneline -1 8995ac870273 (HEAD) [POWERPC] Specify GNUTARGET on $(AR) invocations masahiro@pug:~/ref/linux$ git grep '(AR)' Documentation/kbuild/makefiles.txt: per-directory options to $(LD) and $(AR). arch/powerpc/Makefile:CROSS32AR := GNUTARGET=elf32-powerpc $(AR) arch/powerpc/Makefile:override AR := GNUTARGET=elf$(SZ)-powerpc $(AR) drivers/md/raid6test/Makefile: $(AR) cq $@ $^ scripts/Makefile.build: rm -f $@; $(AR) rcs $@) scripts/Makefile.build:cmd_link_l_target = rm -f $@; $(AR) $(EXTRA_ARFLAGS) rcs $@ $(lib-y) masahiro@pug:~/ref/linux$ git grep '(CROSS32AR)' arch/powerpc/boot/Makefile: cmd_bootar = $(CROSS32AR) -cr $@. $(filter-out FORCE,$^); mv $@. $@ > If ar creates an archive file (a real one, not a thin archive), and it > has no input files, it uses its default object format as destination > format, if it isn't told to use something else. And that doesn't work, > it needs to use some format compatible with what that archive later is > linked with. I compile-tested v4.10, which was before the thin-archive migration, but I did not see any problem for building ppc32/64. Whether or not it is a thin archive, an empty archive is always 8-byte file. masahiro@pug:~/ref/linux$ cat kernel/livepatch/built-in.o ! Is there a room for caring about the under-lying architecture? -- Best Regards Masahiro Yamada
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Thu, Jul 18, 2019 at 12:19:36AM +0900, Masahiro Yamada wrote: > On Wed, Jul 17, 2019 at 11:38 PM Segher Boessenkool > wrote: > > > > On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote: > > > Segher Boessenkool writes: > > > And it's definitely calling ar with no flags, eg: > > > > > > rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a > > > init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o > > > init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o > > > init/init_task.o > > > > This uses thin archives. Those will work fine. > > > > The failing case was empty files IIRC, stuff created from no inputs. > > Actually, empty files are created everywhere. >cmd_ar_builtin = rm -f $@; $(AR) rcSTP$(KBUILD_ARFLAGS) $@ > $(real-prereqs) You use thin archives. Does every config use thin archives always nowadays? > BTW, your commit 8995ac8702737147115e1c75879a1a2d75627b9e > dates back to 2008. > > At that time, thin archive was not used. Yes, I know. This isn't about built-in.[oa], it is about *other* archives we at least *used to* create. If we *know* we do not anymore, then this workaround can of course be removed (and good riddance). If ar creates an archive file (a real one, not a thin archive), and it has no input files, it uses its default object format as destination format, if it isn't told to use something else. And that doesn't work, it needs to use some format compatible with what that archive later is linked with. Segher
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Wed, Jul 17, 2019 at 11:38 PM Segher Boessenkool wrote: > > On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote: > > Segher Boessenkool writes: > > And it's definitely calling ar with no flags, eg: > > > > rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a > > init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o > > init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o > > init/init_task.o > > This uses thin archives. Those will work fine. > > The failing case was empty files IIRC, stuff created from no inputs. Actually, empty files are created everywhere. I do not see any problems whether the target is 32-bit or 64-bit. Just try allnoconfig, for example. You can apply the following to debug. diff --git a/scripts/Makefile.build b/scripts/Makefile.build index be38198d98b2..9d6b30e50663 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -413,6 +413,7 @@ quiet_cmd_ar_builtin = AR $@ cmd_ar_builtin = rm -f $@; $(AR) rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs) $(builtin-target): $(real-obj-y) FORCE + @$(if $(real-prereqs),,echo creating empty archive: $@) $(call if_changed,ar_builtin) targets += $(builtin-target) The following are all empty files created from no input. creating empty archive: usr/built-in.a creating empty archive: arch/powerpc/crypto/built-in.a creating empty archive: arch/powerpc/math-emu/built-in.a CHK include/generated/compile.h creating empty archive: arch/powerpc/net/built-in.a creating empty archive: arch/powerpc/platforms/built-in.a creating empty archive: arch/powerpc/sysdev/built-in.a creating empty archive: certs/built-in.a creating empty archive: ipc/built-in.a creating empty archive: block/built-in.a creating empty archive: drivers/amba/built-in.a creating empty archive: sound/built-in.a creating empty archive: drivers/auxdisplay/built-in.a creating empty archive: crypto/built-in.a creating empty archive: drivers/block/built-in.a creating empty archive: net/built-in.a creating empty archive: drivers/bus/built-in.a creating empty archive: drivers/cdrom/built-in.a creating empty archive: drivers/char/ipmi/built-in.a creating empty archive: arch/powerpc/kernel/trace/built-in.a creating empty archive: drivers/char/agp/built-in.a creating empty archive: virt/lib/built-in.a creating empty archive: drivers/clk/actions/built-in.a creating empty archive: drivers/clocksource/built-in.a creating empty archive: drivers/clk/analogbits/built-in.a creating empty archive: drivers/firewire/built-in.a creating empty archive: drivers/clk/bcm/built-in.a creating empty archive: drivers/clk/imx/built-in.a creating empty archive: drivers/clk/imgtec/built-in.a creating empty archive: drivers/clk/ingenic/built-in.a creating empty archive: drivers/clk/mediatek/built-in.a creating empty archive: drivers/gpu/drm/arm/built-in.a creating empty archive: drivers/clk/mvebu/built-in.a creating empty archive: drivers/firmware/broadcom/built-in.a creating empty archive: drivers/firmware/imx/built-in.a creating empty archive: drivers/gpu/drm/bridge/synopsys/built-in.a creating empty archive: drivers/clk/renesas/built-in.a creating empty archive: drivers/firmware/meson/built-in.a creating empty archive: drivers/firmware/psci/built-in.a creating empty archive: drivers/clk/ti/built-in.a creating empty archive: drivers/gpu/drm/hisilicon/built-in.a creating empty archive: drivers/firmware/tegra/built-in.a creating empty archive: drivers/gpu/drm/i2c/built-in.a creating empty archive: drivers/gpu/drm/omapdrm/displays/built-in.a creating empty archive: drivers/gpu/drm/omapdrm/dss/built-in.a creating empty archive: drivers/firmware/xilinx/built-in.a creating empty archive: drivers/gpu/drm/panel/built-in.a creating empty archive: drivers/gpu/drm/rcar-du/built-in.a creating empty archive: drivers/hwtracing/intel_th/built-in.a creating empty archive: kernel/livepatch/built-in.a creating empty archive: drivers/gpu/drm/tilcdc/built-in.a creating empty archive: drivers/base/firmware_loader/builtin/built-in.a creating empty archive: drivers/base/power/built-in.a creating empty archive: drivers/gpu/vga/built-in.a creating empty archive: drivers/base/test/built-in.a creating empty archive: drivers/i2c/algos/built-in.a creating empty archive: drivers/i3c/built-in.a creating empty archive: drivers/i2c/busses/built-in.a creating empty archive: drivers/idle/built-in.a creating empty archive: drivers/i2c/muxes/built-in.a creating empty archive: drivers/iommu/built-in.a creating empty archive: fs/devpts/built-in.a creating empty archive: drivers/macintosh/built-in.a creating empty archive: fs/notify/dnotify/built-in.a creating empty archive: fs/notify/fanotify/built-in.a creating empty archive: fs/notify/inotify/built-in.a creating empty archive: drivers/media/common/b2c2/built-in.a creating empty archive: drivers/media/common/saa7146/built-in.a creating empty archive: fs/quota/built-in.a creating empty archive:
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote: > Segher Boessenkool writes: > And it's definitely calling ar with no flags, eg: > > rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a init/main.o > init/version.o init/do_mounts.o init/do_mounts_rd.o init/do_mounts_initrd.o > init/do_mounts_md.o init/initramfs.o init/init_task.o This uses thin archives. Those will work fine. The failing case was empty files IIRC, stuff created from no inputs. > So presumably at some point ar learnt to cope with objects that don't > match its default? (how do I ask it what its default is?) The first supported target in the --help list is the default, I think? $ powerpc-linux-ar --help [...] powerpc-linux-ar: supported targets: elf32-powerpc aixcoff-rs6000 elf32-powerpcle ppcboot elf64-powerpc elf64-powerpcle elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex $ powerpc64-linux-ar --help [...] powerpc64-linux-ar: supported targets: elf64-powerpc elf64-powerpcle elf32-powerpc elf32-powerpcle aixcoff-rs6000 aixcoff64-rs6000 aix5coff64-rs6000 elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex $ powerpc64le-linux-ar --help [...] powerpc64le-linux-ar: supported targets: elf64-powerpcle elf64-powerpc elf32-powerpcle elf32-powerpc aixcoff-rs6000 aixcoff64-rs6000 aix5coff64-rs6000 elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex $ powerpcle-linux-ar --help [...] powerpcle-linux-ar: supported targets: elf32-powerpcle aixcoff-rs6000 elf32-powerpc ppcboot elf64-powerpc elf64-powerpcle elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex > > Then again, does that work at *all* nowadays? Do we even consider that > > important, *should* it work? > > Yes and yes. There were a lot of bugs in the kernel makefiles after we > added LE support which prevented a biarch/biendian compiler from working. > But now it does work and we want it to keep working because it means you > can have a single compiler for building 32-bit, 64-bit BE & 64-bit LE. Good to hear :-) Segher
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
Segher Boessenkool writes:
> On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
>> Segher Boessenkool writes:
>> > Yes, that is why I used the environment variable, all binutils work
>> > with that. There was no --target option in GNU ar before 2.22.
>>
>> Yeah, we're not very good at testing with really old binutils, so I
>> guess we broke that.
>>
>> I'm inclined to merge this, it doesn't seem to break anything, and it
>> fixes using --target on old binutils that don't have it.
>
> But we don't set the target any other way either. I don't think this
> will work with a 32-bit toolchain (default target 32 bit) and a 64-bit
> kernel, or the other way around.
I think it does, but maybe I'm misunderstanding.
My test setup is:
~/linux$ export
PATH=/home/toolchains/ppc/gcc-8-branch/powerpc-linux/bin/:$PATH
~/linux$ echo "int test(void) { return 2; }" > test.c
~/linux$ powerpc-linux-gcc -c test.c
~/linux$ file test.o
test.o: ELF 32-bit MSB relocatable, PowerPC or cisco 4500, version 1 (SYSV),
not stripped
~/linux$ make CROSS_COMPILE=powerpc-linux- -s ppc64le_defconfig
~/linux$ make CROSS_COMPILE=powerpc-linux- -s -j 320
~/linux$ echo $?
0
And it's definitely calling ar with no flags, eg:
rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a init/main.o
init/version.o init/do_mounts.o init/do_mounts_rd.o init/do_mounts_initrd.o
init/do_mounts_md.o init/initramfs.o init/init_task.o
So presumably at some point ar learnt to cope with objects that don't
match its default? (how do I ask it what its default is?)
> Then again, does that work at *all* nowadays? Do we even consider that
> important, *should* it work?
Yes and yes. There were a lot of bugs in the kernel makefiles after we
added LE support which prevented a biarch/biendian compiler from working.
But now it does work and we want it to keep working because it means you
can have a single compiler for building 32-bit, 64-bit BE & 64-bit LE.
cheers
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Tue, Jul 16, 2019 at 3:16 AM Segher Boessenkool wrote: > > On Mon, Jul 15, 2019 at 09:03:46PM +0900, Masahiro Yamada wrote: > > On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool > > wrote: > > > > > > On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote: > > > > Segher Boessenkool writes: > > > > > Yes, that is why I used the environment variable, all binutils work > > > > > with that. There was no --target option in GNU ar before 2.22. > > > > I use binutils 2.30 > > It does not understand --target option. > > > > $ powerpc-linux-ar --version > > GNU ar (GNU Binutils) 2.30 > > Copyright (C) 2018 Free Software Foundation, Inc. > > This program is free software; you may redistribute it under the terms of > > the GNU General Public License version 3 or (at your option) any later > > version. > > This program has absolutely no warranty. > > > > If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this: > > powerpc-linux-ar: -t: No such file or directory > > You need to provide a valid command line, like > > $ powerpc-linux-ar tv smth.a --target=elf32-powerpc > > ar is a bit weird. Ah, I see! I had missed the space being required. Since I cannot test old binutils, I will leave this to ppc people. -- Best Regards Masahiro Yamada
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Mon, Jul 15, 2019 at 09:03:46PM +0900, Masahiro Yamada wrote: > On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool > wrote: > > > > On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote: > > > Segher Boessenkool writes: > > > > Yes, that is why I used the environment variable, all binutils work > > > > with that. There was no --target option in GNU ar before 2.22. > > I use binutils 2.30 > It does not understand --target option. > > $ powerpc-linux-ar --version > GNU ar (GNU Binutils) 2.30 > Copyright (C) 2018 Free Software Foundation, Inc. > This program is free software; you may redistribute it under the terms of > the GNU General Public License version 3 or (at your option) any later > version. > This program has absolutely no warranty. > > If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this: > powerpc-linux-ar: -t: No such file or directory You need to provide a valid command line, like $ powerpc-linux-ar tv smth.a --target=elf32-powerpc ar is a bit weird. Segher
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool wrote: > > On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote: > > Segher Boessenkool writes: > > > Yes, that is why I used the environment variable, all binutils work > > > with that. There was no --target option in GNU ar before 2.22. I use binutils 2.30 It does not understand --target option. $ powerpc-linux-ar --version GNU ar (GNU Binutils) 2.30 Copyright (C) 2018 Free Software Foundation, Inc. This program is free software; you may redistribute it under the terms of the GNU General Public License version 3 or (at your option) any later version. This program has absolutely no warranty. If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this: powerpc-linux-ar: -t: No such file or directory > > Yeah, we're not very good at testing with really old binutils, so I > > guess we broke that. > > > > I'm inclined to merge this, it doesn't seem to break anything, and it > > fixes using --target on old binutils that don't have it. > > But we don't set the target any other way either. I don't think this > will work with a 32-bit toolchain (default target 32 bit) and a 64-bit > kernel, or the other way around. > > Then again, does that work at *all* nowadays? Do we even consider that > important, *should* it work? Let me confirm if I understood this discussion. [1] KBUILD_ARFLAGS += --target=elf$(BITS)-$(GNUTARGET) is pointless since it is always overridden by another KBUILD_ARFLAGS assignment. [2] If we stop overriding it, it would cause build errors. So, --target is not only useless, but it is rather harmful. So, we all agreed with this patch, right? We are discussing whether or not to revive GNUTARGET=elf$(BITS)-$(GNUTARGET) in a *separate* patch, correct? -- Best Regards Masahiro Yamada
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote: > Segher Boessenkool writes: > > Yes, that is why I used the environment variable, all binutils work > > with that. There was no --target option in GNU ar before 2.22. > > Yeah, we're not very good at testing with really old binutils, so I > guess we broke that. > > I'm inclined to merge this, it doesn't seem to break anything, and it > fixes using --target on old binutils that don't have it. But we don't set the target any other way either. I don't think this will work with a 32-bit toolchain (default target 32 bit) and a 64-bit kernel, or the other way around. Then again, does that work at *all* nowadays? Do we even consider that important, *should* it work? Segher
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
Segher Boessenkool writes:
> On Sun, Jul 14, 2019 at 07:45:15AM +0900, Masahiro Yamada wrote:
>> On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
>> wrote:
>> > On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
>> > > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
>> > > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
>> > > > in a useful way because it is always overridden by the following code
>> > > > in the top Makefile:
>> > > >
>> > > > # use the deterministic mode of AR if available
>> > > > KBUILD_ARFLAGS := $(call ar-option,D)
>> > > >
>> > > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
>> > > > ("kbuild: Fix build with binutils <= 2.19").
>> > > >
>> > > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
>> > > > beginning.
>> > >
>> > > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
>> > >
>> > > Is it no longer supported to build a 64-bit kernel with a toolchain
>> > > that defaults to 32-bit, or the other way around? And with non-native
>> > > toolchains (this one didn't run on Linux, even).
>> >
>> > It was an --enable-targets=all toolchain, somewhat common for crosses,
>> > if that matters.
>>
>> I always use the same toolchain
>> for compile-testing PPC32/64.
>>
>> I have never been hit by the issue you mention.
>> Somebody would have reported it if it were still a problem.
>
> But did you use --enable-targets=all?
I do. And I don't see any errors with this patch applied.
> The problem was empty archives IIRC. Not a problem anymore with thin
> archives, maybe?
Maybe? Though I can't get it to break even before we switched to them.
>> Moreover, commit 43c9127d94d6
>> translated the environment variable "GNUTARGET"
>> into the command option "--target="
>>
>> My powerpc-linux-ar does not know it:
>>
>> powerpc-linux-ar: -t: No such file or directory
>
> Yes, that is why I used the environment variable, all binutils work
> with that. There was no --target option in GNU ar before 2.22.
Yeah, we're not very good at testing with really old binutils, so I
guess we broke that.
I'm inclined to merge this, it doesn't seem to break anything, and it
fixes using --target on old binutils that don't have it.
cheers
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Sun, Jul 14, 2019 at 07:45:15AM +0900, Masahiro Yamada wrote:
> On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
> wrote:
> > On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> > > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > > > in a useful way because it is always overridden by the following code
> > > > in the top Makefile:
> > > >
> > > > # use the deterministic mode of AR if available
> > > > KBUILD_ARFLAGS := $(call ar-option,D)
> > > >
> > > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > > > ("kbuild: Fix build with binutils <= 2.19").
> > > >
> > > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > > > beginning.
> > >
> > > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
> > >
> > > Is it no longer supported to build a 64-bit kernel with a toolchain
> > > that defaults to 32-bit, or the other way around? And with non-native
> > > toolchains (this one didn't run on Linux, even).
> >
> > It was an --enable-targets=all toolchain, somewhat common for crosses,
> > if that matters.
>
> I always use the same toolchain
> for compile-testing PPC32/64.
>
> I have never been hit by the issue you mention.
> Somebody would have reported it if it were still a problem.
But did you use --enable-targets=all?
The problem was empty archives IIRC. Not a problem anymore with thin
archives, maybe?
> Moreover, commit 43c9127d94d6
> translated the environment variable "GNUTARGET"
> into the command option "--target="
>
> My powerpc-linux-ar does not know it:
>
> powerpc-linux-ar: -t: No such file or directory
Yes, that is why I used the environment variable, all binutils work
with that. There was no --target option in GNU ar before 2.22.
Segher
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
wrote:
>
> On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > > in a useful way because it is always overridden by the following code
> > > in the top Makefile:
> > >
> > > # use the deterministic mode of AR if available
> > > KBUILD_ARFLAGS := $(call ar-option,D)
> > >
> > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > > ("kbuild: Fix build with binutils <= 2.19").
> > >
> > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > > beginning.
> >
> > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
> >
> > Is it no longer supported to build a 64-bit kernel with a toolchain
> > that defaults to 32-bit, or the other way around? And with non-native
> > toolchains (this one didn't run on Linux, even).
>
> It was an --enable-targets=all toolchain, somewhat common for crosses,
> if that matters.
I always use the same toolchain
for compile-testing PPC32/64.
I have never been hit by the issue you mention.
Somebody would have reported it if it were still a problem.
Moreover, commit 43c9127d94d6
translated the environment variable "GNUTARGET"
into the command option "--target="
My powerpc-linux-ar does not know it:
powerpc-linux-ar: -t: No such file or directory
--
Best Regards
Masahiro Yamada
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > in a useful way because it is always overridden by the following code
> > in the top Makefile:
> >
> > # use the deterministic mode of AR if available
> > KBUILD_ARFLAGS := $(call ar-option,D)
> >
> > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > ("kbuild: Fix build with binutils <= 2.19").
> >
> > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > beginning.
>
> That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
>
> Is it no longer supported to build a 64-bit kernel with a toolchain
> that defaults to 32-bit, or the other way around? And with non-native
> toolchains (this one didn't run on Linux, even).
It was an --enable-targets=all toolchain, somewhat common for crosses,
if that matters.
Segher
Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> in a useful way because it is always overridden by the following code
> in the top Makefile:
>
> # use the deterministic mode of AR if available
> KBUILD_ARFLAGS := $(call ar-option,D)
>
> The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> ("kbuild: Fix build with binutils <= 2.19").
>
> The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> beginning.
That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
Is it no longer supported to build a 64-bit kernel with a toolchain
that defaults to 32-bit, or the other way around? And with non-native
toolchains (this one didn't run on Linux, even).
Segher
