Skip to contentSkip to conversation

x86: Asm macros fixes

PATCH Superseded Nadav Amit 3 participants 18 messages Nov 12, 2018 – Dec 1, 2018

Subsystems (2)
x86 Kbuild

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Nadav Amit

author
#0 lore raw

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.

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 | 33 +++++++++++++++++++++++++++++----
 3 files changed, 37 insertions(+), 7 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH 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 concatanate the assembly macros and the compiled (yet not assembled) source file. Since this is less efficient, this compilation mode is only used when make is called with the "DISTCC=y" parameter.

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 | 29 +++++++++++++++++++++++++++--
 3 files changed, 35 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 +27 -2
index 6a6be9f440cf..d2213b041408 100644
@@ -155,8 +155,33 @@ $(obj)/%.ll: $(src)/%.c FORCE
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 
+# If distcc is used, then when an assembly macro files is needed, the
+# compilation stage and the assembly stage need to be separated. Providing
+# "DISTCC=y" option enables the separate compilation and assembly.
+cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
+
+ifeq ($(DISTCC),y)
+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 +196,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
#2 lore raw

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

Quoted 4 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 concatanate the assembly macros and the compiled (yet

s/concatenate

Quoted 6 lines

not assembled) source file. Since this is less efficient, this compilation mode is only used when make is called with the "DISTCC=y" parameter.

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

It's a bit sad that we regressed distcc performance ...

scripts/Makefile.build $(obj)/%.ll: $(src)/%.c FORCE · line 160

Quoted 3 lines
scripts/Makefile.build +3 -0
+# If distcc is used, then when an assembly macro files is needed, the
+# compilation stage and the assembly stage need to be separated. Providing
+# "DISTCC=y" option enables the separate compilation and assembly.

Let's fix the various typos:

Quoted 15 lines
scripts/Makefile.build +12 -0
+# If distcc is used, and when assembly macro files are needed, the
+# compilation stage and the assembly stage needs to be separated. 
+# Providing the "DISTCC=y" option enables separate compilation and 
+# assembly.
 
 
 
+cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
+
+ifeq ($(DISTCC),y)
+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	\

scripts/Makefile.build $(obj)/%.ll: $(src)/%.c FORCE · line 178

Quoted 10 lines
scripts/Makefile.build +10 -0
+		-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)))

There are various stray <space>+<tab> whitespace noise errors in the chunks above.

Thanks,

Ingo

author
#3 lore raw

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

From: Ingo Molnar
Sent: November 13, 2018 at 11:30:00 AM GMT

Quoted 15 lines

To: Nadav Amit <[email protected]>
Cc: Ingo Molnar <[email protected]>, Masahiro Yamada <[email protected]>, Michal Marek <[email protected]>, Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. Peter Anvin <[email protected]>, [email protected], [email protected], [email protected]
Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 4 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 concatanate the assembly macros and the compiled (yet

s/concatenate

Quoted 8 lines
Quoted 6 lines

not assembled) source file. Since this is less efficient, this compilation mode is only used when make is called with the "DISTCC=y" parameter.

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

It's a bit sad that we regressed distcc performance …

I don’t know what the actual impact is, but Logan, who reported the bug says there is an alternative solution for when distcc-pump is used (which presumably would have ~zero performance degradation). distcc is really fragile IMHO - it’s enough that it finds what looks like two source files in the compiler command arguments for it to fall back to local compilation.

[ In this regard, the distcc-pump solution would *not* work if distcc is built with support for distributed assembly, since it will consider the .s file as a second source file. ]

Quoted 10 lines

scripts/Makefile.build $(obj)/%.ll: $(src)/%.c FORCE · line 160

Quoted 3 lines
scripts/Makefile.build +3 -0
+# If distcc is used, then when an assembly macro files is needed, the
+# compilation stage and the assembly stage need to be separated. Providing
+# "DISTCC=y" option enables the separate compilation and assembly.

Let's fix the various typos:

Review

