[XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o

Anthony PERARD posted 16 patches 5 years, 9 months ago
There is a newer version of this series
[XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
Posted by Anthony PERARD 5 years, 9 months ago
In the case where $(obj-y) is empty, we also replace $(c_flags) by
$(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
make trying to include %.h file in the ld command if $(obj-y) isn't
empty anymore on a second run.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v4:
    - Have cmd_ld_builtin depends on CONFIG_LTO, which simplify built_in.o
      rule.

 xen/Rules.mk | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 25bcf45612bd..5e08e14455d7 100644
--- 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
+
+quiet_cmd_cc_builtin = LD      $@
+cmd_cc_builtin = \
+    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
+
+built_in.o: $(obj-y) $(extra-y) FORCE
+ifeq ($(obj-y),)
+	$(call if_changed,cc_builtin)
+else
+	$(call if_changed,ld_builtin)
 endif
 
 targets += built_in.o
-- 
Anthony PERARD


Re: [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
Posted by Jan Beulich 5 years, 9 months ago
On 21.04.2020 18:12, Anthony PERARD wrote:
> In the case where $(obj-y) is empty, we also replace $(c_flags) by
> $(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
> make trying to include %.h file in the ld command if $(obj-y) isn't
> empty anymore on a second run.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Personally I'd prefer ...

> --- 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
> +
> +quiet_cmd_cc_builtin = LD      $@
> +cmd_cc_builtin = \
> +    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
> +
> +built_in.o: $(obj-y) $(extra-y) FORCE
> +ifeq ($(obj-y),)
> +	$(call if_changed,cc_builtin)
> +else
> +	$(call if_changed,ld_builtin)
>  endif

...

   $(call if_changed,$(if $(obj-y),ld,cc)_builtin)

but perhaps I'm the only one.

Jan

Re: [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
Posted by Anthony PERARD 5 years, 9 months ago
On Tue, Apr 28, 2020 at 03:48:13PM +0200, Jan Beulich wrote:
> On 21.04.2020 18:12, Anthony PERARD wrote:
> > In the case where $(obj-y) is empty, we also replace $(c_flags) by
> > $(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
> > make trying to include %.h file in the ld command if $(obj-y) isn't
> > empty anymore on a second run.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> Personally I'd prefer ...
> 
> > --- 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
> > +
> > +quiet_cmd_cc_builtin = LD      $@
> > +cmd_cc_builtin = \
> > +    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
> > +
> > +built_in.o: $(obj-y) $(extra-y) FORCE
> > +ifeq ($(obj-y),)
> > +	$(call if_changed,cc_builtin)
> > +else
> > +	$(call if_changed,ld_builtin)
> >  endif
> 
> ...
> 
>    $(call if_changed,$(if $(obj-y),ld,cc)_builtin)
> 
> but perhaps I'm the only one.

I think so. Spelling the full name of the command makes it easier to
look for where it is used, or for where it is defined.

Linux doesn't have this issue about checking $(obj-y) as they use 'ar'
to make archives of objects, an archive with 0 object is fine. But that
is something I'll look at later, to find out if it is better and why.

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
Posted by Jan Beulich 5 years, 9 months ago
On 01.05.2020 16:42, Anthony PERARD wrote:
> On Tue, Apr 28, 2020 at 03:48:13PM +0200, Jan Beulich wrote:
>> On 21.04.2020 18:12, Anthony PERARD wrote:
>>> In the case where $(obj-y) is empty, we also replace $(c_flags) by
>>> $(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
>>> make trying to include %.h file in the ld command if $(obj-y) isn't
>>> empty anymore on a second run.
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>
>> Personally I'd prefer ...
>>
>>> --- 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
>>> +
>>> +quiet_cmd_cc_builtin = LD      $@
>>> +cmd_cc_builtin = \
>>> +    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
>>> +
>>> +built_in.o: $(obj-y) $(extra-y) FORCE
>>> +ifeq ($(obj-y),)
>>> +	$(call if_changed,cc_builtin)
>>> +else
>>> +	$(call if_changed,ld_builtin)
>>>  endif
>>
>> ...
>>
>>    $(call if_changed,$(if $(obj-y),ld,cc)_builtin)
>>
>> but perhaps I'm the only one.
> 
> I think so. Spelling the full name of the command makes it easier to
> look for where it is used, or for where it is defined.

   $(call if_changed,$(if $(obj-y),ld_builtin,cc_builtin))

then?

Jan