Re: [Xen-devel] [XEN PATCH v3 23/23] xen/build: use if_changed to build guest_%.o

From: Jan Beulich <jbeulich@suse.com>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>, "Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Tim Deegan" <tim@xen.org>,
	xen-devel@lists.xenproject.org,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [XEN PATCH v3 23/23] xen/build: use if_changed to build guest_%.o
Date: Thu, 5 Mar 2020 16:12:32 +0100	[thread overview]
Message-ID: <12dea9ae-4610-0b1b-fe87-141221d61d71@suse.com> (raw)
In-Reply-To: <20200226113355.2532224-24-anthony.perard@citrix.com>

On 26.02.2020 12:33, Anthony PERARD wrote:
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -115,6 +115,9 @@ endif
>  # FIXME LTO broken, but we would need a different way to filter -flto out
>  # $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
>  
> +# target with its suffix stripped
> +target-stem = $(basename $@)

I'd appreciate if the word "stem" was used in a makefile only for
what make doc uses it for - the part of the target of a pattern
rule that % matches. I.e. here perhaps name the variable
target-basename? (But see below, maybe this isn't needed.)

> --- a/xen/arch/x86/mm/Makefile
> +++ b/xen/arch/x86/mm/Makefile
> @@ -11,11 +11,14 @@ obj-y += p2m.o p2m-pt.o
>  obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o
>  obj-y += paging.o
>  
> -guest_walk_%.o: guest_walk.c Makefile
> -	$(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
> +$(foreach gw,$(filter guest_walk_%.o,$(obj-y)),\
> +    $(eval CFLAGS_$(gw) = -DGUEST_PAGING_LEVELS=$$*))

So the $$* here matches ...

> -guest_walk_%.i: guest_walk.c Makefile
> -	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
> +guest_walk_%.o: guest_walk.c FORCE
> +	$(call if_changed_rule,cc_o_c)

... the stem of the target of this rule. This is not good. Can't
you have something like

guest_walk_%.o guest_walk_%.i guest_walk_%.s: CFLAGS_$(target-stem).o = -DGUEST_PAGING_LEVELS=$*

on a line immediately ahead of the rule, so that them having to
match up will be very obvious, and breakage of the connection
very noticable?

(Of course this also demonstrates that tying the CFLAGS modifier
to the object file name may be slightly confusing. But I don't
have a better suggestion. Question is whether here use of an
object [or whatever else] file specific variable is helpful at
all, when make already offers per-target variable customization.
Is there a specific reason the above couldn't e.g. be

guest_walk_%.o guest_walk_%.i guest_walk_%.s: CFLAGS-y += -DGUEST_PAGING_LEVELS=$*

?)

If this alternative worked, there'd be the positive side effect
of us avoiding the use of $(eval ) here - ISTR it not working
very reliably in make 3.80, which we still document as acceptable
for building.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel