x86: Asm macros fixes

PATCH In review Nadav Amit 2 participants 10 messages Nov 15, 2018 – Nov 21, 2018

Subsystems (2)

x86 Kbuild

lore

Share & tools

lore

Nadav Amit

author

2018-11-15 04:01 UTC #0 lore raw

v2 cover x86: Asm macros fixes

There has been a complaint that the recent use of assembly macros in C files broke distcc. The first patch fixes this issue.

The second patch adds a dependency for all C files on macros.S, to trigger their recompilation when the relevant macros change.

I think it will be better to get the patches through tip. Please advise if you disagree.

v1->v2:
* Remove whitespaces [Ingo]
* Automatically enable split compilation when distcc or icecc are used

Nadav Amit (2):
Makefile: Fix distcc compilation with x86 macros
x86: set a dependency on macros.S

 Makefile               |  4 +++-
 arch/x86/Makefile      |  7 +++++--
 scripts/Makefile.build | 34 ++++++++++++++++++++++++++++++----
 3 files changed, 38 insertions(+), 7 deletions(-)

Signature · 1 line

2.17.1

Nadav Amit

author

2018-11-15 04:01 UTC #1 lore raw

PATCH v2 1/2 Makefile: Fix distcc compilation with x86 macros

Introducing the use of asm macros in c-code broke distcc, since it only sends the preprocessed source file. The solution is to break the compilation into two separate phases of compilation and assembly, and between the two concatenate the assembly macros and the compiled (yet not assembled) source file. Since this is less efficient, this compilation mode is only used when distcc or icecc are used.

Note that the assembly stage should also be distributed, if distcc is configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".

Reported-by Logan Gunthorpe <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
 Makefile               |  4 +++-
 arch/x86/Makefile      |  7 +++++--
 scripts/Makefile.build | 30 ++++++++++++++++++++++++++++--
 3 files changed, 36 insertions(+), 5 deletions(-)

Makefile +3 -1

index 9fce8b91c15f..c07349fc38c7 100644
@@ -743,7 +743,9 @@ KBUILD_CFLAGS   += $(call cc-option, -gsplit-dwarf, -g)
 else
 KBUILD_CFLAGS	+= -g
 endif
-KBUILD_AFLAGS	+= -Wa,-gdwarf-2
+AFLAGS_DEBUG_INFO = -Wa,-gdwarf-2
+export AFLAGS_DEBUG_INFO
+KBUILD_AFLAGS	+= $(AFLAGS_DEBUG_INFO)
 endif
 ifdef CONFIG_DEBUG_INFO_DWARF4
 KBUILD_CFLAGS	+= $(call cc-option, -gdwarf-4,)

arch/x86/Makefile +5 -2

index f5d7f4134524..b5953cbcc9c8 100644
@@ -235,10 +235,13 @@ archscripts: scripts_basic
 archheaders:
 	$(Q)$(MAKE) $(build)=arch/x86/entry/syscalls all
 
+ASM_MACRO_FILE = arch/x86/kernel/macros.s
+export ASM_MACRO_FILE
+
 archmacros:
-	$(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
+	$(Q)$(MAKE) $(build)=arch/x86/kernel $(ASM_MACRO_FILE)
 
-ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
+ASM_MACRO_FLAGS = -Wa,$(ASM_MACRO_FILE)
 export ASM_MACRO_FLAGS
 KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)

scripts/Makefile.build +28 -2