Quoted 4 lines
(quoted diff) +4 -0
+# If distcc is used, and when assembly macro files are needed, the
+# compilation stage and the assembly stage needs to be separated. 
+# Providing the "DISTCC=y" option enables separate compilation and 
+# assembly.

That’s grammar, not typos ;-)

Sorry for that - I will fix it an send v2 (as well as the whitespace noise).

Regards,
Nadav

author
#4 lore raw

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

From: Nadav Amit
Sent: November 13, 2018 at 5:55:34 PM GMT

Quoted 47 lines

To: Ingo Molnar <[email protected]>
Cc: Ingo Molnar <[email protected]>, Masahiro Yamada <[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]>, LKML <[email protected]>, Logan Gunthorpe <[email protected]>
Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros

From: Ingo Molnar
Sent: November 13, 2018 at 11:30:00 AM GMT

Quoted 15 lines

To: Nadav Amit <[email protected]>
Cc: Ingo Molnar <[email protected]>, Masahiro Yamada <[email protected]>, Michal Marek <[email protected]>, Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. Peter Anvin <[email protected]>, [email protected], [email protected], [email protected]
Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 4 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 concatanate the assembly macros and the compiled (yet

s/concatenate

Quoted 8 lines
Quoted 6 lines

not assembled) source file. Since this is less efficient, this compilation mode is only used when make is called with the "DISTCC=y" parameter.

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

It's a bit sad that we regressed distcc performance …

I don’t know what the actual impact is, but Logan, who reported the bug says there is an alternative solution for when distcc-pump is used (which presumably would have ~zero performance degradation). distcc is really fragile IMHO - it’s enough that it finds what looks like two source files in the compiler command arguments for it to fall back to local compilation.

[ In this regard, the distcc-pump solution would *not* work if distcc is built with support for distributed assembly, since it will consider the .s file as a second source file. ]

Quoted 6 lines

scripts/Makefile.build $(obj)/%.ll: $(src)/%.c FORCE · line 160

Quoted 3 lines
scripts/Makefile.build +3 -0
+# If distcc is used, then when an assembly macro files is needed, the
+# compilation stage and the assembly stage need to be separated. Providing
+# "DISTCC=y" option enables the separate compilation and assembly.

Let's fix the various typos:

Quoted 8 lines
Quoted 4 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+# If distcc is used, and when assembly macro files are needed, the
+# compilation stage and the assembly stage needs to be separated. 
+# Providing the "DISTCC=y" option enables separate compilation and 
+# assembly.

That’s grammar, not typos ;-)

Sorry for that - I will fix it an send v2 (as well as the whitespace noise).

Just one question before I send v2, since I have second thoughts. Does it make sense to require the “DISTCC” make parameter, or should it be set in the Kconfig? It can be detected automatically, the same way gcc/clang are detected or manually through a config option.

#5 lore raw

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

Quoted 4 lines · Nadav Amit

Just one question before I send v2, since I have second thoughts. Does it make sense to require the “DISTCC” make parameter, or should it be set in the Kconfig? It can be detected automatically, the same way gcc/clang are detected or manually through a config option.

I very much prefer the make variable as it can be set in the environment without having to change the Kconfig.

Logan

author
#6 lore raw

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

From: Logan Gunthorpe
Sent: November 14, 2018 at 7:29:38 AM GMT

Quoted 7 lines

To: Nadav Amit <[email protected]>, Ingo Molnar <[email protected]>
Cc: Ingo Molnar <[email protected]>, Masahiro Yamada <[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]>, LKML <[email protected]>
Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 8 lines
Quoted 4 lines · Nadav Amit

Just one question before I send v2, since I have second thoughts. Does it make sense to require the “DISTCC” make parameter, or should it be set in the Kconfig? It can be detected automatically, the same way gcc/clang are detected or manually through a config option.

I very much prefer the make variable as it can be set in the environment without having to change the Kconfig.

Actually, we can just figure out whether distcc or icecc are used in the Makefile according to the “version”, similarly to the way clang is detected. This would neither require new Makefile arguments or Kconfig options.

