Re: [PATCH] Fix configure script comments(!?!) (Was: Re: [PATCH] genemit: Split insn-emit.cc into ten files)
> I'm not sure what that means, whether a wrong version of > autoconf/automake was used (though when I accidentally tried that, it > has always complained loudly) or if some environment difference can > cause this. Perhaps I should change the script not to care about > commits though that won't happen soon (or perhaps I should drop the > checks completely) but would people be OK with me checking in the patch > above (with appropriate ChangeLog) to silence buildbot for a while > again? Hmm, I made sure to regenerate them on an Intel compile-farm machine because my local autoconf/automake was too new. I thought that I also regenerated after the last rebase before pushing. But maybe it auto-merged that change at some point and I forgot? Regards Robin
[PATCH] Fix configure script comments(!?!) (Was: Re: [PATCH] genemit: Split insn-emit.cc into ten files)
Hello, On Thu, Oct 12 2023, Robin Dapp wrote: > [...] > gcc/ChangeLog: > > PR bootstrap/84402 > PR target/111600 > > * Makefile.in: Handle split insn-emit.cc. > * configure: Regenerate. > * configure.ac: Add --with-insnemit-partitions. > * genemit.cc (output_peephole2_scratches): Print to file instead > of stdout. > (print_code): Ditto. > (gen_rtx_scratch): Ditto. > (gen_exp): Ditto. > (gen_emit_seq): Ditto. > (emit_c_code): Ditto. > (gen_insn): Ditto. > (gen_expand): Ditto. > (gen_split): Ditto. > (output_add_clobbers): Ditto. > (output_added_clobbers_hard_reg_p): Ditto. > (print_overload_arguments): Ditto. > (print_overload_test): Ditto. > (handle_overloaded_code_for): Ditto. > (handle_overloaded_gen): Ditto. > (print_header): New function. > (handle_arg): New function. > (main): Split output into 10 files. > * gensupport.cc (count_patterns): New function. > * gensupport.h (count_patterns): Define. > * read-md.cc (md_reader::print_md_ptr_loc): Add file argument. > * read-md.h (class md_reader): Change definition. Following this commit, our buildbot script which checks that configure scripts where re-generated correctly is unhappy because it insists comments are wrong, it wants to them to be like this: diff --git a/gcc/configure b/gcc/configure index 4d0357cbc28..0d818ae6850 100755 --- a/gcc/configure +++ b/gcc/configure @@ -2,7 +2,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 19995 "configure" +#line 20003 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -20106,7 +20106,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 20101 "configure" +#line 20109 "configure" #include "confdefs.h" #if HAVE_DLFCN_H I'm not sure what that means, whether a wrong version of autoconf/automake was used (though when I accidentally tried that, it has always complained loudly) or if some environment difference can cause this. Perhaps I should change the script not to care about commits though that won't happen soon (or perhaps I should drop the checks completely) but would people be OK with me checking in the patch above (with appropriate ChangeLog) to silence buildbot for a while again? Thanks, Martin
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
On 10/27/23 13:04, Robin Dapp wrote: After working with Sam off-list (thanks) I managed to get hppa to build. Initially it looked as if hppa just had a very small number of instruction patterns so we wouldn't generate all 10 output files. However, the actual issue (which we will only hit with a low pattern count) was with counting all the patterns vs only counting the patterns that will be output. A wrong pattern count lead to prematurely stopping to write output files. With that corrected, hppa "just works" until I hit linker errors due to relocations - most likely unrelated: bin/ld: unwind-dw2-fde-dip_s.o(.data.rel.ro+0): cannot handle R_PARISC_FPTR64 for __pthread_key_create@@GLIBC_2.34 Attached is v3 that has been bootstrapped and tested on x86 and power10, aarch64 bootstrap was ok, testsuite is still running. A riscv build and testsuite run was successful as well. Regards Robin From 248744c328440bff9cc339d2bf622852cbaac343 Mon Sep 17 00:00:00 2001 From: Robin Dapp Date: Thu, 12 Oct 2023 11:23:26 +0200 Subject: [PATCH v3] genemit: Split insn-emit.cc into several partitions. On riscv insn-emit.cc has grown to over 1.2 mio lines of code and compiling it takes considerable time. Therefore, this patch adjust genemit to create several partitions (insn-emit-1.cc to insn-emit-n.cc). The available patterns are written to the given files in a sequential fashion. Similar to match.pd a configure option --with-emitinsn-partitions=num is introduced that makes the number of partition configurable. gcc/ChangeLog: PR bootstrap/84402 PR target/111600 * Makefile.in: Handle split insn-emit.cc. * configure: Regenerate. * configure.ac: Add --with-insnemit-partitions. * genemit.cc (output_peephole2_scratches): Print to file instead of stdout. (print_code): Ditto. (gen_rtx_scratch): Ditto. (gen_exp): Ditto. (gen_emit_seq): Ditto. (emit_c_code): Ditto. (gen_insn): Ditto. (gen_expand): Ditto. (gen_split): Ditto. (output_add_clobbers): Ditto. (output_added_clobbers_hard_reg_p): Ditto. (print_overload_arguments): Ditto. (print_overload_test): Ditto. (handle_overloaded_code_for): Ditto. (handle_overloaded_gen): Ditto. (print_header): New function. (handle_arg): New function. (main): Split output into 10 files. * gensupport.cc (count_patterns): New function. * gensupport.h (count_patterns): Define. * read-md.cc (md_reader::print_md_ptr_loc): Add file argument. * read-md.h (class md_reader): Change definition. Just one note on testing. I threw this into my tester which ran through its usual set of crosses as well as native emulated builds of alpha, hppa, m68k, sh4, sh4eb, riscv, aarch64. s390x and ppc64le are in progress, and have progressed beyond their build phase. Note that the emulated natives other than risc-v are 3-stage bootstrapped. OK for the trunk. Thanks for taking care of this. I guess I'll need to time a risc-v bootstrap again. It's currently using --disable-bootstrap at configure time in my tester. jeff
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
After working with Sam off-list (thanks) I managed to get hppa to build. Initially it looked as if hppa just had a very small number of instruction patterns so we wouldn't generate all 10 output files. However, the actual issue (which we will only hit with a low pattern count) was with counting all the patterns vs only counting the patterns that will be output. A wrong pattern count lead to prematurely stopping to write output files. With that corrected, hppa "just works" until I hit linker errors due to relocations - most likely unrelated: bin/ld: unwind-dw2-fde-dip_s.o(.data.rel.ro+0): cannot handle R_PARISC_FPTR64 for __pthread_key_create@@GLIBC_2.34 Attached is v3 that has been bootstrapped and tested on x86 and power10, aarch64 bootstrap was ok, testsuite is still running. A riscv build and testsuite run was successful as well. Regards Robin >From 248744c328440bff9cc339d2bf622852cbaac343 Mon Sep 17 00:00:00 2001 From: Robin Dapp Date: Thu, 12 Oct 2023 11:23:26 +0200 Subject: [PATCH v3] genemit: Split insn-emit.cc into several partitions. On riscv insn-emit.cc has grown to over 1.2 mio lines of code and compiling it takes considerable time. Therefore, this patch adjust genemit to create several partitions (insn-emit-1.cc to insn-emit-n.cc). The available patterns are written to the given files in a sequential fashion. Similar to match.pd a configure option --with-emitinsn-partitions=num is introduced that makes the number of partition configurable. gcc/ChangeLog: PR bootstrap/84402 PR target/111600 * Makefile.in: Handle split insn-emit.cc. * configure: Regenerate. * configure.ac: Add --with-insnemit-partitions. * genemit.cc (output_peephole2_scratches): Print to file instead of stdout. (print_code): Ditto. (gen_rtx_scratch): Ditto. (gen_exp): Ditto. (gen_emit_seq): Ditto. (emit_c_code): Ditto. (gen_insn): Ditto. (gen_expand): Ditto. (gen_split): Ditto. (output_add_clobbers): Ditto. (output_added_clobbers_hard_reg_p): Ditto. (print_overload_arguments): Ditto. (print_overload_test): Ditto. (handle_overloaded_code_for): Ditto. (handle_overloaded_gen): Ditto. (print_header): New function. (handle_arg): New function. (main): Split output into 10 files. * gensupport.cc (count_patterns): New function. * gensupport.h (count_patterns): Define. * read-md.cc (md_reader::print_md_ptr_loc): Add file argument. * read-md.h (class md_reader): Change definition. --- gcc/Makefile.in | 36 ++- gcc/configure | 24 +- gcc/configure.ac | 13 ++ gcc/genemit.cc| 542 +- gcc/gensupport.cc | 55 + gcc/gensupport.h | 1 + gcc/read-md.cc| 4 +- gcc/read-md.h | 2 +- 8 files changed, 422 insertions(+), 255 deletions(-) diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 91d6bfbea4d..d8bfad8de15 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -236,6 +236,13 @@ GIMPLE_MATCH_PD_SEQ_O = $(patsubst %, gimple-match-%.o, $(MATCH_SPLITS_SEQ)) GENERIC_MATCH_PD_SEQ_SRC = $(patsubst %, generic-match-%.cc, $(MATCH_SPLITS_SEQ)) GENERIC_MATCH_PD_SEQ_O = $(patsubst %, generic-match-%.o, $(MATCH_SPLITS_SEQ)) +# The number of splits to be made for the insn-emit files. +NUM_INSNEMIT_SPLITS = @DEFAULT_INSNEMIT_PARTITIONS@ +INSNEMIT_SPLITS_SEQ = $(wordlist 1,$(NUM_INSNEMIT_SPLITS),$(one_to_)) +INSNEMIT_SEQ_SRC = $(patsubst %, insn-emit-%.cc, $(INSNEMIT_SPLITS_SEQ)) +INSNEMIT_SEQ_TMP = $(patsubst %, tmp-emit-%.cc, $(INSNEMIT_SPLITS_SEQ)) +INSNEMIT_SEQ_O = $(patsubst %, insn-emit-%.o, $(INSNEMIT_SPLITS_SEQ)) + # These files are to have specific diagnostics suppressed, or are not to # be subject to -Werror: # flex output may yield harmless "no previous prototype" warnings @@ -1356,7 +1363,7 @@ OBJS = \ insn-attrtab.o \ insn-automata.o \ insn-dfatab.o \ - insn-emit.o \ + $(INSNEMIT_SEQ_O) \ insn-extract.o \ insn-latencytab.o \ insn-modes.o \ @@ -1857,7 +1864,8 @@ TREECHECKING = @TREECHECKING@ FULL_DRIVER_NAME=$(target_noncanonical)-gcc-$(version)$(exeext) MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ - insn-output.cc insn-recog.cc insn-emit.cc insn-extract.cc insn-peep.cc \ + insn-output.cc insn-recog.cc $(INSNEMIT_SEQ_SRC) \ + insn-extract.cc insn-peep.cc \ insn-attr.h insn-attr-common.h insn-attrtab.cc insn-dfatab.cc \ insn-latencytab.cc insn-opinit.cc insn-opinit.h insn-preds.cc insn-constants.h \ tm-preds.h tm-constrs.h checksum-options $(GIMPLE_MATCH_PD_SEQ_SRC) \ @@ -2489,11 +2497,11 @@ $(common_out_object_file): $(common_out_file) # and compile them. .PRECIOUS: insn-config.h insn-flags.h insn-codes.h insn-constants.h \ - insn-emit.cc insn-recog.cc insn-extract.cc insn-output.cc insn-peep.cc \ - insn-attr.h
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
On 10/17/23 01:04, Robin Dapp wrote: Natively, things seem fine, but for cross, I get failures on a few targets (hppa2.0-unknown-linux-gnu, hppa64-unknown-linux-gnu). With ./configure --host=x86_64-pc-linux-gnu --target=hppa2.0-unknown-linux-gnu --build=x86_64-pc-linux-gnu && make -j$(nproc), I get a bunch of stuff like: mv: cannot stat 'tmp-emit-9.cc': No such file or directory echo timestamp > s-insn-emit-8 mv: cannot stat 'tmp-emit-10.cc': No such file or directory make[2]: *** [Makefile:2598: s-insn-emit-9] Error 1 make[2]: *** Waiting for unfinished jobs make[2]: *** [Makefile:2598: s-insn-emit-10] Error 1 Thanks. I presume this is not a native vs cross problem (as I have been building crosses with it for some days now) but rather a "race" i.e. missing dependency on the individual output files. Need to re-check this. Yea, that's almost certainly a missed dependency. I'm ready to throw this into the tester once you've got the dependency issue resolved. It's been running behind the last several weeks until I did some revamping of how jobs are fired off to make it more efficient. It seems to be keeping up now. Jeff
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
> Natively, things seem fine, but for cross, I get failures on a few > targets (hppa2.0-unknown-linux-gnu, hppa64-unknown-linux-gnu). > > With ./configure --host=x86_64-pc-linux-gnu > --target=hppa2.0-unknown-linux-gnu --build=x86_64-pc-linux-gnu && make > -j$(nproc), I get a bunch of stuff like: > > mv: cannot stat 'tmp-emit-9.cc': No such file or directory > echo timestamp > s-insn-emit-8 > mv: cannot stat 'tmp-emit-10.cc': No such file or directory > make[2]: *** [Makefile:2598: s-insn-emit-9] Error 1 > make[2]: *** Waiting for unfinished jobs > make[2]: *** [Makefile:2598: s-insn-emit-10] Error 1 Thanks. I presume this is not a native vs cross problem (as I have been building crosses with it for some days now) but rather a "race" i.e. missing dependency on the individual output files. Need to re-check this. Regards Robin
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
Robin Dapp writes: > Hi, > > the attached v2 includes Tamar's suggestion of keeping the current > stdout behavior. When no output files are passed (via -O) the output > is written to stdout as before. > > Tamar also mentioned off-list that, similar to match.pd, it might make > sense to balance the partitions in a better way than a fixed number > of patterns threshold. That's a good idea but I'd rather do that > separately as the current approach already helps considerably. > > Attached v2 was bootstrapped and regtested on power10, aarch64 and > x86 are still running. > Stefan also tested v1 on s390 where the partitioning does not help > but also doesn't slow anything down. insn-emit.cc isn't very large > to begin with on s390. > > Regards > Robin > > From 34d05113a4e3c7e83a4731020307e26c1144af69 Mon Sep 17 00:00:00 2001 > From: Robin Dapp > Date: Thu, 12 Oct 2023 11:23:26 +0200 > Subject: [PATCH v2] genemit: Split insn-emit.cc into several partitions. > > On riscv insn-emit.cc has grown to over 1.2 mio lines of code and > compiling it takes considerable time. > Therefore, this patch adjust genemit to create several partitions > (insn-emit-1.cc to insn-emit-n.cc). In order to do so it first counts > the number of available patterns, calculates the number of patterns per > file and starts a new file whenever that number is reached. > > Similar to match.pd a configure option --with-emitinsn-partitions=num > is introduced that makes the number of partition configurable. > Natively, things seem fine, but for cross, I get failures on a few targets (hppa2.0-unknown-linux-gnu, hppa64-unknown-linux-gnu). With ./configure --host=x86_64-pc-linux-gnu --target=hppa2.0-unknown-linux-gnu --build=x86_64-pc-linux-gnu && make -j$(nproc), I get a bunch of stuff like: mv: cannot stat 'tmp-emit-9.cc': No such file or directory echo timestamp > s-insn-emit-8 mv: cannot stat 'tmp-emit-10.cc': No such file or directory make[2]: *** [Makefile:2598: s-insn-emit-9] Error 1 make[2]: *** Waiting for unfinished jobs make[2]: *** [Makefile:2598: s-insn-emit-10] Error 1
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
Robin Dapp writes: > Hi, > > the attached v2 includes Tamar's suggestion of keeping the current > stdout behavior. When no output files are passed (via -O) the output > is written to stdout as before. > > Tamar also mentioned off-list that, similar to match.pd, it might make > sense to balance the partitions in a better way than a fixed number > of patterns threshold. That's a good idea but I'd rather do that > separately as the current approach already helps considerably. > > Attached v2 was bootstrapped and regtested on power10, aarch64 and > x86 are still running. > Stefan also tested v1 on s390 where the partitioning does not help > but also doesn't slow anything down. insn-emit.cc isn't very large > to begin with on s390. I tested v1 on x86/arm64, I'll do at least the same for v2. (I also backported it to 13 for my own purposes locally and everything seemed fine there.) I didn't notice any change on my x86 machine but it's already quite powerful and I didn't have a chance to try on anything weaker, so I wasn't too surprised. > > Regards > Robin > > From 34d05113a4e3c7e83a4731020307e26c1144af69 Mon Sep 17 00:00:00 2001 > From: Robin Dapp > Date: Thu, 12 Oct 2023 11:23:26 +0200 > Subject: [PATCH v2] genemit: Split insn-emit.cc into several partitions. > > On riscv insn-emit.cc has grown to over 1.2 mio lines of code and > compiling it takes considerable time. > Therefore, this patch adjust genemit to create several partitions > (insn-emit-1.cc to insn-emit-n.cc). In order to do so it first counts > the number of available patterns, calculates the number of patterns per > file and starts a new file whenever that number is reached. > > Similar to match.pd a configure option --with-emitinsn-partitions=num > is introduced that makes the number of partition configurable. > > gcc/ChangeLog: > > PR bootstrap/84402 > PR target/111600 > > * Makefile.in: Handle split insn-emit.cc. > * configure: Regenerate. > * configure.ac: Add --with-insnemit-partitions. > * genemit.cc (output_peephole2_scratches): Print to file instead > of stdout. > (print_code): Ditto. > (gen_rtx_scratch): Ditto. > (gen_exp): Ditto. > (gen_emit_seq): Ditto. > (emit_c_code): Ditto. > (gen_insn): Ditto. > (gen_expand): Ditto. > (gen_split): Ditto. > (output_add_clobbers): Ditto. > (output_added_clobbers_hard_reg_p): Ditto. > (print_overload_arguments): Ditto. > (print_overload_test): Ditto. > (handle_overloaded_code_for): Ditto. > (handle_overloaded_gen): Ditto. > (print_header): New function. > (handle_arg): New function. > (main): Split output into 10 files. > * gensupport.cc (count_patterns): New function. > * gensupport.h (count_patterns): Define. > * read-md.cc (md_reader::print_md_ptr_loc): Add file argument. > * read-md.h (class md_reader): Change definition. > --- > gcc/Makefile.in | 38 +++- > gcc/configure | 24 ++- > gcc/configure.ac | 13 ++ > gcc/genemit.cc| 536 +- > gcc/gensupport.cc | 36 > gcc/gensupport.h | 1 + > gcc/read-md.cc| 4 +- > gcc/read-md.h | 2 +- > 8 files changed, 399 insertions(+), 255 deletions(-) > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index 9cc16268abf..ca0a616f768 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -236,6 +236,13 @@ GIMPLE_MATCH_PD_SEQ_O = $(patsubst %, gimple-match-%.o, > $(MATCH_SPLITS_SEQ)) > GENERIC_MATCH_PD_SEQ_SRC = $(patsubst %, generic-match-%.cc, > $(MATCH_SPLITS_SEQ)) > GENERIC_MATCH_PD_SEQ_O = $(patsubst %, generic-match-%.o, > $(MATCH_SPLITS_SEQ)) > > +# The number of splits to be made for the insn-emit files. > +NUM_INSNEMIT_SPLITS = @DEFAULT_INSNEMIT_PARTITIONS@ > +INSNEMIT_SPLITS_SEQ = $(wordlist 1,$(NUM_INSNEMIT_SPLITS),$(one_to_)) > +INSNEMIT_SEQ_SRC = $(patsubst %, insn-emit-%.cc, $(INSNEMIT_SPLITS_SEQ)) > +INSNEMIT_SEQ_TMP = $(patsubst %, tmp-emit-%.cc, $(INSNEMIT_SPLITS_SEQ)) > +INSNEMIT_SEQ_O = $(patsubst %, insn-emit-%.o, $(INSNEMIT_SPLITS_SEQ)) > + > # These files are to have specific diagnostics suppressed, or are not to > # be subject to -Werror: > # flex output may yield harmless "no previous prototype" warnings > @@ -1354,7 +1361,7 @@ OBJS = \ > insn-attrtab.o \ > insn-automata.o \ > insn-dfatab.o \ > - insn-emit.o \ > + $(INSNEMIT_SEQ_O) \ > insn-extract.o \ > insn-latencytab.o \ > insn-modes.o \ > @@ -1852,7 +1859,8 @@ TREECHECKING = @TREECHECKING@ > FULL_DRIVER_NAME=$(target_noncanonical)-gcc-$(version)$(exeext) > > MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ > - insn-output.cc insn-recog.cc insn-emit.cc insn-extract.cc insn-peep.cc \ > + insn-output.cc insn-recog.cc $(INSNEMIT_SEQ_SRC) \ > + insn-extract.cc insn-peep.cc \ > insn-attr.h insn-attr-common.h insn-attrtab.cc insn-dfatab.cc \ >
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
Hi, the attached v2 includes Tamar's suggestion of keeping the current stdout behavior. When no output files are passed (via -O) the output is written to stdout as before. Tamar also mentioned off-list that, similar to match.pd, it might make sense to balance the partitions in a better way than a fixed number of patterns threshold. That's a good idea but I'd rather do that separately as the current approach already helps considerably. Attached v2 was bootstrapped and regtested on power10, aarch64 and x86 are still running. Stefan also tested v1 on s390 where the partitioning does not help but also doesn't slow anything down. insn-emit.cc isn't very large to begin with on s390. Regards Robin >From 34d05113a4e3c7e83a4731020307e26c1144af69 Mon Sep 17 00:00:00 2001 From: Robin Dapp Date: Thu, 12 Oct 2023 11:23:26 +0200 Subject: [PATCH v2] genemit: Split insn-emit.cc into several partitions. On riscv insn-emit.cc has grown to over 1.2 mio lines of code and compiling it takes considerable time. Therefore, this patch adjust genemit to create several partitions (insn-emit-1.cc to insn-emit-n.cc). In order to do so it first counts the number of available patterns, calculates the number of patterns per file and starts a new file whenever that number is reached. Similar to match.pd a configure option --with-emitinsn-partitions=num is introduced that makes the number of partition configurable. gcc/ChangeLog: PR bootstrap/84402 PR target/111600 * Makefile.in: Handle split insn-emit.cc. * configure: Regenerate. * configure.ac: Add --with-insnemit-partitions. * genemit.cc (output_peephole2_scratches): Print to file instead of stdout. (print_code): Ditto. (gen_rtx_scratch): Ditto. (gen_exp): Ditto. (gen_emit_seq): Ditto. (emit_c_code): Ditto. (gen_insn): Ditto. (gen_expand): Ditto. (gen_split): Ditto. (output_add_clobbers): Ditto. (output_added_clobbers_hard_reg_p): Ditto. (print_overload_arguments): Ditto. (print_overload_test): Ditto. (handle_overloaded_code_for): Ditto. (handle_overloaded_gen): Ditto. (print_header): New function. (handle_arg): New function. (main): Split output into 10 files. * gensupport.cc (count_patterns): New function. * gensupport.h (count_patterns): Define. * read-md.cc (md_reader::print_md_ptr_loc): Add file argument. * read-md.h (class md_reader): Change definition. --- gcc/Makefile.in | 38 +++- gcc/configure | 24 ++- gcc/configure.ac | 13 ++ gcc/genemit.cc| 536 +- gcc/gensupport.cc | 36 gcc/gensupport.h | 1 + gcc/read-md.cc| 4 +- gcc/read-md.h | 2 +- 8 files changed, 399 insertions(+), 255 deletions(-) diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 9cc16268abf..ca0a616f768 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -236,6 +236,13 @@ GIMPLE_MATCH_PD_SEQ_O = $(patsubst %, gimple-match-%.o, $(MATCH_SPLITS_SEQ)) GENERIC_MATCH_PD_SEQ_SRC = $(patsubst %, generic-match-%.cc, $(MATCH_SPLITS_SEQ)) GENERIC_MATCH_PD_SEQ_O = $(patsubst %, generic-match-%.o, $(MATCH_SPLITS_SEQ)) +# The number of splits to be made for the insn-emit files. +NUM_INSNEMIT_SPLITS = @DEFAULT_INSNEMIT_PARTITIONS@ +INSNEMIT_SPLITS_SEQ = $(wordlist 1,$(NUM_INSNEMIT_SPLITS),$(one_to_)) +INSNEMIT_SEQ_SRC = $(patsubst %, insn-emit-%.cc, $(INSNEMIT_SPLITS_SEQ)) +INSNEMIT_SEQ_TMP = $(patsubst %, tmp-emit-%.cc, $(INSNEMIT_SPLITS_SEQ)) +INSNEMIT_SEQ_O = $(patsubst %, insn-emit-%.o, $(INSNEMIT_SPLITS_SEQ)) + # These files are to have specific diagnostics suppressed, or are not to # be subject to -Werror: # flex output may yield harmless "no previous prototype" warnings @@ -1354,7 +1361,7 @@ OBJS = \ insn-attrtab.o \ insn-automata.o \ insn-dfatab.o \ - insn-emit.o \ + $(INSNEMIT_SEQ_O) \ insn-extract.o \ insn-latencytab.o \ insn-modes.o \ @@ -1852,7 +1859,8 @@ TREECHECKING = @TREECHECKING@ FULL_DRIVER_NAME=$(target_noncanonical)-gcc-$(version)$(exeext) MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ - insn-output.cc insn-recog.cc insn-emit.cc insn-extract.cc insn-peep.cc \ + insn-output.cc insn-recog.cc $(INSNEMIT_SEQ_SRC) \ + insn-extract.cc insn-peep.cc \ insn-attr.h insn-attr-common.h insn-attrtab.cc insn-dfatab.cc \ insn-latencytab.cc insn-opinit.cc insn-opinit.h insn-preds.cc insn-constants.h \ tm-preds.h tm-constrs.h checksum-options $(GIMPLE_MATCH_PD_SEQ_SRC) \ @@ -2481,11 +2489,11 @@ $(common_out_object_file): $(common_out_file) # and compile them. .PRECIOUS: insn-config.h insn-flags.h insn-codes.h insn-constants.h \ - insn-emit.cc insn-recog.cc insn-extract.cc insn-output.cc insn-peep.cc \ - insn-attr.h insn-attr-common.h insn-attrtab.cc insn-dfatab.cc \ - insn-latencytab.cc insn-preds.cc
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
> Hmm why? The same callback you use to consume the listed arguments > can be used to consume the list can it not? I may be wrong, but from > what I remember the callback is called when main can't consume an > argv value and it's allowed to eat all remaining input? Ah, I see. If that's possible, then surely it shouldn't be that large a change and I can add it still. Regards Robin
RE: [PATCH] genemit: Split insn-emit.cc into ten files.
> -Original Message- > From: Robin Dapp > Sent: Friday, October 13, 2023 4:15 PM > To: gcc-patches > Cc: rdapp@gmail.com; jeffreyalaw ; Tamar > Christina ; rjie...@linux.alibaba.com > Subject: Re: [PATCH] genemit: Split insn-emit.cc into ten files. > > > Testsuite is unchanged on all but x86 where, strangely, I saw several > > illegal instructions in the pch tests. Those were not reproducible in > > a second manual test suite run. I'm just running another full > > bootstrap and testsuite cycle with the latest trunk. > > Follow-up on the pch tests. The errors are an artifact of my testing. > I usually build unpatched without bootstrap and patched with it, comparing > the testsuite results. > > When bootstrapping unpatched, the same errors occur. When bootstrapping > with --with-arch=native we essentially use a vpxor for a memset which is an > illegal instruction on gc188 of the compile farm. This might be a known bug > but I haven't found something in bugzilla. > > In total: Bootstrap and testsuite unchanged on x86, aarch64 and power10. > > Regarding Tamar's comment: > > > I'll leave the review to Richard, but I think you should adopt the > > same approach taken by the match.pd split, in that you provide the > > list of files as an argument to the genemit instead of the number of > > files. And if no list is provided it outputs to stdout as it does today. > > I think this would involve rewriting the argument handling for all gen* tools > (it > is shared via gensupport). Currently, I make use of the callback and just add > two new options which helps limit the number of changes. Is stdout so > valuable from a debugging point of view that changing it would be > prohibitive? Of course it's a change but I usually grep through insn-emit.cc > anyways and that would still be possible. Debugging here refers to debugging the output of gen*, which usually one does With small examples when modifying the tool itself. > I think this would involve rewriting the argument handling for all gen* tools > (it > is shared via gensupport). Currently, I make use of the callback and just add > two new options which helps limit the number of changes. Hmm why? The same callback you use to consume the listed arguments can be used to consume the list can it not? I may be wrong, but from what I remember the callback is called when main can't consume an argv value and it's allowed to eat all remaining input? But I'll leave it up to Richard. Having to read multiple files to check a change to gen* seems like a usability issue though. Regards, Tamar > > Regards > Robin
Re: [PATCH] genemit: Split insn-emit.cc into ten files.
> Testsuite is unchanged on all but x86 where, strangely, I saw several > illegal instructions in the pch tests. Those were not reproducible > in a second manual test suite run. I'm just running another full > bootstrap and testsuite cycle with the latest trunk. Follow-up on the pch tests. The errors are an artifact of my testing. I usually build unpatched without bootstrap and patched with it, comparing the testsuite results. When bootstrapping unpatched, the same errors occur. When bootstrapping with --with-arch=native we essentially use a vpxor for a memset which is an illegal instruction on gc188 of the compile farm. This might be a known bug but I haven't found something in bugzilla. In total: Bootstrap and testsuite unchanged on x86, aarch64 and power10. Regarding Tamar's comment: > I'll leave the review to Richard, but I think you should adopt the same > approach > taken by the match.pd split, in that you provide the list of files as an > argument > to the genemit instead of the number of files. And if no list is provided it > outputs to stdout as it does today. I think this would involve rewriting the argument handling for all gen* tools (it is shared via gensupport). Currently, I make use of the callback and just add two new options which helps limit the number of changes. Is stdout so valuable from a debugging point of view that changing it would be prohibitive? Of course it's a change but I usually grep through insn-emit.cc anyways and that would still be possible. Regards Robin
RE: [PATCH] genemit: Split insn-emit.cc into ten files.
Hi, Thanks for doing this! I'll leave the review to Richard, but I think you should adopt the same approach taken by the match.pd split, in that you provide the list of files as an argument to the genemit instead of the number of files. And if no list is provided it outputs to stdout as it does today. This will make it possible to still debug these utilities easily as is done today. Cheers, Tamar > -Original Message- > From: Robin Dapp > Sent: Thursday, October 12, 2023 9:45 PM > To: gcc-patches > Cc: rdapp@gmail.com; jeffreyalaw ; Tamar > Christina ; rjie...@linux.alibaba.com > Subject: [PATCH] genemit: Split insn-emit.cc into ten files. > > Hi, > > on riscv insn-emit.cc has grown to over 1.2 mio lines of code and compiling it > takes considerable time. > Therefore, this patch adjust genemit to create ten files insn-emit-1.cc to > insn- > emit-10.cc. In order to do so it first counts the number of available > patterns, > calculates the number of patterns per file and starts a new file whenever that > number is reached. Most of the changes are mechanical - genemit would > output to stdout and I changed it to write to a FILE. > > Similar to match.pd a configure option --with-emitinsn-partitions=num is > introduced that makes the number of partition configurable. > > This survived some bootstraps on aarch64, x86 and power10 as well as > regular cross builds on riscv. I didn't to extensive timing on targets but > on my > machine the compilation of all 10 insn-emit-...cc files for riscv takes about > 40 > seconds now while the full file took roughly > 10 minutes. > > Testsuite is unchanged on all but x86 where, strangely, I saw several illegal > instructions in the pch tests. Those were not reproducible in a second manual > test suite run. I'm just running another full bootstrap and testsuite cycle > with > the latest trunk. > > Still figured I'd send the current state in order to get some feedback about > the > general approach. > > Regards > Robin > > gcc/ChangeLog: > > PR bootstrap/84402 > PR target/111600 > > * Makefile.in: Handle split insn-emit.cc. > * configure: Regenerate. > * configure.ac: Add --with-insnemit-partitions. > * genemit.cc (output_peephole2_scratches): Print to file instead > of stdout. > (print_code): Ditto. > (gen_rtx_scratch): Ditto. > (gen_exp): Ditto. > (gen_emit_seq): Ditto. > (emit_c_code): Ditto. > (gen_insn): Ditto. > (gen_expand): Ditto. > (gen_split): Ditto. > (output_add_clobbers): Ditto. > (output_added_clobbers_hard_reg_p): Ditto. > (print_overload_arguments): Ditto. > (print_overload_test): Ditto. > (handle_overloaded_code_for): Ditto. > (handle_overloaded_gen): Ditto. > (print_header): New function. > (handle_arg): New function. > (main): Split output into 10 files. > * gensupport.cc (count_patterns): New function. > * gensupport.h (count_patterns): Define. > * read-md.cc (md_reader::print_md_ptr_loc): Add file argument. > * read-md.h (class md_reader): Change definition. > --- > gcc/Makefile.in | 37 +++- > gcc/configure | 24 +- > gcc/configure.ac | 13 ++ > gcc/genemit.cc| 542 +- > gcc/gensupport.cc | 36 +++ > gcc/gensupport.h | 1 + > gcc/read-md.cc| 4 +- > gcc/read-md.h | 2 +- > 8 files changed, 404 insertions(+), 255 deletions(-) > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in index > 9cc16268abf..1988327f311 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -236,6 +236,12 @@ GIMPLE_MATCH_PD_SEQ_O = $(patsubst %, > gimple-match-%.o, $(MATCH_SPLITS_SEQ)) GENERIC_MATCH_PD_SEQ_SRC = > $(patsubst %, generic-match-%.cc, $(MATCH_SPLITS_SEQ)) > GENERIC_MATCH_PD_SEQ_O = $(patsubst %, generic-match-%.o, > $(MATCH_SPLITS_SEQ)) > > +# The number of splits to be made for the insn-emit files. > +NUM_INSNEMIT_SPLITS = @DEFAULT_INSNEMIT_PARTITIONS@ > INSNEMIT_SPLITS_SEQ > += $(wordlist 1,$(NUM_INSNEMIT_SPLITS),$(one_to_)) > +INSNEMIT_SEQ_SRC = $(patsubst %, insn-emit-%.cc, > +$(INSNEMIT_SPLITS_SEQ)) INSNEMIT_SEQ_O = $(patsubst %, insn-emit- > %.o, > +$(INSNEMIT_SPLITS_SEQ)) > + > # These files are to have specific diagnostics suppressed, or are not to # > be > subject to -Werror: > # flex output may yield harmless "no previous prototype" warnings @@ - > 1354,7 +1360,7 @@ OBJS = \ > insn-attrtab.o \ > insn-automata.o \ > insn-dfatab.o \ > - insn-emit.o \ > + $(INSNEMIT_SEQ_O) \ > insn-ex
[PATCH] genemit: Split insn-emit.cc into ten files.
Hi, on riscv insn-emit.cc has grown to over 1.2 mio lines of code and compiling it takes considerable time. Therefore, this patch adjust genemit to create ten files insn-emit-1.cc to insn-emit-10.cc. In order to do so it first counts the number of available patterns, calculates the number of patterns per file and starts a new file whenever that number is reached. Most of the changes are mechanical - genemit would output to stdout and I changed it to write to a FILE. Similar to match.pd a configure option --with-emitinsn-partitions=num is introduced that makes the number of partition configurable. This survived some bootstraps on aarch64, x86 and power10 as well as regular cross builds on riscv. I didn't to extensive timing on targets but on my machine the compilation of all 10 insn-emit-...cc files for riscv takes about 40 seconds now while the full file took roughly 10 minutes. Testsuite is unchanged on all but x86 where, strangely, I saw several illegal instructions in the pch tests. Those were not reproducible in a second manual test suite run. I'm just running another full bootstrap and testsuite cycle with the latest trunk. Still figured I'd send the current state in order to get some feedback about the general approach. Regards Robin gcc/ChangeLog: PR bootstrap/84402 PR target/111600 * Makefile.in: Handle split insn-emit.cc. * configure: Regenerate. * configure.ac: Add --with-insnemit-partitions. * genemit.cc (output_peephole2_scratches): Print to file instead of stdout. (print_code): Ditto. (gen_rtx_scratch): Ditto. (gen_exp): Ditto. (gen_emit_seq): Ditto. (emit_c_code): Ditto. (gen_insn): Ditto. (gen_expand): Ditto. (gen_split): Ditto. (output_add_clobbers): Ditto. (output_added_clobbers_hard_reg_p): Ditto. (print_overload_arguments): Ditto. (print_overload_test): Ditto. (handle_overloaded_code_for): Ditto. (handle_overloaded_gen): Ditto. (print_header): New function. (handle_arg): New function. (main): Split output into 10 files. * gensupport.cc (count_patterns): New function. * gensupport.h (count_patterns): Define. * read-md.cc (md_reader::print_md_ptr_loc): Add file argument. * read-md.h (class md_reader): Change definition. --- gcc/Makefile.in | 37 +++- gcc/configure | 24 +- gcc/configure.ac | 13 ++ gcc/genemit.cc| 542 +- gcc/gensupport.cc | 36 +++ gcc/gensupport.h | 1 + gcc/read-md.cc| 4 +- gcc/read-md.h | 2 +- 8 files changed, 404 insertions(+), 255 deletions(-) diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 9cc16268abf..1988327f311 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -236,6 +236,12 @@ GIMPLE_MATCH_PD_SEQ_O = $(patsubst %, gimple-match-%.o, $(MATCH_SPLITS_SEQ)) GENERIC_MATCH_PD_SEQ_SRC = $(patsubst %, generic-match-%.cc, $(MATCH_SPLITS_SEQ)) GENERIC_MATCH_PD_SEQ_O = $(patsubst %, generic-match-%.o, $(MATCH_SPLITS_SEQ)) +# The number of splits to be made for the insn-emit files. +NUM_INSNEMIT_SPLITS = @DEFAULT_INSNEMIT_PARTITIONS@ +INSNEMIT_SPLITS_SEQ = $(wordlist 1,$(NUM_INSNEMIT_SPLITS),$(one_to_)) +INSNEMIT_SEQ_SRC = $(patsubst %, insn-emit-%.cc, $(INSNEMIT_SPLITS_SEQ)) +INSNEMIT_SEQ_O = $(patsubst %, insn-emit-%.o, $(INSNEMIT_SPLITS_SEQ)) + # These files are to have specific diagnostics suppressed, or are not to # be subject to -Werror: # flex output may yield harmless "no previous prototype" warnings @@ -1354,7 +1360,7 @@ OBJS = \ insn-attrtab.o \ insn-automata.o \ insn-dfatab.o \ - insn-emit.o \ + $(INSNEMIT_SEQ_O) \ insn-extract.o \ insn-latencytab.o \ insn-modes.o \ @@ -1852,7 +1858,8 @@ TREECHECKING = @TREECHECKING@ FULL_DRIVER_NAME=$(target_noncanonical)-gcc-$(version)$(exeext) MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ - insn-output.cc insn-recog.cc insn-emit.cc insn-extract.cc insn-peep.cc \ + insn-output.cc insn-recog.cc $(INSNEMIT_SEQ_SRC) \ + insn-extract.cc insn-peep.cc \ insn-attr.h insn-attr-common.h insn-attrtab.cc insn-dfatab.cc \ insn-latencytab.cc insn-opinit.cc insn-opinit.h insn-preds.cc insn-constants.h \ tm-preds.h tm-constrs.h checksum-options $(GIMPLE_MATCH_PD_SEQ_SRC) \ @@ -2481,11 +2488,11 @@ $(common_out_object_file): $(common_out_file) # and compile them. .PRECIOUS: insn-config.h insn-flags.h insn-codes.h insn-constants.h \ - insn-emit.cc insn-recog.cc insn-extract.cc insn-output.cc insn-peep.cc \ - insn-attr.h insn-attr-common.h insn-attrtab.cc insn-dfatab.cc \ - insn-latencytab.cc insn-preds.cc $(GIMPLE_MATCH_PD_SEQ_SRC) \ - $(GENERIC_MATCH_PD_SEQ_SRC) gimple-match-auto.h generic-match-auto.h \ - insn-target-def.h + $(INSNEMIT_SEQ_SRC) insn-recog.cc insn-extract.cc insn-output.cc \ + insn-peep.cc
