[3/4] Makefile: Replace old suffix rules with pattern rules

  From: Simon Marchi <simon.marchi@polymtl.ca>

As mentioned here [1], suffix rules are obsolete and have been
superseeded with pattern rules.  People (myself included, before writing
this patch) are more likely to know what pattern rules are than suffix
rules.

AFAIK, .SUFFIXES targets are only used for those rules, and can be
removed as well.

New in v2:

  - Replace rule in gdbserver/Makefile.in as well.

[1] https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html

gdb/ChangeLog:

	* Makefile.in (.c.o): Replace rule with ...
	(%.o: %.c): ... this one.
	(.po.gmo): Replace rule with ...
	(%.gmo: %.po): ... this one.
	(.po.pox): Replace rule with ...
	(%.pox: %.po): ... this one.
	(.y.c): Replace rule with ...
	(%.c: %.y): ... this one.
	(.l.c): Replace rule with ...
	(%.c: %.l): ... this one.
	(.SUFFIXES): Remove all instances.

gdb/gdbserver/ChangeLog:

	* Makefile.in (.c.o): Replace rule with ...
	(%.o: %.c): ... this one.
---
 gdb/Makefile.in           | 12 +++++-------
 gdb/gdbserver/Makefile.in |  2 +-
 2 files changed, 6 insertions(+), 8 deletions(-)
  

> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Wed, 16 Nov 2016 11:08:07 -0500
> 
> AFAIK, .SUFFIXES targets are only used for those rules, and can be
> removed as well.

AFAIK, .SUFFIXES affects built-in rules, and so having there only
suffixes relevant to our build will make the build faster, sometimes
much faster, because Make doesn't need to consider irrelevant built-in
rules.

When in doubt, we can ask the GNU Make maintainer to help us.
  

On 11/16/2016 04:34 PM, Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Wed, 16 Nov 2016 11:08:07 -0500
>>
>> AFAIK, .SUFFIXES targets are only used for those rules, and can be
>> removed as well.
> 
> AFAIK, .SUFFIXES affects built-in rules, and so having there only
> suffixes relevant to our build will make the build faster, sometimes
> much faster, because Make doesn't need to consider irrelevant built-in
> rules.
> 

Given the shared ancestry, and the fact that GCC nowadays requires
GNU make, I think it may be worth it to take a look at what
does GCC's Makefile.in do.

In this case, it has:

~~~
# Suppress smart makes who think they know how to automake yacc and flex file
.y.c:
.l.c:

# The only suffixes we want for implicit rules are .c and .o, so clear
# the list and add them.  This speeds up GNU Make, and allows -r to work.
# For i18n support, we also need .gmo, .po, .pox.
# This must come before the language makefile fragments to allow them to
# add suffixes and rules of their own.
.SUFFIXES:
.SUFFIXES: .c .cc .o .po .pox .gmo
~~~

I don't know why they still add some suffixes instead of relying
on the pattern rules.  Might just be legacy.

> When in doubt, we can ask the GNU Make maintainer to help us.

Here's what he was saying in an internal GNU list (about speeding
up make and emptying '.SUFFIXES'):

~~~
This doesn't get rid of all the implicit rules in GNU make, however,
because some default rules are pattern rules which are not affected by
the .SUFFIXES special target.

To get rid of all implicit rules in GNU make you have to either invoke
make with the -r option [...], or else add this to your makefile:

  .SUFFIXES:
  %:: %,v
  %:: RCS/%,v
  %:: RCS/%
  %:: s.%
  %:: SCCS/s.%
~~~

I'd be curious if this makes any difference in a "make" invocation
that ends up building nothing (because all targets are already
up to date).

Thanks,
Pedro Alves
  

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 16 Nov 2016 16:56:02 +0000
> 
> Given the shared ancestry, and the fact that GCC nowadays requires
> GNU make, I think it may be worth it to take a look at what
> does GCC's Makefile.in do.
> 
> In this case, it has:
> 
> ~~~
> # Suppress smart makes who think they know how to automake yacc and flex file
> .y.c:
> .l.c:
> 
> # The only suffixes we want for implicit rules are .c and .o, so clear
> # the list and add them.  This speeds up GNU Make, and allows -r to work.
> # For i18n support, we also need .gmo, .po, .pox.
> # This must come before the language makefile fragments to allow them to
> # add suffixes and rules of their own.
> .SUFFIXES:
> .SUFFIXES: .c .cc .o .po .pox .gmo
> ~~~
> 
> I don't know why they still add some suffixes instead of relying
> on the pattern rules.  Might just be legacy.

