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(-)
--
2.17.1
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(-)
diff --git a/Makefile b/Makefile
index 9fce8b91c15f..c07349fc38c7 100644
--- a/Makefile
+++ b/Makefile
@@ -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,)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f4134524..b5953cbcc9c8 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -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)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 6a6be9f440cf..d2213b041408 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -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 \
--
2.17.1
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(-)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d2213b041408..ffe3e1a01210 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -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 $@; \
--
2.17.1
* Nadav Amit <[email protected]> wrote:
> 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
> 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 ...
> +# 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:
> +# 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 \
> + -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
* Nadav Amit <[email protected]> wrote:
> 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(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index d2213b041408..ffe3e1a01210 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -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 $@; \
Acked-by: Ingo Molnar <[email protected]>
Thanks,
Ingo
From: Ingo Molnar
Sent: November 13, 2018 at 11:30:00 AM GMT
> 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
>
>
>
> * Nadav Amit <[email protected]> wrote:
>
>> 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
>
>> 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. ]
>> +# 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:
>
>> +# 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
From: Nadav Amit
Sent: November 13, 2018 at 5:55:34 PM GMT
> 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
>> 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
>>
>>
>>
>> * Nadav Amit <[email protected]> wrote:
>>
>>> 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
>>
>>> 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. ]
>
>>> +# 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:
>>
>>> +# 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.
On 13/11/18 11:34 AM, Nadav Amit wrote:
> 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
From: Logan Gunthorpe
Sent: November 14, 2018 at 7:29:38 AM GMT
> 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
>
>
>
>
> On 13/11/18 11:34 AM, Nadav Amit wrote:
>> 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(-)
diff --git a/Makefile b/Makefile
index 9fce8b91c15f..c07349fc38c7 100644
--- a/Makefile
+++ b/Makefile
@@ -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,)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f4134524..b5953cbcc9c8 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -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)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 6a6be9f440cf..bac761c07bf8 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -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 \
--
2.17.1
On 14/11/18 10:46 AM, Nadav Amit wrote:
>
> 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
From: Logan Gunthorpe
Sent: November 15, 2018 at 1:19:45 AM GMT
> 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
>
>
>
>
> On 14/11/18 10:46 AM, Nadav Amit wrote:
>> 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.
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).
On 14/11/18 06:57 PM, Nadav Amit wrote:
> 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.
> 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.
> 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
On 2018-11-14 6:57 p.m., Nadav Amit wrote:
> 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
> On Nov 28, 2018, at 3:09 PM, Logan Gunthorpe <[email protected]> wrote:
>
>
>
> On 2018-11-14 6:57 p.m., Nadav Amit wrote:
>> 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://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ficecc%2Ficecream%2Fpull%2F430&data=02%7C01%7Cnamit%40vmware.com%7C044d795be30143ae26ef08d65586953b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636790433884222635&sdata=4h72yLjvOF29kcAC5CM7CB9ukH3HoALNUfgKA62E4Gs%3D&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?
On 2018-11-28 5:38 p.m., Nadav Amit wrote:
> 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
> On Nov 28, 2018, at 4:49 PM, Logan Gunthorpe <[email protected]> wrote:
>
>
>
> On 2018-11-28 5:38 p.m., Nadav Amit wrote:
>> 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.
A silly `$(CC) —version | grep pump ` test.
Anyhow, it is up to Masahiro.
On 2018-11-28 6:31 p.m., Nadav Amit wrote:
>> On Nov 28, 2018, at 4:49 PM, Logan Gunthorpe <[email protected]> wrote:
>>
>>
>>
>> On 2018-11-28 5:38 p.m., Nadav Amit wrote:
>>> 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.
>
> 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
> On Nov 29, 2018, at 8:43 AM, Logan Gunthorpe <[email protected]> wrote:
>
>
>
> On 2018-11-28 6:31 p.m., Nadav Amit wrote:
>>> On Nov 28, 2018, at 4:49 PM, Logan Gunthorpe <[email protected]> wrote:
>>>
>>>
>>>
>>> On 2018-11-28 5:38 p.m., Nadav Amit wrote:
>>>> 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.
>>
>> 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.