index 6a6be9f440cf..b8d26bdf48b0 100644
@@ -155,8 +155,34 @@ $(obj)/%.ll: $(src)/%.c FORCE
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 
+# If distcc (or icecc) are used, and when assembly macro files are needed, the
+# compilation stage and the assembly stage needs to be separated. Providing the
+# "IGNORE_DISTCC=y" option disables separate compilation and assembly.
+
+cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
+
+ifneq ($(if $(IGNORE_DISTCC),,$(shell $(CC) --version 2>&1 | head -n 1 | grep -E 'distcc|ICECC')),)
+a_flags_no_debug = $(filter-out $(AFLAGS_DEBUG_INFO), $(a_flags))
+c_flags_no_macros = $(filter-out $(ASM_MACRO_FLAGS), $(c_flags))
+
+cmd_cc_o_c_two_steps =							\
+	$(CC) $(c_flags_no_macros) $(DISABLE_LTO) -fverbose-asm -S	\
+		-o $(@D)/.$(@F:.o=.s) $<;				\
+	cat $(ASM_MACRO_FILE) $(@D)/.$(@F:.o=.s) >			\
+		$(@D)/.tmp_$(@F:.o=.s);					\
+	$(CC) $(a_flags_no_debug) -c -o $(1) $(@D)/.tmp_$(@F:.o=.s);	\
+	rm -f $(@D)/.$(@F:.o=.s) $(@D)/.tmp_$(@F:.o=.s)			\
+
+cmd_cc_o_c_helper =							\
+	$(if $(findstring $(ASM_MACRO_FLAGS),$(c_flags)),		\
+		$(call cmd_cc_o_c_two_steps, $(1)),			\
+		$(call cmd_cc_o_c_direct, $(1)))
+else
+cmd_cc_o_c_helper = $(call cmd_cc_o_c_direct, $(1))
+endif
+
 ifndef CONFIG_MODVERSIONS
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$@)
 
 else
 # When module versioning is enabled the following steps are executed:

@@ -171,7 +197,7 @@ else
 #   replace the unresolved symbols __crc_exported_symbol with
 #   the actual value of the checksum generated by genksyms
 
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$(@D)/.tmp_$(@F))
 
 cmd_modversions_c =								\
 	if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then		\

Signature · 1 line

2.17.1

Masahiro Yamada

2018-11-16 07:46 UTC #2 lore raw

Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 8 lines · Nadav Amit

Introducing the use of asm macros in c-code broke distcc, since it only sends the preprocessed source file. The solution is to break the compilation into two separate phases of compilation and assembly, and between the two concatenate the assembly macros and the compiled (yet not assembled) source file. Since this is less efficient, this compilation mode is only used when distcc or icecc are used.

Quoted 5 lines

Note that the assembly stage should also be distributed, if distcc is configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
Reported-by Logan Gunthorpe <[email protected]>
Signed-off-by Nadav Amit <[email protected]>

Wow, this is so ugly.
I realized how much I hated this by now.

My question is, how long do we need to carry this?

As far as I understood from the long discussion
https://lkml.org/lkml/2018/10/7/92
people are trying to deal with it on the compiler side. Is it right?

https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01932.html

Once it is supported, what would happen on the kernel side?

Quoted 93 lines

---
 Makefile               |  4 +++-
 arch/x86/Makefile      |  7 +++++--
 scripts/Makefile.build | 30 ++++++++++++++++++++++++++++--
 3 files changed, 36 insertions(+), 5 deletions(-)

Makefile +3 -1

index 9fce8b91c15f..c07349fc38c7 100644
@@ -743,7 +743,9 @@ KBUILD_CFLAGS   += $(call cc-option, -gsplit-dwarf, -g)
 else
 KBUILD_CFLAGS  += -g
 endif
-KBUILD_AFLAGS  += -Wa,-gdwarf-2
+AFLAGS_DEBUG_INFO = -Wa,-gdwarf-2
+export AFLAGS_DEBUG_INFO
+KBUILD_AFLAGS  += $(AFLAGS_DEBUG_INFO)
 endif
 ifdef CONFIG_DEBUG_INFO_DWARF4
 KBUILD_CFLAGS  += $(call cc-option, -gdwarf-4,)

arch/x86/Makefile +5 -2