No, it's because of the built-in rules.  They are by default
considered no matter which pattern rules you have in the Makefile,
because theoretically each .c file can be built from some other file
in any number of ways.

> This doesn't get rid of all the implicit rules in GNU make, however,
> because some default rules are pattern rules which are not affected by
> the .SUFFIXES special target.
> 
> To get rid of all implicit rules in GNU make you have to either invoke
> make with the -r option [...], or else add this to your makefile:
> 
>   .SUFFIXES:
>   %:: %,v
>   %:: RCS/%,v
>   %:: RCS/%
>   %:: s.%
>   %:: SCCS/s.%
> ~~~
> 
> I'd be curious if this makes any difference in a "make" invocation
> that ends up building nothing (because all targets are already
> up to date).

It might produce a significant difference, but of course the
interesting case is when a small number of files need to be
recompiled.
  

On 11/16/2016 05:14 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Wed, 16 Nov 2016 16:56:02 +0000
>>
>> Given the shared ancestry, and the fact that GCC nowadays requires
>> GNU make, I think it may be worth it to take a look at what
>> does GCC's Makefile.in do.
>>
>> In this case, it has:
>>
>> ~~~
>> # Suppress smart makes who think they know how to automake yacc and flex file
>> .y.c:
>> .l.c:
>>
>> # The only suffixes we want for implicit rules are .c and .o, so clear
>> # the list and add them.  This speeds up GNU Make, and allows -r to work.
>> # For i18n support, we also need .gmo, .po, .pox.
>> # This must come before the language makefile fragments to allow them to
>> # add suffixes and rules of their own.
>> .SUFFIXES:
>> .SUFFIXES: .c .cc .o .po .pox .gmo
>> ~~~
>>
>> I don't know why they still add some suffixes instead of relying
>> on the pattern rules.  Might just be legacy.
> 
> No, it's because of the built-in rules.  They are by default
> considered no matter which pattern rules you have in the Makefile,
> because theoretically each .c file can be built from some other file
> in any number of ways.

I still don't understand.  The question is why they add back
some suffixes _after_ having deleted all the implicit rules.
I.e., why do:

 .SUFFIXES:
 .SUFFIXES: .c

instead of:

 .SUFFIXES:
 %.o: %.c

They use pattern rules for other, more specific cases, AFAICS.

Thanks,
Pedro Alves
  

> Cc: simon.marchi@ericsson.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 16 Nov 2016 17:32:28 +0000
> 
> >> # The only suffixes we want for implicit rules are .c and .o, so clear
> >> # the list and add them.  This speeds up GNU Make, and allows -r to work.
> >> # For i18n support, we also need .gmo, .po, .pox.
> >> # This must come before the language makefile fragments to allow them to
> >> # add suffixes and rules of their own.
> >> .SUFFIXES:
> >> .SUFFIXES: .c .cc .o .po .pox .gmo
> >> ~~~
> >>
> >> I don't know why they still add some suffixes instead of relying
> >> on the pattern rules.  Might just be legacy.
> > 
> > No, it's because of the built-in rules.  They are by default
> > considered no matter which pattern rules you have in the Makefile,
> > because theoretically each .c file can be built from some other file
> > in any number of ways.
> 
> I still don't understand.  The question is why they add back
> some suffixes _after_ having deleted all the implicit rules.
> I.e., why do:
> 
>  .SUFFIXES:
>  .SUFFIXES: .c
> 
> instead of:
> 
>  .SUFFIXES:
>  %.o: %.c
> 
> They use pattern rules for other, more specific cases, AFAICS.

Doesn't the comment explain that?

And who said you should use pattern rules for everything?  Suffix
rules are not a dirty word.
  

On 11/16/2016 05:49 PM, Eli Zaretskii wrote:

> And who said you should use pattern rules for everything?  Suffix
> rules are not a dirty word.

Of course not.  The point was --- is there is perhaps some odd
reason for adding back the default rules, when you have pattern
rules that would match anyway.  I don't know of one, but ...
Anyway, doesn't really matter.

Thanks,
Pedro Alves
  

On 11/16/2016 04:08 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> As mentioned here [1], suffix rules are obsolete and have been
> superseeded with pattern rules.  People (myself included, before writing
> this patch) are more likely to know what pattern rules are than suffix
> rules.
> 
> AFAIK, .SUFFIXES targets are only used for those rules, and can be
> removed as well.
> 
> New in v2:
> 
>   - Replace rule in gdbserver/Makefile.in as well.
> 
> [1] https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html
> 
> gdb/ChangeLog:
> 
> 	* Makefile.in (.c.o): Replace rule with ...
> 	(%.o: %.c): ... this one.
> 	(.po.gmo): Replace rule with ...
> 	(%.gmo: %.po): ... this one.
> 	(.po.pox): Replace rule with ...
> 	(%.pox: %.po): ... this one.
> 	(.y.c): Replace rule with ...
> 	(%.c: %.y): ... this one.
> 	(.l.c): Replace rule with ...
> 	(%.c: %.l): ... this one.
> 	(.SUFFIXES): Remove all instances.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* Makefile.in (.c.o): Replace rule with ...
> 	(%.o: %.c): ... this one.