What do you say about the following?

-- >8 --

Subject: [PATCH] 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..bac761c07bf8 100644
@@ -155,8 +155,34 @@ $(obj)/%.ll: $(src)/%.c FORCE
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 
+# If distcc is used, and when assembly macro files are needed, the compilation
+# stage and the assembly stage needs to be separated. Providing the "DISTCC=y"
+# option enables separate compilation and assembly.
+
+cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
+
+ifneq ($(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
#7 lore raw

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

Quoted 6 lines · Nadav Amit

Actually, we can just figure out whether distcc or icecc are used in the Makefile according to the “version”, similarly to the way clang is detected. This would neither require new Makefile arguments or Kconfig options.

What do you say about the following?

That may be a good idea, but I was kind of hoping to be able to add support for this to at least a future version of icecc (though I'm not committing to that...). So it would be nice to have a way to force it one way or the other with an environment variable.

Thanks,

Logan

author
#8 lore raw

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

From: Logan Gunthorpe
Sent: November 15, 2018 at 1:19:45 AM GMT

Quoted 12 lines

To: Nadav Amit <[email protected]>, Ingo Molnar <[email protected]>
Cc: Ingo Molnar <[email protected]>, Masahiro Yamada <[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]>, LKML <[email protected]>
Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros

Quoted 4 lines · Nadav Amit

Actually, we can just figure out whether distcc or icecc are used in the Makefile according to the “version”, similarly to the way clang is detected. This would neither require new Makefile arguments or Kconfig options.

Quoted 6 lines
Quoted 1 line

What do you say about the following?

That may be a good idea, but I was kind of hoping to be able to add support for this to at least a future version of icecc (though I'm not committing to that...). So it would be nice to have a way to force it one way or the other with an environment variable.

As long as the argument was *required* to get distcc to work at all, you could expect people would figure out an argument is needed. In this case, I suspect nobody will ever know about this argument (except you).

Eventually, if you get a fix into icecc, we will need to change the Makefile, consider the version number and act accordingly.

Anyhow, I’ll add an argument as you asked, and let Ingo (& others) be the judge(s).

#9 lore raw

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

Quoted 3 lines · Nadav Amit

As long as the argument was *required* to get distcc to work at all, you could expect people would figure out an argument is needed. In this case, I suspect nobody will ever know about this argument (except you).

I agree with this completely.

Quoted 2 lines

Eventually, if you get a fix into icecc, we will need to change the Makefile, consider the version number and act accordingly.

Yeah, that's probably a sensible way forward.

Quoted 1 line

Anyhow, I’ll add an argument as you asked, and let Ingo (& others) be the

An argument to force it to a regular compile would at least help with experimenting with this stuff.

Logan

#10 lore raw

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

In reply to Nadav Amit
Quoted 2 lines · Nadav Amit

Eventually, if you get a fix into icecc, we will need to change the Makefile, consider the version number and act accordingly.

I got a fix pulled into icecc[1] and it works quite well. It ought to be included in the next version of icecc. And distcc (with pump at least) has a patch too. In my experience, icecc does a much better job distributing the kernel build though.

Logan

[1] https://github.com/icecc/icecream/pull/430

author
#11 lore raw

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

Quoted 8 lines · Logan Gunthorpe
Quoted 2 lines · Nadav Amit

Eventually, if you get a fix into icecc, we will need to change the Makefile, consider the version number and act accordingly.

Quoted 8 lines

I got a fix pulled into icecc[1] and it works quite well. It ought to be included in the next version of icecc. And distcc (with pump at least) has a patch too. In my experience, icecc does a much better job distributing the kernel build though.

Logan

[1] https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ficecc%2Ficecream%2Fpull%2F430&amp;data=02%7C01%7Cnamit%40vmware.com%7C044d795be30143ae26ef08d65586953b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636790433884222635&amp;sdata=4h72yLjvOF29kcAC5CM7CB9ukH3HoALNUfgKA62E4Gs%3D&amp;reserved=0

Thanks for taking care of it. This change is certainly simpler and cleaner than the one I came with.

So what’s your take? Would you think this patch is still needed? Should it only be enabled automatically for distcc and not for distcc-pump?

#12 lore raw

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

Quoted 2 lines · Nadav Amit

So what’s your take? Would you think this patch is still needed? Should it only be enabled automatically for distcc and not for distcc-pump?

Not sure. The patch will probably slow things down a lot (seeing assembly is always done locally and there are twice as many compile steps) and will create some confusion once it's possible to disable it for the new versions. Maybe hold off and see if anyone else complains?

I don't really know how you'd detect whether pump is in use or not and I'm uncertain as to whether any of the auto detection can actually be made to be reliable.

Logan

author
#13 lore raw

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

Quoted 8 lines · Logan Gunthorpe
Quoted 2 lines · Nadav Amit

So what’s your take? Would you think this patch is still needed? Should it only be enabled automatically for distcc and not for distcc-pump?

Quoted 8 lines

Not sure. The patch will probably slow things down a lot (seeing assembly is always done locally and there are twice as many compile steps) and will create some confusion once it's possible to disable it for the new versions. Maybe hold off and see if anyone else complains?

I don't really know how you'd detect whether pump is in use or not and I'm uncertain as to whether any of the auto detection can actually be made to be reliable.

A silly $(CC) —version | grep pump test.

Anyhow, it is up to Masahiro.

#14 lore raw

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

Quoted 13 lines · Nadav Amit
Quoted 13 lines · Logan Gunthorpe
Quoted 2 lines · Nadav Amit

So what’s your take? Would you think this patch is still needed? Should it only be enabled automatically for distcc and not for distcc-pump?

Not sure. The patch will probably slow things down a lot (seeing assembly is always done locally and there are twice as many compile steps) and will create some confusion once it's possible to disable it for the new versions. Maybe hold off and see if anyone else complains?

Quoted 5 lines
Quoted 3 lines

I don't really know how you'd detect whether pump is in use or not and I'm uncertain as to whether any of the auto detection can actually be made to be reliable.

A silly $(CC) —version | grep pump test.

Actually I'm not sure that's going to work in all cases. If CC="distcc gcc", then "$(CC) --version" just looks like "gcc --version"...

Logan

author
#15 lore raw

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

Quoted 18 lines · Logan Gunthorpe
Quoted 13 lines · Nadav Amit
Quoted 13 lines · Logan Gunthorpe
Quoted 2 lines · Nadav Amit

So what’s your take? Would you think this patch is still needed? Should it only be enabled automatically for distcc and not for distcc-pump?

Not sure. The patch will probably slow things down a lot (seeing assembly is always done locally and there are twice as many compile steps) and will create some confusion once it's possible to disable it for the new versions. Maybe hold off and see if anyone else complains?

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

I don't really know how you'd detect whether pump is in use or not and I'm uncertain as to whether any of the auto detection can actually be made to be reliable.

A silly $(CC) —version | grep pump test.

Actually I'm not sure that's going to work in all cases. If CC="distcc gcc", then "$(CC) --version" just looks like "gcc --version"...

Err.. You’re right. I just tried distcc --version.

author
#16 lore raw

PATCH 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.

Signed-off-by Nadav Amit <[email protected]>
---
 scripts/Makefile.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
scripts/Makefile.build +2 -2
index d2213b041408..ffe3e1a01210 100644
@@ -312,13 +312,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
Acked-by on v1 2/2
#17 lore raw

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

Quoted 18 lines · Nadav Amit

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

Signed-off-by Nadav Amit <[email protected]>
---
 scripts/Makefile.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
scripts/Makefile.build +1 -1
index d2213b041408..ffe3e1a01210 100644
@@ -312,13 +312,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

scripts/Makefile.build cmd_undef_syms = echo · line 324

Quoted 10 lines
scripts/Makefile.build +1 -1
 	$(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 $@; \
Acked-by Ingo Molnar <[email protected]>

Thanks,

Ingo