index f5d7f4134524..b5953cbcc9c8 100644
@@ -235,10 +235,13 @@ archscripts: scripts_basic
 archheaders:
        $(Q)$(MAKE) $(build)=arch/x86/entry/syscalls all
 
+ASM_MACRO_FILE = arch/x86/kernel/macros.s
+export ASM_MACRO_FILE
+
 archmacros:
-       $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
+       $(Q)$(MAKE) $(build)=arch/x86/kernel $(ASM_MACRO_FILE)
 
-ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
+ASM_MACRO_FLAGS = -Wa,$(ASM_MACRO_FILE)
 export ASM_MACRO_FLAGS
 KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)

scripts/Makefile.build +28 -3

index 6a6be9f440cf..b8d26bdf48b0 100644
@@ -155,8 +155,34 @@ $(obj)/%.ll: $(src)/%.c FORCE
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 
+# If distcc (or icecc) are used, and when assembly macro files are needed, the
+# compilation stage and the assembly stage needs to be separated. Providing the
+# "IGNORE_DISTCC=y" option disables separate compilation and assembly.
+
+cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
+
+ifneq ($(if $(IGNORE_DISTCC),,$(shell $(CC) --version 2>&1 | head -n 1 | grep -E 'distcc|ICECC')),)
+a_flags_no_debug = $(filter-out $(AFLAGS_DEBUG_INFO), $(a_flags))
+c_flags_no_macros = $(filter-out $(ASM_MACRO_FLAGS), $(c_flags))
+
+cmd_cc_o_c_two_steps =                                                 \
+       $(CC) $(c_flags_no_macros) $(DISABLE_LTO) -fverbose-asm -S      \
+               -o $(@D)/.$(@F:.o=.s) $<;                               \
+       cat $(ASM_MACRO_FILE) $(@D)/.$(@F:.o=.s) >                      \
+               $(@D)/.tmp_$(@F:.o=.s);                                 \
+       $(CC) $(a_flags_no_debug) -c -o $(1) $(@D)/.tmp_$(@F:.o=.s);    \
+       rm -f $(@D)/.$(@F:.o=.s) $(@D)/.tmp_$(@F:.o=.s)                 \
+
+cmd_cc_o_c_helper =                                                    \
+       $(if $(findstring $(ASM_MACRO_FLAGS),$(c_flags)),               \
+               $(call cmd_cc_o_c_two_steps, $(1)),                     \
+               $(call cmd_cc_o_c_direct, $(1)))
+else
+cmd_cc_o_c_helper = $(call cmd_cc_o_c_direct, $(1))
+endif
+
 ifndef CONFIG_MODVERSIONS
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$@)
 
 else
 # When module versioning is enabled the following steps are executed:

@@ -171,7 +197,7 @@ else
 #   replace the unresolved symbols __crc_exported_symbol with
 #   the actual value of the checksum generated by genksyms
 
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$(@D)/.tmp_$(@F))
 
 cmd_modversions_c =                                                            \
        if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then             \
--

2.17.1

Signature · 2 lines

Best Regards
Masahiro Yamada

Nadav Amit

author

2018-11-16 21:01 UTC #3 lore raw

Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

From: Masahiro Yamada
Sent: November 16, 2018 at 7:45:45 AM GMT

Quoted 13 lines

To: Nadav Amit <[email protected]>
Cc: Ingo Molnar <[email protected]>, Michal Marek <[email protected]>, Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. Peter Anvin <[email protected]>, X86 ML <[email protected]>, Linux Kbuild mailing list <[email protected]>, Linux Kernel Mailing List <[email protected]>
Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
Quoted 7 lines · Nadav Amit
Introducing the use of asm macros in c-code broke distcc, since it only sends the preprocessed source file. The solution is to break the compilation into two separate phases of compilation and assembly, and between the two concatenate the assembly macros and the compiled (yet not assembled) source file. Since this is less efficient, this compilation mode is only used when distcc or icecc are used.

Quoted 8 lines

