Re: [PATCH v2 3/6] Makefile: Do not force elf last for the others and tests passes

On 03/07/26 07:59, Sam James wrote:
> Adhemerval Zanella <[email protected]> writes:
> 
>> The requirement that the elf subdirectory comes last in the
>> subdirectory ordering stems from its lib pass: the rtld link consumes
>> $(common-objpfx)libc_pic.a, which aggregates every other
>> subdirectory's objects.
>>
>> The others, tests, and xtests classes have no such dependency: everything
>> they consume from other subdirectories is provided by the pass barriers
>> (others after lib, tests after others).  Keep elf last only for the
>> object-building classes and let its others and tests sub-makes run
>> concurrently with the other subdirectories.
>>
>> With elf no longer forced last for those classes, the Depend edges
>> pointing to elf (e.g. support/Depend) no longer create a cycle there,
>> so honor them instead of dropping them.
>>
>> This improves the make check withr run-built-tests=no, specially on
> 
> with

Ack.

> 
>> machine with many cores.  Results on a x86_64 machine [1] it improves
>> from 190s to 181s, while on a aarch64 machine [2] it improves from
>> 298.726s to 243.098s.
>>
>> Build results remain bit-identical and the tests.sum failure sets are
>> unchanged.
>>
>> [1] Ryzen 5900x, 12c/24t, gcc 16.1.1, binutils 2.26, and GNU make 4.3
>> [2] N1, 80c, gcc 15.1.1, binutils 2.25, GNU make 4.3
>> ---
>>  Makefile | 34 +++++++++++++++++++++++-----------
>>  1 file changed, 23 insertions(+), 11 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 45dc36942a8..a6e5317de7b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -508,21 +508,28 @@ endef
>>  #  * The edges requested by the Depend files (emitted by gen-sorted.awk
>>  #    as subdir-deps-*) are preserved.
>>  #
>> -#  * elf stays last, as in the sorted list.  Its rtld build recurses into
>> -#    the other subdirectories' object directories via elf/rtld-Rules.
>> +#  * elf stays last for the object-building classes, as in the sorted
>> +#    list: its rtld link consumes $(common-objpfx)libc_pic.a, which
>> +#    aggregates every other subdirectory's objects, and its rtld-Rules
>> +#    recursion compiles into the other subdirectories' object
>> +#    directories.  The others/tests/xtests classes have no such
>> +#    dependency (the pass barriers below provide everything they need),
>> +#    so elf is unordered there.
>>  #
>>  #  * Only target classes without cross-directory file conflicts use this
>>  #    sparse ordering; everything else (install, clean, abi, stubs) keeps
>>  #    the previous total order via a serial chain.
>>  
>> -+parallel_subdir_targets := \
>> -  subdir_lib \
>> -  objects \
>> ++elf_last_subdir_targets := \
>> +  subdir_lib objects \
>>    objs \
>> +  subdir_objs \
>> +  # +elf_last_subdir_targets
>> ++parallel_subdir_targets := \
>> +  $(+elf_last_subdir_targets) \
>>    others \
>>    tests \
>>    xtests \
>> -  subdir_objs \
>>    # +parallel_subdir_targets
>>  +serial_subdir_targets := $(filter-out $(+parallel_subdir_targets),\
>>  				       $(+subdir_targets))
>> @@ -552,14 +559,19 @@ $(foreach t,$(+ordered_parallel_subdir_targets),$(eval \
>>  $(foreach d,$(+subdir-pregen),$(foreach t,$(+ordered_parallel_subdir_targets),$(eval \
>>    $(d)/$(t): $(addsuffix /$(t),$(+subdir-pregen-prev))))\
>>    $(eval +subdir-pregen-prev := $(d)))
>> -# Edges pointing to elf are dropped; the sorted list always forces elf
>> -# last, overriding any Depend request, and the elf-last edges below would
>> -# otherwise create a cycle.
>> -$(foreach t,$(+ordered_parallel_subdir_targets),$(foreach d,$(+subdir-rest),$(eval \
>> +# For the classes where elf is forced last, edges pointing to elf are
>> +# dropped: the sorted list always overrides such Depend requests today
>> +# (e.g. support/Depend), and the elf-last edges below would otherwise
>> +# create a cycle.  The remaining classes honor them.
>> +$(foreach t,$(+elf_last_subdir_targets),$(foreach d,$(+subdir-rest),$(eval \
>>    $(d)/$(t): $(addsuffix /$(t),\
>>  	      $(filter-out elf,$(filter $(subdirs),$(subdir-deps-$(d))))))))
>> +$(foreach t,$(filter-out $(+elf_last_subdir_targets),\
>> +		         $(+ordered_parallel_subdir_targets)),\
>> +  $(foreach d,$(+subdir-rest),$(eval \
>> +  $(d)/$(t): $(addsuffix /$(t),$(filter $(subdirs),$(subdir-deps-$(d)))))))
>>  ifneq (,$(filter elf,$(subdirs)))
>> -$(foreach t,$(+ordered_parallel_subdir_targets),$(eval \
>> +$(foreach t,$(+elf_last_subdir_targets),$(eval \
>>    elf/$(t): $(addsuffix /$(t),$(filter-out elf,$(subdirs)))))
>>  endif
> 
> Not a huge fan of how complex this is but it already was pretty
> complex and the comments help.

I tend to agree, and the build dependencies are not straightforward visible
without parsing the multiple implicit rules we have.  I tried to not avoid adding
*more* complexity, but I don't see a way to simplify it without refactoring 
*a lot* of Make rules.

> 
> Reviewed-by: Sam James <[email protected]>