diff mbox series

[RFC,kvm-unit-tests] build: fix .aux.o target building

Message ID 20240605081623.8765-1-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series [RFC,kvm-unit-tests] build: fix .aux.o target building | expand

Commit Message

Nicholas Piggin June 5, 2024, 8:16 a.m. UTC
Here's another oddity I ran into with the build system. Try run make
twice. With arm64 and ppc64, the first time it removes some intermediate
files and the second causes another rebuild of several files. After
that it's fine. s390x seems to follow a similar pattern but does not
suffer from the problem. Also, the .PRECIOUS directive is not preventing
them from being deleted inthe first place. So... that probably means I
haven't understood it properly and the fix may not be correct, but it
does appear to DTRT... Anybody with some good Makefile knowledge might
have a better idea.

Thanks,
Nick
---
powerpc and arm64 remove .aux.o files because they are seen as
intermediate, I think because they don't have an explicit target
(for some reason not s390x, haven't tested arm or riscv). This
causes them to be those files to be removed after the make runs.
If make is run again the .aux.o and .elf targets are rebuild,
but for some reason they are treated differently and not removed,
leading to the 3rd make being a no-op.
---
 arm/Makefile.common     | 3 ++-
 powerpc/Makefile.common | 2 +-
 riscv/Makefile          | 2 +-
 s390x/Makefile          | 2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)

Comments

Marc Hartmayer June 5, 2024, 10:42 a.m. UTC | #1
On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote:
> Here's another oddity I ran into with the build system. Try run make
> twice. With arm64 and ppc64, the first time it removes some intermediate
> files and the second causes another rebuild of several files. After
> that it's fine. s390x seems to follow a similar pattern but does not
> suffer from the problem. Also, the .PRECIOUS directive is not preventing
> them from being deleted inthe first place. So... that probably means I
> haven't understood it properly and the fix may not be correct, but it
> does appear to DTRT... Anybody with some good Makefile knowledge might
> have a better idea.
>

$ make clean -j &>/dev/null && make -d
…
Successfully remade target file 'all'.
Removing intermediate files...
rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o

So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean).

.PRECIOUS: %.o %.aux.o

Fixed the issue (I’ve tested on ppc64 only).

>
>
Andrew Jones June 5, 2024, 10:49 a.m. UTC | #2
On Wed, Jun 05, 2024 at 12:42:30PM GMT, Marc Hartmayer wrote:
> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote:
> > Here's another oddity I ran into with the build system. Try run make
> > twice. With arm64 and ppc64, the first time it removes some intermediate
> > files and the second causes another rebuild of several files. After
> > that it's fine. s390x seems to follow a similar pattern but does not
> > suffer from the problem. Also, the .PRECIOUS directive is not preventing
> > them from being deleted inthe first place. So... that probably means I
> > haven't understood it properly and the fix may not be correct, but it
> > does appear to DTRT... Anybody with some good Makefile knowledge might
> > have a better idea.
> >
> 
> $ make clean -j &>/dev/null && make -d
> …
> Successfully remade target file 'all'.
> Removing intermediate files...
> rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o
> 
> So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean).
> 
> .PRECIOUS: %.o %.aux.o

Just using .PRECIOUS is fine by me.

Thanks,
drew

> 
> Fixed the issue (I’ve tested on ppc64 only).
> 
> >
> >
> -- 
> Kind regards / Beste Grüße
>    Marc Hartmayer
> 
> IBM Deutschland Research & Development GmbH
> Vorsitzender des Aufsichtsrats: Wolfgang Wendt
> Geschäftsführung: David Faller
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
Nicholas Piggin June 5, 2024, 10:53 a.m. UTC | #3
On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote:
> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote:
> > Here's another oddity I ran into with the build system. Try run make
> > twice. With arm64 and ppc64, the first time it removes some intermediate
> > files and the second causes another rebuild of several files. After
> > that it's fine. s390x seems to follow a similar pattern but does not
> > suffer from the problem. Also, the .PRECIOUS directive is not preventing
> > them from being deleted inthe first place. So... that probably means I
> > haven't understood it properly and the fix may not be correct, but it
> > does appear to DTRT... Anybody with some good Makefile knowledge might
> > have a better idea.
> >
>
> $ make clean -j &>/dev/null && make -d
> …
> Successfully remade target file 'all'.
> Removing intermediate files...
> rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o
>
> So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean).
>
> .PRECIOUS: %.o %.aux.o