Quoted 5 lines
Note that the assembly stage should also be distributed, if distcc is configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
Reported-by Logan Gunthorpe <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
Wow, this is so ugly.

Indeed, it is not pretty from the Makefile system point of view.

But it does have merits when it comes to the actual use (by developers). If you look on x86, there are a lot of duplicated implementation for C and Asm and crazy C macros, for example for PV function call or the alternative mechanism. The code is also less readable in its current form.

Quoted 3 lines

I realized how much I hated this by now.
My question is, how long do we need to carry this?

If it is purely about performance, I am not sure it would make sense to put it in, as it will indeed be (eventually) solved by the compiler, and the penalty is not too great.

There is also an advantage for having assembly macros that can override the generated assembly that was generated by the compiler. I think HPA (cc’d) looks in this direction (and so do I).

Regards,
Nadav

Masahiro Yamada

2018-11-18 04:21 UTC #4 lore raw

Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 32 lines · Nadav Amit

From: Masahiro Yamada
Sent: November 16, 2018 at 7:45:45 AM GMT
Quoted 13 lines
To: Nadav Amit <[email protected]>
Cc: Ingo Molnar <[email protected]>, Michal Marek <[email protected]>, Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. Peter Anvin <[email protected]>, X86 ML <[email protected]>, Linux Kbuild mailing list <[email protected]>, Linux Kernel Mailing List <[email protected]>
Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
Quoted 7 lines · Nadav Amit
Introducing the use of asm macros in c-code broke distcc, since it only sends the preprocessed source file. The solution is to break the compilation into two separate phases of compilation and assembly, and between the two concatenate the assembly macros and the compiled (yet not assembled) source file. Since this is less efficient, this compilation mode is only used when distcc or icecc are used.
Quoted 8 lines
Quoted 5 lines
Note that the assembly stage should also be distributed, if distcc is configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
Reported-by Logan Gunthorpe <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
Wow, this is so ugly.
Indeed, it is not pretty from the Makefile system point of view.
But it does have merits when it comes to the actual use (by developers). If you look on x86, there are a lot of duplicated implementation for C and Asm and crazy C macros, for example for PV function call or the alternative mechanism. The code is also less readable in its current form.

Quoted 7 lines

Quoted 3 lines
I realized how much I hated this by now.
My question is, how long do we need to carry this?
If it is purely about performance, I am not sure it would make sense to put it in, as it will indeed be (eventually) solved by the compiler, and the penalty is not too great.

It is unfortunate to mess up the code
by having two different ways to achieve the same goal.

When GCC with asm inline support is shipped,
would you revert 77b0bf55 ?

Quoted 6 lines

There is also an advantage for having assembly macros that can override the generated assembly that was generated by the compiler. I think HPA (cc’d) looks in this direction (and so do I).
Regards,
Nadav

Signature · 2 lines

Best Regards
Masahiro Yamada

Nadav Amit

author

2018-11-19 17:18 UTC #5 lore raw

Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 36 lines · Masahiro Yamada

Quoted 35 lines · Nadav Amit
From: Masahiro Yamada
Sent: November 16, 2018 at 7:45:45 AM GMT
Quoted 13 lines
To: Nadav Amit <[email protected]>
Cc: Ingo Molnar <[email protected]>, Michal Marek <[email protected]>, Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. Peter Anvin <[email protected]>, X86 ML <[email protected]>, Linux Kbuild mailing list <[email protected]>, Linux Kernel Mailing List <[email protected]>
Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
Quoted 7 lines · Nadav Amit
Introducing the use of asm macros in c-code broke distcc, since it only sends the preprocessed source file. The solution is to break the compilation into two separate phases of compilation and assembly, and between the two concatenate the assembly macros and the compiled (yet not assembled) source file. Since this is less efficient, this compilation mode is only used when distcc or icecc are used.
Quoted 8 lines
Quoted 5 lines
Note that the assembly stage should also be distributed, if distcc is configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
Reported-by Logan Gunthorpe <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
Wow, this is so ugly.
Indeed, it is not pretty from the Makefile system point of view.
But it does have merits when it comes to the actual use (by developers). If you look on x86, there are a lot of duplicated implementation for C and Asm and crazy C macros, for example for PV function call or the alternative mechanism. The code is also less readable in its current form.
Quoted 3 lines
I realized how much I hated this by now.
My question is, how long do we need to carry this?