IMO, whether to explicitly remove default suffixes from the
the implicit rule suffixes list for efficiency is a separate
subject, since we're not currently doing it either.

Just to be sure none of the default suffix rules is necessary,
can you confirm:

1. that "make -r" (from scratch) still works.

2. that "make -r diststuff" in the gdb build dir still works.

If the above work, then this is OK with me to push in.

Thanks,
Pedro Alves
  

On 2016-11-16 11:56, Pedro Alves wrote:
> # Suppress smart makes who think they know how to automake yacc and 
> flex file
> .y.c:
> .l.c:

I don't understand how that can be useful.  According to the GNU make 
doc:

   Suffix rules with no recipe are also meaningless. They do not remove 
previous
   rules as do pattern rules with no recipe (see Canceling Implicit 
Rules). They
   simply enter the suffix or pair of suffixes concatenated as a target 
in the
   data base.

   Source: 
https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html

So those two would be meaningless?

> ~~~
> This doesn't get rid of all the implicit rules in GNU make, however,
> because some default rules are pattern rules which are not affected by
> the .SUFFIXES special target.
> 
> To get rid of all implicit rules in GNU make you have to either invoke
> make with the -r option [...], or else add this to your makefile:
> 
>   .SUFFIXES:
>   %:: %,v
>   %:: RCS/%,v
>   %:: RCS/%
>   %:: s.%
>   %:: SCCS/s.%
> ~~~
> 
> I'd be curious if this makes any difference in a "make" invocation
> that ends up building nothing (because all targets are already
> up to date).

When looking at the make debug output (make -d), I think it becomes 
obvious:

   http://pastebin.com/raw/cETk9W3v

That's taken without .SUFFIXES or other means to disable implicit rules. 
  For _each_ .c file, make tries to determine if it was generated 
somehow, and you can see the lines which the rules you mentioned would 
suppress.  In our case, most C files are not generated, and those that 
are have an explicit rule.  I don't think we rely on implicit rules.  
Since we don't use them, I think it makes sense to disable then as much 
as possible.  Plus, I can imagine them being a possible source of 
"bugs".  If you happen to have a file called infrun.l by accident, the 
the build will fail and you'll wonder why.

I did some experiments, here's the time it takes to run make in the gdb/ 
directory with nothing to re-build.  The other number is the number of 
lines printed when running make -d.  It gives a rough idea of the amount 
of operations make does.

Note that these results are by changing both gdb/Makefile.in and 
gdb/gdbserver/Makefile.in.  That's fair, since the -r applies 
recursively as well.

                               Baseline: 2.5 seconds, 2306335 lines
                         With .SUFFIXES: 0.7 seconds,  307706 lines
With .SUFFIXES and the other %:: rules: 0.6 seconds,  255386 lines
                 With -r flag (make -r): 0.5 seconds,  160682 lines

So I think it shows that it wouldn't hurt to use ".SUFFIXES =" and the 
other rules from the gcc Makefile.  I couldn't manage to get rid of the 
%.{y,l,w} -> %.c implicit rules though no matter what I tried.  Calling 
make with the -r flag was the only way.  At this point the returns are 
minimal though, so I don't think we should worry about it.
  

On 11/16/2016 07:38 PM, Simon Marchi wrote:

> I did some experiments, here's the time it takes to run make in the gdb/
> directory with nothing to re-build.  The other number is the number of
> lines printed when running make -d.  It gives a rough idea of the amount
> of operations make does.
> 
> Note that these results are by changing both gdb/Makefile.in and
> gdb/gdbserver/Makefile.in.  That's fair, since the -r applies
> recursively as well.
> 
>                               Baseline: 2.5 seconds, 2306335 lines
>                         With .SUFFIXES: 0.7 seconds,  307706 lines
> With .SUFFIXES and the other %:: rules: 0.6 seconds,  255386 lines
>                 With -r flag (make -r): 0.5 seconds,  160682 lines

That's a nice speedup.  Presumably if you change gdb/doc/ and
gdb/testsuite/ too, the number without -r gets even closer to
the -r number.