Ah, so %.o does not match %.aux.o. That answers that. Did you see
why s390x is immune? Maybe it defines the target explicitly somewhere.

Is it better to define explicit targets if we want to keep them, or
add to .PRECIOUS? Your patch would be simpler.

Thanks,
Nick
Marc Hartmayer June 5, 2024, 3:07 p.m. UTC | #4
On Wed, Jun 05, 2024 at 08:53 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote:
> On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote:
>> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote:
>> > Here's another oddity I ran into with the build system. Try run make
>> > twice. With arm64 and ppc64, the first time it removes some intermediate
>> > files and the second causes another rebuild of several files. After
>> > that it's fine. s390x seems to follow a similar pattern but does not
>> > suffer from the problem. Also, the .PRECIOUS directive is not preventing
>> > them from being deleted inthe first place. So... that probably means I
>> > haven't understood it properly and the fix may not be correct, but it
>> > does appear to DTRT... Anybody with some good Makefile knowledge might
>> > have a better idea.
>> >
>>
>> $ make clean -j &>/dev/null && make -d
>> …
>> Successfully remade target file 'all'.
>> Removing intermediate files...
>> rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o
>>
>> So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean).
>>
>> .PRECIOUS: %.o %.aux.o
>
> Ah, so %.o does not match %.aux.o. That answers that. Did you see
> why s390x is immune? Maybe it defines the target explicitly somewhere.

Not yet :/ But what was also interesting is that if I’m using multiple
jobs I don’t see the issue.

make clean -j; make -j; make -j # <- the last make has nothing to do

if I’m using:

make clean -j; make; make -j # <- the last make has something to do…
                                  that something that irritates me

>
> Is it better to define explicit targets if we want to keep them, or
> add to .PRECIOUS? Your patch would be simpler.

Normally, I would say without .PRECIOUS it’s cleaner, but there is
already a .PRECIOUS for %.so… So as Andrew has already written

.PRECIOUS: %.so %.aux.o

should also be fine.

>
> Thanks,
> Nick
>
Nicholas Piggin June 6, 2024, 3:49 a.m. UTC | #5
On Thu Jun 6, 2024 at 1:07 AM AEST, Marc Hartmayer wrote:
> On Wed, Jun 05, 2024 at 08:53 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote:
> > On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote:
> >> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote:
> >> > Here's another oddity I ran into with the build system. Try run make
> >> > twice. With arm64 and ppc64, the first time it removes some intermediate
> >> > files and the second causes another rebuild of several files. After
> >> > that it's fine. s390x seems to follow a similar pattern but does not
> >> > suffer from the problem. Also, the .PRECIOUS directive is not preventing
> >> > them from being deleted inthe first place. So... that probably means I
> >> > haven't understood it properly and the fix may not be correct, but it
> >> > does appear to DTRT... Anybody with some good Makefile knowledge might
> >> > have a better idea.
> >> >
> >>
> >> $ make clean -j &>/dev/null && make -d
> >> …
> >> Successfully remade target file 'all'.
> >> Removing intermediate files...
> >> rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o
> >>
> >> So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean).
> >>
> >> .PRECIOUS: %.o %.aux.o
> >
> > Ah, so %.o does not match %.aux.o. That answers that. Did you see
> > why s390x is immune? Maybe it defines the target explicitly somewhere.
>
> Not yet :/

Strange. Both unpatched powerpc and s390x have these lines.

   Looking for a rule with intermediate file 'powerpc/emulator.aux.o'.

I think that's why it's considered intermediate at least for powerpc,
but I can't see why s390x is different... oh, taking out the .SECONDARY
lines makes s390x delete the aux intermediates. I guess those targets
transitively depend on the aux files which prevents the aux from being
deleted.

Is that fragile? I'm not sure. The patch that introduced them was not
solving this problem (that came before your %.aux.o target patch).
s390x does not need any .PRECIOUS targets at all at the moment.

I guess that mostly explains things.


> But what was also interesting is that if I’m using multiple
> jobs I don’t see the issue.
>
> make clean -j; make -j; make -j # <- the last make has nothing to do
>
> if I’m using:
>
> make clean -j; make; make -j # <- the last make has something to do…
>                                   that something that irritates me

This is with s390x? Maybe with parallel make, the target is getting
rebuilt via a different prerequisite that is not a .SECONDARY target?
Adding %.aux.o in PRECIOUS there should help in that case.