Quoted 7 lines

Quoted 3 lines
If it is purely about performance, I am not sure it would make sense to put it in, as it will indeed be (eventually) solved by the compiler, and the penalty is not too great.
It is unfortunate to mess up the code
by having two different ways to achieve the same goal.

Indeed, but I fail to see how this statement fits with the next sentence.

Quoted 2 lines

When GCC with asm inline support is shipped,
would you revert 77b0bf55 ?

This patch and the following ones were not written to be reverted, i.e., reverting them later may not be easy since they remove redundant C inline assembly chunks.

Since gcc will solve the inline assembly issues, the value of these patches is in unifying the assembly that is used in .c and .S files. Due to the lack of m4-like preprocessor in the Linux make-system, the solution is a bit “hacky” (in other words, I don’t see a reasonable alternative solution).

So there is a downside in the form of performance degradation of distcc, and hacky (not too hacky?) Makefile changes. On the upside, except addressing the gcc statement cost computation (inline) issue, the patch removes redundant code, and improves assembly code readability. In addition, it provides the option to make assembly manipulations after compilation, which HPA and others (me) look into.

Anyhow, if after all, the downside is considered greater than the upside, it is best to remove the patches sooner than later, as a revert later will be more painful.

Regards,
Nadav

Masahiro Yamada

2018-11-21 02:48 UTC #6 lore raw

Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 44 lines · Nadav Amit

Quoted 41 lines · Masahiro Yamada
Quoted 38 lines · Nadav Amit
From: Masahiro Yamada
Sent: November 16, 2018 at 7:45:45 AM GMT
Quoted 13 lines
To: Nadav Amit <[email protected]>
Cc: Ingo Molnar <[email protected]>, Michal Marek <[email protected]>, Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. Peter Anvin <[email protected]>, X86 ML <[email protected]>, Linux Kbuild mailing list <[email protected]>, Linux Kernel Mailing List <[email protected]>
Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
Quoted 7 lines · Nadav Amit
Introducing the use of asm macros in c-code broke distcc, since it only sends the preprocessed source file. The solution is to break the compilation into two separate phases of compilation and assembly, and between the two concatenate the assembly macros and the compiled (yet not assembled) source file. Since this is less efficient, this compilation mode is only used when distcc or icecc are used.
Quoted 8 lines
Quoted 5 lines
Note that the assembly stage should also be distributed, if distcc is configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
Reported-by Logan Gunthorpe <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
Wow, this is so ugly.
Indeed, it is not pretty from the Makefile system point of view.
But it does have merits when it comes to the actual use (by developers). If you look on x86, there are a lot of duplicated implementation for C and Asm and crazy C macros, for example for PV function call or the alternative mechanism. The code is also less readable in its current form.
Quoted 3 lines
I realized how much I hated this by now.
My question is, how long do we need to carry this?
If it is purely about performance, I am not sure it would make sense to put it in, as it will indeed be (eventually) solved by the compiler, and the penalty is not too great.

Quoted 4 lines

Quoted 2 lines
It is unfortunate to mess up the code
by having two different ways to achieve the same goal.
Indeed, but I fail to see how this statement fits with the next sentence.

I am not tracking the progress on GCC side.
I just did not sure if the 'asm inline' work was on the right track.

Quoted 12 lines

