[RFC] Prevent unnecessary recompilation for trivial params.def changes

[RFC] Prevent unnecessary recompilation for trivial params.def changes

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

Hi,

this patch adds generation of params.list, a file containing a list of 
names of all the parameters in params.def.

By including params.list in params.h, rather than params.def itself, we 
prevent recompiling the 118 c files that include params.h for trivial 
changes in params.def, such as f.i. changing the default value of a 
parameter.

I did a minimal build with the patch, and tested the behaviour by doing 
both trivial an non-trivial changes in params.def, and rebuilding.

Any comments?

Thanks,
- Tom

[-- Attachment #2: 0006-Add-params.list.patch --]
[-- Type: text/x-patch, Size: 1941 bytes --]

Add params.list

---
 gcc/Makefile.in   | 6 +++++-
 gcc/params-list.h | 4 ++++
 gcc/params.h      | 5 +----
 3 files changed, 10 insertions(+), 5 deletions(-)
 create mode 100644 gcc/params-list.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 3d1c1e5..f1ce154 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2415,7 +2415,7 @@ generated_files = config.h tm.h $(TM_P_H) $(TM_H) multilib.h \
        $(ALL_GTFILES_H) gtype-desc.c gtype-desc.h gcov-iov.h \
        options.h target-hooks-def.h insn-opinit.h \
        common/common-target-hooks-def.h pass-instances.def \
-       c-family/c-target-hooks-def.h
+       c-family/c-target-hooks-def.h params.list
 
 #\f
 # How to compile object files to run on the build machine.
@@ -3236,6 +3236,10 @@ installdirs:
 	$(mkinstalldirs) $(DESTDIR)$(man1dir)
 	$(mkinstalldirs) $(DESTDIR)$(man7dir)
 
+params.list: $(srcdir)/params-list.h $(srcdir)/params.def
+	$(CPP) $(srcdir)/params-list.h | sed 's/^#.*//;/^$$/d' > tmp-params.list
+	$(SHELL) $(srcdir)/../move-if-change tmp-params.list params.list
+
 PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
   toplev.h $(DIAGNOSTIC_CORE_H) $(BASIC_BLOCK_H) $(HASH_TABLE_H) \
   tree-ssa-alias.h $(INTERNAL_FN_H) gimple-fold.h tree-eh.h gimple-expr.h \
diff --git a/gcc/params-list.h b/gcc/params-list.h
new file mode 100644
index 0000000..49301d2
--- /dev/null
+++ b/gcc/params-list.h
@@ -0,0 +1,4 @@
+#define DEFPARAM(enumerator, option, nocmsgid, default, min, max) \
+  enumerator,
+#include "params.def"
+#undef DEFPARAM
diff --git a/gcc/params.h b/gcc/params.h
index f53426d..9f7618a 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -81,10 +81,7 @@ extern void set_param_value (const char *name, int value,
 
 enum compiler_param
 {
-#define DEFPARAM(enumerator, option, nocmsgid, default, min, max) \
-  enumerator,
-#include "params.def"
-#undef DEFPARAM
+#include "params.list"
   LAST_PARAM
 };
 
-- 
1.9.1

Re: [RFC] Prevent unnecessary recompilation for trivial params.def changes

[Andreas Schwab]

Tom de Vries <Tom_deVries@mentor.com> writes:

> @@ -3236,6 +3236,10 @@ installdirs:
>  	$(mkinstalldirs) $(DESTDIR)$(man1dir)
>  	$(mkinstalldirs) $(DESTDIR)$(man7dir)
>  
> +params.list: $(srcdir)/params-list.h $(srcdir)/params.def
> +	$(CPP) $(srcdir)/params-list.h | sed 's/^#.*//;/^$$/d' > tmp-params.list
> +	$(SHELL) $(srcdir)/../move-if-change tmp-params.list params.list
> +

You need a stamp file to avoid continuous rebuilding, don't you?

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [RFC] Prevent unnecessary recompilation for trivial params.def changes

[Tom de Vries]

On 08/09/15 13:00, Andreas Schwab wrote:
> Tom de Vries <Tom_deVries@mentor.com> writes:
>
>> @@ -3236,6 +3236,10 @@ installdirs:
>>   	$(mkinstalldirs) $(DESTDIR)$(man1dir)
>>   	$(mkinstalldirs) $(DESTDIR)$(man7dir)
>>
>> +params.list: $(srcdir)/params-list.h $(srcdir)/params.def
>> +	$(CPP) $(srcdir)/params-list.h | sed 's/^#.*//;/^$$/d' > tmp-params.list
>> +	$(SHELL) $(srcdir)/../move-if-change tmp-params.list params.list
>> +
>
> You need a stamp file to avoid continuous rebuilding, don't you?
>

After a trivial change and a rebuild, I see the files being rebuild:
...
/usr/bin/gcc-4.6 -E src/gcc/params-list.h | sed 's/^#.*//;/^$/d' > 
tmp-params.list
/bin/bash src/gcc/../move-if-change tmp-params.list params.list
...

After a subsequent rebuild I don't see anything being rebuild. So I 
don't observe 'continuous rebuilding'.

Thanks,
- Tom

Re: [RFC] Prevent unnecessary recompilation for trivial params.def changes

[Andreas Schwab]

Tom de Vries <Tom_deVries@mentor.com> writes:

> After a subsequent rebuild I don't see anything being rebuild. So I don't
> observe 'continuous rebuilding'.

What happens when you just touch params-list.h or params.def?
move-if-change will leave the target untouched when unchanged (that's
the whole point of it), so it will remain older than the dependencies.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[PATCH] Prevent unnecessary recompilation for trivial params.def changes

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

[ was: Re: [RFC] Prevent unnecessary recompilation for trivial 
params.def changes ]

On 08/09/15 14:03, Andreas Schwab wrote:
> Tom de Vries <Tom_deVries@mentor.com> writes:
>
>> After a subsequent rebuild I don't see anything being rebuild. So I don't
>> observe 'continuous rebuilding'.
>
> What happens when you just touch params-list.h or params.def?
> move-if-change will leave the target untouched when unchanged (that's
> the whole point of it), so it will remain older than the dependencies.

I could reproduce the problem using these instructions, thanks.

I also found a bit "On the use of stamps" in gcc/Makefile.in, which 
explains the problem and how to fix things.

Updated patch accordingly.

OK for trunk if bootstrap succeeds?

Thanks,
- Tom


[-- Attachment #2: 0001-Prevent-unnecessary-recompilation-for-trivial-params.patch --]
[-- Type: text/x-patch, Size: 2359 bytes --]

Prevent unnecessary recompilation for trivial params.def changes

2015-09-08  Tom de Vries  <tom@codesourcery.com>

	* Makefile.in (generated_files): Add params.list.
	(params.list, s-params.list): Add rule.
	* params.h (enum compiler_param): Include params-list.h.  Move define
	DEFPARAM, include params.def and undef DEFPARAM ...
	* params-list.h: ... here.  New file.
---
 gcc/Makefile.in   | 8 +++++++-
 gcc/params-list.h | 4 ++++
 gcc/params.h      | 5 +----
 3 files changed, 12 insertions(+), 5 deletions(-)
 create mode 100644 gcc/params-list.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 3d1c1e5..b495bd2 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2415,7 +2415,7 @@ generated_files = config.h tm.h $(TM_P_H) $(TM_H) multilib.h \
        $(ALL_GTFILES_H) gtype-desc.c gtype-desc.h gcov-iov.h \
        options.h target-hooks-def.h insn-opinit.h \
        common/common-target-hooks-def.h pass-instances.def \
-       c-family/c-target-hooks-def.h
+       c-family/c-target-hooks-def.h params.list
 
 #\f
 # How to compile object files to run on the build machine.
@@ -3236,6 +3236,12 @@ installdirs:
 	$(mkinstalldirs) $(DESTDIR)$(man1dir)
 	$(mkinstalldirs) $(DESTDIR)$(man7dir)
 
+params.list: s-params.list; @true
+s-params.list: $(srcdir)/params-list.h $(srcdir)/params.def
+	$(CPP) $(srcdir)/params-list.h | sed 's/^#.*//;/^$$/d' > tmp-params.list
+	$(SHELL) $(srcdir)/../move-if-change tmp-params.list params.list
+	$(STAMP) s-params.list
+
 PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
   toplev.h $(DIAGNOSTIC_CORE_H) $(BASIC_BLOCK_H) $(HASH_TABLE_H) \
   tree-ssa-alias.h $(INTERNAL_FN_H) gimple-fold.h tree-eh.h gimple-expr.h \
diff --git a/gcc/params-list.h b/gcc/params-list.h
new file mode 100644
index 0000000..49301d2
--- /dev/null
+++ b/gcc/params-list.h
@@ -0,0 +1,4 @@
+#define DEFPARAM(enumerator, option, nocmsgid, default, min, max) \
+  enumerator,
+#include "params.def"
+#undef DEFPARAM
diff --git a/gcc/params.h b/gcc/params.h
index f53426d..9f7618a 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -81,10 +81,7 @@ extern void set_param_value (const char *name, int value,
 
 enum compiler_param
 {
-#define DEFPARAM(enumerator, option, nocmsgid, default, min, max) \
-  enumerator,
-#include "params.def"
-#undef DEFPARAM
+#include "params.list"
   LAST_PARAM
 };
 
-- 
1.9.1

Re: [PATCH] Prevent unnecessary recompilation for trivial params.def changes

[Jeff Law]

On 09/08/2015 07:21 AM, Tom de Vries wrote:
> [ was: Re: [RFC] Prevent unnecessary recompilation for trivial
> params.def changes ]
>
> On 08/09/15 14:03, Andreas Schwab wrote:
>> Tom de Vries <Tom_deVries@mentor.com> writes:
>>
>>> After a subsequent rebuild I don't see anything being rebuild. So I
>>> don't
>>> observe 'continuous rebuilding'.
>>
>> What happens when you just touch params-list.h or params.def?
>> move-if-change will leave the target untouched when unchanged (that's
>> the whole point of it), so it will remain older than the dependencies.
>
> I could reproduce the problem using these instructions, thanks.
>
> I also found a bit "On the use of stamps" in gcc/Makefile.in, which
> explains the problem and how to fix things.
>
> Updated patch accordingly.
>
> OK for trunk if bootstrap succeeds?
Yes.
jeff

Re: [PATCH] Prevent unnecessary recompilation for trivial params.def changes

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

On 08/09/15 23:08, Jeff Law wrote:
> On 09/08/2015 07:21 AM, Tom de Vries wrote:
>> [ was: Re: [RFC] Prevent unnecessary recompilation for trivial
>> params.def changes ]
>>
>> On 08/09/15 14:03, Andreas Schwab wrote:
>>> Tom de Vries <Tom_deVries@mentor.com> writes:
>>>
>>>> After a subsequent rebuild I don't see anything being rebuild. So I
>>>> don't
>>>> observe 'continuous rebuilding'.
>>>
>>> What happens when you just touch params-list.h or params.def?
>>> move-if-change will leave the target untouched when unchanged (that's
>>> the whole point of it), so it will remain older than the dependencies.
>>
>> I could reproduce the problem using these instructions, thanks.
>>
>> I also found a bit "On the use of stamps" in gcc/Makefile.in, which
>> explains the problem and how to fix things.
>>
>> Updated patch accordingly.
>>
>> OK for trunk if bootstrap succeeds?
> Yes.

This patch adds the missing copyright notice in params-list.h.

Committed as trivial.

Thanks,
- Tom

[-- Attachment #2: 0001-Add-copyright-in-gcc-params-list.h.patch --]
[-- Type: text/x-patch, Size: 1215 bytes --]

Add copyright in gcc/params-list.h

2015-09-09  Tom de Vries  <tom@codesourcery.com>

	* params-list.h: Add missing copyright notice.
---
 gcc/params-list.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/gcc/params-list.h b/gcc/params-list.h
index 49301d2..ee33ef5 100644
--- a/gcc/params-list.h
+++ b/gcc/params-list.h
@@ -1,3 +1,22 @@
+/* File used to generate params.list
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
 #define DEFPARAM(enumerator, option, nocmsgid, default, min, max) \
   enumerator,
 #include "params.def"
-- 
1.9.1