[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v4 10/18] xen/build: use if_changed on built_in.o



On 08/04/2020 13:40, Jan Beulich wrote:
> On 31.03.2020 12:30, Anthony PERARD wrote:
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -130,15 +130,24 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
>>  c_flags += $(CFLAGS-y)
>>  a_flags += $(CFLAGS-y) $(AFLAGS-y)
>>  
>> -built_in.o: $(obj-y) $(extra-y)
>> -ifeq ($(obj-y),)
>> -    $(CC) $(c_flags) -c -x c /dev/null -o $@
>> -else
>> +quiet_cmd_ld_builtin = LD      $@
>>  ifeq ($(CONFIG_LTO),y)
>> -    $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
>> +cmd_ld_builtin = \
>> +    $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>>  else
>> -    $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
>> +cmd_ld_builtin = \
>> +    $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>>  endif
> How about going yet one step further and doing away with the
> ifeq here altogether:
>
> cmd_ld_builtin-y = \
>     $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
> cmd_ld_builtin-$(CONFIG_LTO) = \
>     $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))

Please don't.

Logic like this is substantially harder to follow than a plain if/else
construct, and clarity is of far higher importance than optimising the
line count in the build system.

This trick only works for trivial cases, and interferes with diff's when
the logic inevitably becomes less trivial.  LTO support in particular
needs a complete overhaul, but there is no way I'm going to touch that
with a barge pole until this series is in place.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.