Quoted 2 lines
When GCC with asm inline support is shipped,
would you revert 77b0bf55 ?
This patch and the following ones were not written to be reverted, i.e., reverting them later may not be easy since they remove redundant C inline assembly chunks.
Since gcc will solve the inline assembly issues, the value of these patches is in unifying the assembly that is used in .c and .S files. Due to the lack of m4-like preprocessor in the Linux make-system, the solution is a bit “hacky” (in other words, I don’t see a reasonable alternative solution).

Quoted 4 lines

So there is a downside in the form of performance degradation of distcc, and hacky (not too hacky?) Makefile changes. On the upside, except addressing the gcc statement cost computation (inline) issue, the patch removes redundant code, and improves assembly code readability.

It is true that the assembly part itself is readable but the readability as a whole is worse IMHO.

Each macro implementation was split into "asm volatile" (in C) + ".macro" (in ASM) parts.

Quoted 7 lines

In addition, it
provides the option to make assembly manipulations after compilation, which HPA and others (me) look into.
Anyhow, if after all, the downside is considered greater than the upside,
it is best to remove the patches sooner than later, as a revert later will be more painful.

Then, I'd be happier with the revert now.

--
Best Regards
Masahiro Yamada

Nadav Amit

author

2018-11-15 04:00 UTC #7 lore raw

PATCH v2 2/2 x86: set a dependency on macros.S

In reply to Nadav Amit

Changes in macros.S should trigger the recompilation of all C files, as the macros might need to affect their compilation.

Acked-by Ingo Molnar <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
 scripts/Makefile.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

scripts/Makefile.build +2 -2

index b8d26bdf48b0..efec77991c2b 100644
@@ -313,13 +313,13 @@ cmd_undef_syms = echo
 endif
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; \

Signature · 1 line

2.17.1

Masahiro Yamada

2018-11-16 07:38 UTC #8 lore raw

Re: [PATCH v2 2/2] x86: set a dependency on macros.S

Quoted 7 lines · Nadav Amit

Changes in macros.S should trigger the recompilation of all C files, as the macros might need to affect their compilation.
Acked-by Ingo Molnar <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
---

When we talked about this last time, we agreed to not do this
because a single line change in asm headers would cause global rebuilding.

Did you change your mind?

Quoted 26 lines

 scripts/Makefile.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

scripts/Makefile.build +2 -3

index b8d26bdf48b0..efec77991c2b 100644
@@ -313,13 +313,13 @@ cmd_undef_syms = echo
 endif
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
        $(call cmd,force_checksrc)
        $(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
        $(call cmd,force_checksrc)
        $(call if_changed_rule,cc_o_c)
        @{ echo $(@:.o=.ko); echo $@; \
--

2.17.1

Signature · 2 lines

Best Regards
Masahiro Yamada

Nadav Amit

author

2018-11-16 20:19 UTC #9 lore raw

Re: [PATCH v2 2/2] x86: set a dependency on macros.S

From: Masahiro Yamada
Sent: November 16, 2018 at 7:37:46 AM GMT

Quoted 13 lines

To: Nadav Amit <[email protected]>
Cc: Ingo Molnar <[email protected]>, Michal Marek <[email protected]>, Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. Peter Anvin <[email protected]>, X86 ML <[email protected]>, Linux Kbuild mailing list <[email protected]>, Linux Kernel Mailing List <[email protected]>
Subject: Re: [PATCH v2 2/2] x86: set a dependency on macros.S
Quoted 6 lines · Nadav Amit
Changes in macros.S should trigger the recompilation of all C files, as the macros might need to affect their compilation.
Acked-by Ingo Molnar <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
---

Quoted 6 lines

When we talked about this last time,
we agreed to not do this
because a single line change in asm headers
would cause global rebuilding.
Did you change your mind?

I was wrong (and you were right? I don’t quite remember). Anyhow, another patch I work on made me realize how wrong I was.

If required, we can extract the macros from some of the header files into separate header files to prevent unnecessary rebuilds.

Thanks,
Nadav