If it works, I think it'll be nice to put the
".SUFFIXES and the other %:: rules" bits in a shared makefile fragment that
is included (with the include directive) by all the main Makefile.in files.

> So I think it shows that it wouldn't hurt to use ".SUFFIXES =" and the
> other rules from the gcc Makefile.  I couldn't manage to get rid of the
> %.{y,l,w} -> %.c implicit rules though no matter what I tried.  Calling
> make with the -r flag was the only way.  At this point the returns are
> minimal though, so I don't think we should worry about it.

Thanks,
Pedro Alves
  

On 2016-11-16 14:58, Pedro Alves wrote:
> On 11/16/2016 07:38 PM, Simon Marchi wrote:
> 
>> I did some experiments, here's the time it takes to run make in the 
>> gdb/
>> directory with nothing to re-build.  The other number is the number of
>> lines printed when running make -d.  It gives a rough idea of the 
>> amount
>> of operations make does.
>> 
>> Note that these results are by changing both gdb/Makefile.in and
>> gdb/gdbserver/Makefile.in.  That's fair, since the -r applies
>> recursively as well.
>> 
>>                               Baseline: 2.5 seconds, 2306335 lines
>>                         With .SUFFIXES: 0.7 seconds,  307706 lines
>> With .SUFFIXES and the other %:: rules: 0.6 seconds,  255386 lines
>>                 With -r flag (make -r): 0.5 seconds,  160682 lines
> 
> That's a nice speedup.  Presumably if you change gdb/doc/ and
> gdb/testsuite/ too, the number without -r gets even closer to
> the -r number.

Right, but not by much I think.  The implicit rules are mostly for yacc, 
lex and c files.  There isn't much target matching those in testsuite 
and doc.

> If it works, I think it'll be nice to put the
> ".SUFFIXES and the other %:: rules" bits in a shared makefile fragment 
> that
> is included (with the include directive) by all the main Makefile.in 
> files.

Good idea.
  

On 2016-11-16 14:10, Pedro Alves wrote:
> IMO, whether to explicitly remove default suffixes from the
> the implicit rule suffixes list for efficiency is a separate
> subject, since we're not currently doing it either.
> 
> Just to be sure none of the default suffix rules is necessary,
> can you confirm:
> 
> 1. that "make -r" (from scratch) still works.

"make -r" from scratch from the top-level fails in the readline 
directory:

   ar: readline.o: No such file or directory

It seems like readline relies on implicit rules.  It shouldn't be 
affected by gdb disabling them though.  I did a "make" in the readline 
directory to make it build, then resume the top-level build with "make 
-r", and it finished cleanly.

> 2. that "make -r diststuff" in the gdb build dir still works.

The commands completes successfully, so it looks good.  Still, perhaps 
Joel should be a little bit more careful when doing the next release to 
make sure nothing it missing.

> If the above work, then this is OK with me to push in.

Just to be clear, this patchset does not disable the default suffix 
rules, so I don't think it was really necessary to check that for this 
patch.  But at least we know it's safe for when we'll want to disable 
them.

Thanks,

Simon
  

On 11/17/2016 04:52 PM, Simon Marchi wrote:
> On 2016-11-16 14:10, Pedro Alves wrote:
>> IMO, whether to explicitly remove default suffixes from the
>> the implicit rule suffixes list for efficiency is a separate
>> subject, since we're not currently doing it either.
>>
>> Just to be sure none of the default suffix rules is necessary,
>> can you confirm:
>>
>> 1. that "make -r" (from scratch) still works.
> 
> "make -r" from scratch from the top-level fails in the readline directory:
> 
>   ar: readline.o: No such file or directory
> 
> It seems like readline relies on implicit rules.  It shouldn't be
> affected by gdb disabling them though.  I did a "make" in the readline
> directory to make it build, then resume the top-level build with "make
> -r", and it finished cleanly.
> 
>> 2. that "make -r diststuff" in the gdb build dir still works.
> 
> The commands completes successfully, so it looks good.  Still, perhaps
> Joel should be a little bit more careful when doing the next release to
> make sure nothing it missing.
> 
>> If the above work, then this is OK with me to push in.
> 
> Just to be clear, this patchset does not disable the default suffix
> rules, so I don't think it was really necessary to check that for this
> patch.  But at least we know it's safe for when we'll want to disable them.

You've changed .pot etc. rules which aren't triggered by a normal
build, so I wanted to be sure that they weren't now working after
the patch just because we'd happen to pick the default rules instead
of the new patterns.  So I think it was useful testing.  In any case,
it works, so I'm happy.  :-)

Thanks,
Pedro Alves