> >
> > Is it better to define explicit targets if we want to keep them, or
> > add to .PRECIOUS? Your patch would be simpler.
>
> Normally, I would say without .PRECIOUS it’s cleaner, but there is
> already a .PRECIOUS for %.so… So as Andrew has already written
>
> .PRECIOUS: %.so %.aux.o
>
> should also be fine.

Okay, for a minimal fix I will do that.

Thanks,
Nick
Marc Hartmayer June 6, 2024, 7:58 a.m. UTC | #6
On Thu, Jun 06, 2024 at 01:49 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote:
> On Thu Jun 6, 2024 at 1:07 AM AEST, Marc Hartmayer wrote:
>> On Wed, Jun 05, 2024 at 08:53 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote:
>> > On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote:
>> >> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote:

[…snip…]

>
>
>> But what was also interesting is that if I’m using multiple
>> jobs I don’t see the issue.
>>
>> make clean -j; make -j; make -j # <- the last make has nothing to do
>>
>> if I’m using:
>>
>> make clean -j; make; make -j # <- the last make has something to do…
>>                                   that something that irritates me
>
> This is with s390x? Maybe with parallel make, the target is getting

No, it’s a ppc64 cross-build on x86.

> rebuilt via a different prerequisite that is not a .SECONDARY target?
> Adding %.aux.o in PRECIOUS there should help in that case.

Yes, it helps - that was the reason for my fix :)

>
>> >
>> > Is it better to define explicit targets if we want to keep them, or
>> > add to .PRECIOUS? Your patch would be simpler.
>>
>> Normally, I would say without .PRECIOUS it’s cleaner, but there is
>> already a .PRECIOUS for %.so… So as Andrew has already written
>>
>> .PRECIOUS: %.so %.aux.o
>>
>> should also be fine.
>
> Okay, for a minimal fix I will do that.
>
> Thanks,
> Nick
>
diff mbox series

Patch

diff --git a/arm/Makefile.common b/arm/Makefile.common
index f828dbe01..3ebbcc13e 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -72,7 +72,7 @@  eabiobjs = lib/arm/eabi_compat.o
 FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
 
 ifeq ($(CONFIG_EFI),y)
-%.aux.o: $(SRCDIR)/lib/auxinfo.c
+$(tests-all:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c
 	$(CC) $(CFLAGS) -c -o $@ $< \
 		-DPROGNAME=\"$(@:.aux.o=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
 
@@ -92,6 +92,7 @@  ifeq ($(CONFIG_EFI),y)
 		-j .reloc \
 		-O binary $^ $@
 else
+$(tests-all:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c
 %.aux.o: $(SRCDIR)/lib/auxinfo.c
 	$(CC) $(CFLAGS) -c -o $@ $< \
 		-DPROGNAME=\"$(@:.aux.o=.flat)\" -DAUXFLAGS=$(AUXFLAGS)
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index b98f71c2f..3b219eee0 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -52,7 +52,7 @@  cflatobjs += lib/powerpc/smp.o
 
 OBJDIRS += lib/powerpc
 
-%.aux.o: $(SRCDIR)/lib/auxinfo.c
+$(tests-all:.elf=.aux.o): $(SRCDIR)/lib/auxinfo.c
 	$(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\"
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
diff --git a/riscv/Makefile b/riscv/Makefile
index 919a3ebb5..4610cb4d4 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -81,7 +81,7 @@  CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt
 asm-offsets = lib/riscv/asm-offsets.h
 include $(SRCDIR)/scripts/asm-offsets.mak
 
-%.aux.o: $(SRCDIR)/lib/auxinfo.c
+$(tests:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c
 	$(CC) $(CFLAGS) -c -o $@ $< \
 		-DPROGNAME=\"$(notdir $(@:.aux.o=.$(exe)))\" -DAUXFLAGS=$(AUXFLAGS)
 
diff --git a/s390x/Makefile b/s390x/Makefile
index 23342bd64..4c0c8085c 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -178,7 +178,7 @@  lds-autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -MT $@
 %.lds: %.lds.S $(asm-offsets)
 	$(CPP) $(lds-autodepend-flags) $(CPPFLAGS) -P -C -o $@ $<
 
-%.aux.o: $(SRCDIR)/lib/auxinfo.c
+$(tests:.elf=.aux.o): $(SRCDIR)/lib/auxinfo.c
 	$(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\"
 
 .SECONDEXPANSION: