diff mbox series

[v3] genemit.c (main): split insn-emit.c for compiling parallelly

Message ID 20200724045330.37337-1-jiejie_rong@c-sky.com
State New
Headers show
Series [v3] genemit.c (main): split insn-emit.c for compiling parallelly | expand

Commit Message

Jojo R July 24, 2020, 4:53 a.m. UTC
gcc/ChangeLog:

	* genemit.c (main): Print 'split line'.
	* Makefile.in (insn-emit.c): Define split count and file

---
 gcc/Makefile.in | 10 ++++++
 gcc/genemit.c   | 87 ++++++++++++++++++++++++++++---------------------
 2 files changed, 59 insertions(+), 38 deletions(-)

Comments

Rainer Orth July 24, 2020, 8:12 a.m. UTC | #1
Jojo R <jiejie_rong@c-sky.com> writes:

> gcc/ChangeLog:
>
> 	* genemit.c (main): Print 'split line'.
> 	* Makefile.in (insn-emit.c): Define split count and file
[...]
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 2ba76656dbf..75841e49127 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1253,6 +1253,13 @@ ANALYZER_OBJS = \
>  # We put the *-match.o and insn-*.o files first so that a parallel make
>  # will build them sooner, because they are large and otherwise tend to be
>  # the last objects to finish building.
> +
> +insn-generated-split-num = $(shell grep -c ^processor /proc/cpuinfo)

This is highly unportable, probably Linux-only.  It certainly doesn't
exist on at least Solaris and macOS.  Maybe gnulib has something here.

	Rainer
Richard Biener July 24, 2020, 10:03 a.m. UTC | #2
On Fri, Jul 24, 2020 at 10:13 AM Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
>
> Jojo R <jiejie_rong@c-sky.com> writes:
>
> > gcc/ChangeLog:
> >
> >       * genemit.c (main): Print 'split line'.
> >       * Makefile.in (insn-emit.c): Define split count and file
> [...]
> > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> > index 2ba76656dbf..75841e49127 100644
> > --- a/gcc/Makefile.in
> > +++ b/gcc/Makefile.in
> > @@ -1253,6 +1253,13 @@ ANALYZER_OBJS = \
> >  # We put the *-match.o and insn-*.o files first so that a parallel make
> >  # will build them sooner, because they are large and otherwise tend to be
> >  # the last objects to finish building.
> > +
> > +insn-generated-split-num = $(shell grep -c ^processor /proc/cpuinfo)
>
> This is highly unportable, probably Linux-only.  It certainly doesn't
> exist on at least Solaris and macOS.  Maybe gnulib has something here.

It will also produce different build results depending on the build host
which is even more undesirable.

Richard,

>         Rainer
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
Segher Boessenkool July 24, 2020, 1:19 p.m. UTC | #3
On Fri, Jul 24, 2020 at 12:03:16PM +0200, Richard Biener via Gcc-patches wrote:
> On Fri, Jul 24, 2020 at 10:13 AM Rainer Orth
> <ro@cebitec.uni-bielefeld.de> wrote:
> > Jojo R <jiejie_rong@c-sky.com> writes:
> > > +insn-generated-split-num = $(shell grep -c ^processor /proc/cpuinfo)
> >
> > This is highly unportable, probably Linux-only.  It certainly doesn't
> > exist on at least Solaris and macOS.  Maybe gnulib has something here.
> 
> It will also produce different build results depending on the build host
> which is even more undesirable.

Yeah, it has to be the same number everywhere.  Something high enough
that bigger machines benefit (so 100 or so), but not so high that the
overhead increases too much...  It shouldn't be quite as high for
smaller backends...  so some fixed ratio of number of define_insns
perhaps, something like that?

Something else.  Does this make the generated compiler slower?  It will
at least potentially have fewer inlining opportunities, but does that
matter?

Thanks for working on this,


Segher
Joseph Myers July 24, 2020, 1:56 p.m. UTC | #4
On Fri, 24 Jul 2020, Jojo R wrote:

> +	-csplit insn-$*.c /parallel\ compilation/ -k -s {$(insn-generated-split-num)} -f insn-$* -b "%d.c"
> +	-( [ ! -s insn-$*0.c ] && for i in {1..$(insn-generated-split-num)}; do touch insn-$*$$i.c; done && echo "" > insn-$*.c)

Ignoring errors (disk full, out of memory, etc.) here is not appropriate.  
If csplit fails, the build must fail.
Jojo R July 29, 2020, 8:01 a.m. UTC | #5
在 2020年7月24日 +0800 PM4:12,Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>,写道:
> Jojo R <jiejie_rong@c-sky.com> writes:
>
> > gcc/ChangeLog:
> >
> > * genemit.c (main): Print 'split line'.
> > * Makefile.in (insn-emit.c): Define split count and file
> [...]
> > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> > index 2ba76656dbf..75841e49127 100644
> > --- a/gcc/Makefile.in
> > +++ b/gcc/Makefile.in
> > @@ -1253,6 +1253,13 @@ ANALYZER_OBJS = \
> > # We put the *-match.o and insn-*.o files first so that a parallel make
> > # will build them sooner, because they are large and otherwise tend to be
> > # the last objects to finish building.
> > +
> > +insn-generated-split-num = $(shell grep -c ^processor /proc/cpuinfo)
>
> This is highly unportable, probably Linux-only. It certainly doesn't
> exist on at least Solaris and macOS. Maybe gnulib has something here.
>

Thank you and it’s fixed in the next version patch
> Rainer
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
Martin Liška July 29, 2020, 8:10 a.m. UTC | #6
On 7/29/20 10:01 AM, Jojo R wrote:
> Thank you and it’s fixed in the next version patch

Hey.

One small note: can you please link the next v4 email thread to this one?
It's much easier to follow all the discussion related to your patch set.

Thanks,
Martin
Jojo R July 29, 2020, 8:22 a.m. UTC | #7
在 2020年7月24日 +0800 PM6:03,Richard Biener <richard.guenther@gmail.com>,写道:
> On Fri, Jul 24, 2020 at 10:13 AM Rainer Orth
> <ro@cebitec.uni-bielefeld.de> wrote:
> >
> > Jojo R <jiejie_rong@c-sky.com> writes:
> >
> > > gcc/ChangeLog:
> > >
> > > * genemit.c (main): Print 'split line'.
> > > * Makefile.in (insn-emit.c): Define split count and file
> > [...]
> > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> > > index 2ba76656dbf..75841e49127 100644
> > > --- a/gcc/Makefile.in
> > > +++ b/gcc/Makefile.in
> > > @@ -1253,6 +1253,13 @@ ANALYZER_OBJS = \
> > > # We put the *-match.o and insn-*.o files first so that a parallel make
> > > # will build them sooner, because they are large and otherwise tend to be
> > > # the last objects to finish building.
> > > +
> > > +insn-generated-split-num = $(shell grep -c ^processor /proc/cpuinfo)
> >
> > This is highly unportable, probably Linux-only. It certainly doesn't
> > exist on at least Solaris and macOS. Maybe gnulib has something here.
>
> It will also produce different build results depending on the build host
> which is even more undesirable.
>

Theoretically possible it’s the highest performance for building insn-emit.c if all processors are used.
User should not pay much more attention at auto-generated file like insn-emit.c, it’s right ?
> Richard,
>
> > Rainer
> >
> > --
> > -----------------------------------------------------------------------------
> > Rainer Orth, Center for Biotechnology, Bielefeld University
Jojo R July 29, 2020, 8:26 a.m. UTC | #8
在 2020年7月24日 +0800 PM9:57,Joseph Myers <joseph@codesourcery.com>,写道:
> On Fri, 24 Jul 2020, Jojo R wrote:
>
> > + -csplit insn-$*.c /parallel\ compilation/ -k -s {$(insn-generated-split-num)} -f insn-$* -b "%d.c"
> > + -( [ ! -s insn-$*0.c ] && for i in {1..$(insn-generated-split-num)}; do touch insn-$*$$i.c; done && echo "" > insn-$*.c)
>
> Ignoring errors (disk full, out of memory, etc.) here is not appropriate.
> If csplit fails, the build must fail.

Thank you and it’s fixed in the next version patch
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
Jojo R July 29, 2020, 9:02 a.m. UTC | #9
在 2020年7月24日 +0800 PM9:19,Segher Boessenkool <segher@kernel.crashing.org>,写道:
> On Fri, Jul 24, 2020 at 12:03:16PM +0200, Richard Biener via Gcc-patches wrote:
> > On Fri, Jul 24, 2020 at 10:13 AM Rainer Orth
> > <ro@cebitec.uni-bielefeld.de> wrote:
> > > Jojo R <jiejie_rong@c-sky.com> writes:
> > > > +insn-generated-split-num = $(shell grep -c ^processor /proc/cpuinfo)
> > >
> > > This is highly unportable, probably Linux-only. It certainly doesn't
> > > exist on at least Solaris and macOS. Maybe gnulib has something here.
> >
> > It will also produce different build results depending on the build host
> > which is even more undesirable.
>
> Yeah, it has to be the same number everywhere. Something high enough
> that bigger machines benefit (so 100 or so), but not so high that the
> overhead increases too much... It shouldn't be quite as high for
> smaller backends... so some fixed ratio of number of define_insns
> perhaps, something like that?
>

We can get bigger benefit with much more huge file in this simple solution,
Indeed, it does not cover smaller backends.

Yes, we should consider to target all backends, I thinks there is no high enough
benefit for smaller backends, is it necessary ?
> Something else. Does this make the generated compiler slower? It will
> at least potentially have fewer inlining opportunities, but does that
> matter?
>

Yes, some machine will take > 30 minutes in my testcase,
It’s really matter in gcc development stage :(
> Thanks for working on this,
>
>
> Segher
Segher Boessenkool July 29, 2020, 2:44 p.m. UTC | #10
Hi!

On Wed, Jul 29, 2020 at 05:02:04PM +0800, Jojo R wrote:
> 在 2020年7月24日 +0800 PM9:19,Segher Boessenkool <segher@kernel.crashing.org>,写道:
> > On Fri, Jul 24, 2020 at 12:03:16PM +0200, Richard Biener via Gcc-patches wrote:
> > > It will also produce different build results depending on the build host
> > > which is even more undesirable.
> >
> > Yeah, it has to be the same number everywhere. Something high enough
> > that bigger machines benefit (so 100 or so), but not so high that the
> > overhead increases too much... It shouldn't be quite as high for
> > smaller backends... so some fixed ratio of number of define_insns
> > perhaps, something like that?
> 
> We can get bigger benefit with much more huge file in this simple solution,
> Indeed, it does not cover smaller backends.
> 
> Yes, we should consider to target all backends, I thinks there is no high enough
> benefit for smaller backends, is it necessary ?

It should work everywhere.  It does not matter much at all how much it
can speed up tiny backends, but there is a large spread in how big the
bigger backends are, too.  So either we make it adjust itself, or we
will have to tune it manually, and continuously.

> > Something else. Does this make the generated compiler slower? It will
> > at least potentially have fewer inlining opportunities, but does that
> > matter?
> 
> Yes, some machine will take > 30 minutes in my testcase,
> It’s really matter in gcc development stage :(

That wasn't really my question (or I don't understand your answer).


Segher
diff mbox series

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 2ba76656dbf..75841e49127 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1253,6 +1253,13 @@  ANALYZER_OBJS = \
 # We put the *-match.o and insn-*.o files first so that a parallel make
 # will build them sooner, because they are large and otherwise tend to be
 # the last objects to finish building.
+
+insn-generated-split-num = $(shell grep -c ^processor /proc/cpuinfo)
+
+insn-emit-split-c = $(foreach o, $(shell for i in {1..$(insn-generated-split-num)}; do echo $$i; done), insn-emit$(o).c)
+insn-emit-split-obj = $(patsubst %.c,%.o, $(insn-emit-split-c))
+$(insn-emit-split-c): insn-emit.c
+
 OBJS = \
 	gimple-match.o \
 	generic-match.o \
@@ -1260,6 +1267,7 @@  OBJS = \
 	insn-automata.o \
 	insn-dfatab.o \
 	insn-emit.o \
+	$(insn-emit-split-obj) \
 	insn-extract.o \
 	insn-latencytab.o \
 	insn-modes.o \
@@ -2367,6 +2375,8 @@  $(simple_generated_c:insn-%.c=s-%): s-%: build/gen%$(build_exeext)
 	$(RUN_GEN) build/gen$*$(build_exeext) $(md_file) \
 	  $(filter insn-conditions.md,$^) > tmp-$*.c
 	$(SHELL) $(srcdir)/../move-if-change tmp-$*.c insn-$*.c
+	-csplit insn-$*.c /parallel\ compilation/ -k -s {$(insn-generated-split-num)} -f insn-$* -b "%d.c"
+	-( [ ! -s insn-$*0.c ] && for i in {1..$(insn-generated-split-num)}; do touch insn-$*$$i.c; done && echo "" > insn-$*.c)
 	$(STAMP) s-$*
 
 # gencheck doesn't read the machine description, and the file produced
diff --git a/gcc/genemit.c b/gcc/genemit.c
index 84d07d388ee..3aaaeb62b0a 100644
--- a/gcc/genemit.c
+++ b/gcc/genemit.c
@@ -847,6 +847,46 @@  handle_overloaded_gen (overloaded_name *oname)
     }
 }
 
+#define printf_include() do { \
+  printf ("/* Generated automatically by the program `genemit'\n\
+from the machine description file `md'.  */\n\n");      \
+  printf ("#define IN_TARGET_CODE 1\n");                \
+  printf ("#include \"config.h\"\n");                   \
+  printf ("#include \"system.h\"\n");                   \
+  printf ("#include \"coretypes.h\"\n");                \
+  printf ("#include \"backend.h\"\n");                  \
+  printf ("#include \"predict.h\"\n");                  \
+  printf ("#include \"tree.h\"\n");                     \
+  printf ("#include \"rtl.h\"\n");                      \
+  printf ("#include \"alias.h\"\n");                    \
+  printf ("#include \"varasm.h\"\n");                   \
+  printf ("#include \"stor-layout.h\"\n");              \
+  printf ("#include \"calls.h\"\n");                    \
+  printf ("#include \"memmodel.h\"\n");                 \
+  printf ("#include \"tm_p.h\"\n");                     \
+  printf ("#include \"flags.h\"\n");                    \
+  printf ("#include \"insn-config.h\"\n");              \
+  printf ("#include \"expmed.h\"\n");                   \
+  printf ("#include \"dojump.h\"\n");                   \
+  printf ("#include \"explow.h\"\n");                   \
+  printf ("#include \"emit-rtl.h\"\n");                 \
+  printf ("#include \"stmt.h\"\n");                     \
+  printf ("#include \"expr.h\"\n");                     \
+  printf ("#include \"insn-codes.h\"\n");               \
+  printf ("#include \"optabs.h\"\n");                   \
+  printf ("#include \"dfp.h\"\n");                      \
+  printf ("#include \"output.h\"\n");                   \
+  printf ("#include \"recog.h\"\n");                    \
+  printf ("#include \"df.h\"\n");                       \
+  printf ("#include \"resource.h\"\n");                 \
+  printf ("#include \"reload.h\"\n");                   \
+  printf ("#include \"diagnostic-core.h\"\n");          \
+  printf ("#include \"regs.h\"\n");                     \
+  printf ("#include \"tm-constrs.h\"\n");               \
+  printf ("#include \"ggc.h\"\n");                      \
+  printf ("#include \"target.h\"\n\n");                 \
+} while (0)
+
 int
 main (int argc, const char **argv)
 {
@@ -862,49 +902,19 @@  main (int argc, const char **argv)
   /* Assign sequential codes to all entries in the machine description
      in parallel with the tables in insn-output.c.  */
 
-  printf ("/* Generated automatically by the program `genemit'\n\
-from the machine description file `md'.  */\n\n");
-
-  printf ("#define IN_TARGET_CODE 1\n");
-  printf ("#include \"config.h\"\n");
-  printf ("#include \"system.h\"\n");
-  printf ("#include \"coretypes.h\"\n");
-  printf ("#include \"backend.h\"\n");
-  printf ("#include \"predict.h\"\n");
-  printf ("#include \"tree.h\"\n");
-  printf ("#include \"rtl.h\"\n");
-  printf ("#include \"alias.h\"\n");
-  printf ("#include \"varasm.h\"\n");
-  printf ("#include \"stor-layout.h\"\n");
-  printf ("#include \"calls.h\"\n");
-  printf ("#include \"memmodel.h\"\n");
-  printf ("#include \"tm_p.h\"\n");
-  printf ("#include \"flags.h\"\n");
-  printf ("#include \"insn-config.h\"\n");
-  printf ("#include \"expmed.h\"\n");
-  printf ("#include \"dojump.h\"\n");
-  printf ("#include \"explow.h\"\n");
-  printf ("#include \"emit-rtl.h\"\n");
-  printf ("#include \"stmt.h\"\n");
-  printf ("#include \"expr.h\"\n");
-  printf ("#include \"insn-codes.h\"\n");
-  printf ("#include \"optabs.h\"\n");
-  printf ("#include \"dfp.h\"\n");
-  printf ("#include \"output.h\"\n");
-  printf ("#include \"recog.h\"\n");
-  printf ("#include \"df.h\"\n");
-  printf ("#include \"resource.h\"\n");
-  printf ("#include \"reload.h\"\n");
-  printf ("#include \"diagnostic-core.h\"\n");
-  printf ("#include \"regs.h\"\n");
-  printf ("#include \"tm-constrs.h\"\n");
-  printf ("#include \"ggc.h\"\n");
-  printf ("#include \"target.h\"\n\n");
+  long long read_count = 0;
 
   /* Read the machine description.  */
 
   md_rtx_info info;
   while (read_md_rtx (&info))
+    {
+    if (!(read_count++ % 10000))
+      {
+        printf ("/* Split file into separate compilation units for parallel compilation %lld */\n\n", read_count);
+        printf_include();
+      }
+
     switch (GET_CODE (info.def))
       {
       case DEFINE_INSN:
@@ -929,6 +939,7 @@  from the machine description file `md'.  */\n\n");
       default:
 	break;
       }
+    }
 
   /* Write out the routines to add CLOBBERs to a pattern and say whether they
      clobber a hard reg.  */