[PATCH v2] drm/xe: replace 'grouped target' in Makefile with pattern rule

[Gustavo Sousa]

Quoting Lucas De Marchi (2024-02-26 00:31:57-03:00)
>On Sun, Feb 25, 2024 at 01:07:08PM +0200, Dafna Hirschfeld wrote:
>>On 21.02.2024 23:49, Lucas De Marchi wrote:
>>>On Wed, Feb 21, 2024 at 11:55:56AM +0200, Dafna Hirschfeld wrote:
>>>>Since 'grouped target' is used only in 'make' 4.3, it should
>>>>be avoided. Replace it with 'multi-target pattern rule' which
>>>>has the same behavior.
>>>>
>>>>Signed-off-by: Dafna Hirschfeld <dhirschfeld at habana.ai>
>>>>---
>>>>v2 changes: instead of removing 'grouped target' we replace it with pattern-rule
>>>>here is v1: https://lore.kernel.org/intel-xe/20240219135845.1463992-1-dhirschfeld@habana.ai/
>>>>
>>>>drivers/gpu/drm/xe/Makefile | 2 +-
>>>>1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>>>>index c531210695db..64903841fbd4 100644
>>>>--- a/drivers/gpu/drm/xe/Makefile
>>>>+++ b/drivers/gpu/drm/xe/Makefile
>>>>@@ -42,7 +42,7 @@ generated_oob := $(obj)/generated/xe_wa_oob.c $(obj)/generated/xe_wa_oob.h
>>>>quiet_cmd_wa_oob = GEN     $(notdir $(generated_oob))
>>>>     cmd_wa_oob = mkdir -p $(@D); $^ $(generated_oob)
>>>>
>>>>-$(generated_oob) &: $(obj)/xe_gen_wa_oob $(srctree)/$(src)/xe_wa_oob.rules
>>>>+$(obj)/generated/%.c $(obj)/generated/%.h:  $(obj)/xe_gen_wa_oob $(srctree)/$(src)/xe_wa_oob.rules
>>>
>>>that means we can only ever have generated files for xe_wa_oob and
>>>nothing else (or we would have to change this line too.
>>>
>>>did you see any issue with the line I suggested?
>>
>>I checked your suggestion:
>>"
>>%/generated/xe_wa_oob.c %/generated/xe_wa_oob.h: $(obj)/xe_gen_wa_oob $(srctree)/$(src)/xe_wa_oob.rules
>>"
>>which works as well. But the doc says "Always use $(obj) when referring to generated files."
>>
>>https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#custom-rules

Since "%/generated/xe_wa_oob.c
%/generated/xe_wa_oob.h: ..." is a pattern rule, it would
only "spawn" into a "real" rule when there are targets (e.g. as
pre-requisite of another rule) that match it.

As such, I think the real thing "referring to generated files" would be
explicit make targets or pre-requisites. So, I'm not sure the pattern
suggested by Lucas really breaks the rule quoted above.

That said, I think the "%/generated/xe_wa_oob.c %/generated/xe_wa_oob.h"
is susceptible to failure in following the rule on other places.

>>
>>So I think we rather keep it.
>
>humn... but % at the beginning is already globbing it.
>Note that the syntax I suggested is the very same used in /Makefile:
>
>        # This exploits the 'multi-target pattern rule' trick.
>        # The syncconfig should be executed only once to make all the targets.
>        # (Note: use the grouped target '&:' when we bump to GNU Make 4.3)
>        #
>        # Do not use $(call cmd,...) here. That would suppress prompts from syncconfig,
>        # so you cannot notice that Kconfig is waiting for the user input.
>        %/config/auto.conf %/config/auto.conf.cmd %/generated/autoconf.h %/generated/rustc_cfg: $(KCONFIG_CONFIG)
>                $(Q)$(kecho) "  SYNC    $@"
>                $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
>
>Alternative would be to use
>
>        $(obj)/generated/%_wa_oob.c $(obj)/generated/%_wa_oob.h:
>
>... that should also work (although ugly), but since the root Makefile
>skips it, I'd just use the very same pattern.

I do not think it is ugly. I like this one better actually, because it
looks more self-contained and would further avoid accidental matches.

--
Gustavo Sousa

>
>The syntax you used is wrong because then the rule matches any 
>generated/*.{c,h}, even if the file has nothing to do with oob WAs.
>
>Lucas De Marchi
>
>>
>>thanks,
>>Dafna
>>
>>
>>>
>>>Lucas De Marchi
>>>
>>>>        $(call cmd,wa_oob)
>>>>
>>>>uses_generated_oob := \
>>>>-- 
>>>>2.34.1
>>>>