diff mbox

[BUILDROBOT] frv-linux fallout (was: [PATCH 009/236] Replace BB_HEAD et al macros with functions)

Message ID 20140823184932.GG12042@lug-owl.de
State New
Headers show

Commit Message

Jan-Benedict Glaw Aug. 23, 2014, 6:49 p.m. UTC
On Wed, 2014-08-06 13:19:48 -0400, David Malcolm <dmalcolm@redhat.com> wrote:
> This is further scaffolding; convert the BB_* and SET_BB_* macros
> into functions.  Convert the BB_* rvalue-style functions into returning
> rtx_insn * rather than plain rtx.
[...]

This gave some fallout for frv-linux (see eg. build
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=345281): 

g++ -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. -I. -I/home/vaxbuild/repos/gcc/gcc -I/home/vaxbuild/repos/gcc/gcc/. -I/home/vaxbuild/repos/gcc/gcc/../include -I/home/vaxbuild/repos/gcc/gcc/../libcpp/include  -I/home/vaxbuild/repos/gcc/gcc/../libdecnumber -I/home/vaxbuild/repos/gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I/home/vaxbuild/repos/gcc/gcc/../libbacktrace    -o ifcvt.o -MT ifcvt.o -MMD -MP -MF ./.deps/ifcvt.TPo /home/vaxbuild/repos/gcc/gcc/ifcvt.c
In file included from ./tm.h:23:0,
                 from /home/vaxbuild/repos/gcc/gcc/ifcvt.c:23:
/home/vaxbuild/repos/gcc/gcc/ifcvt.c: In function ‘int cond_exec_process_insns(ce_if_block*, rtx, rtx, rtx, int, int)’:
/home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1964:58: error: ‘frv_ifcvt_modify_insn’ was not declared in this scope
 (PATTERN) = frv_ifcvt_modify_insn (CE_INFO, PATTERN, INSN)
                                                          ^
/home/vaxbuild/repos/gcc/gcc/ifcvt.c:408:7: note: in expansion of macro ‘IFCVT_MODIFY_INSN’
       IFCVT_MODIFY_INSN (ce_info, pattern, insn);
       ^
/home/vaxbuild/repos/gcc/gcc/ifcvt.c: In function ‘int cond_exec_process_if_block(ce_if_block*, int)’:
/home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1948:57: error: ‘frv_ifcvt_modify_tests’ was not declared in this scope
 frv_ifcvt_modify_tests (CE_INFO, &TRUE_EXPR, &FALSE_EXPR)
                                                         ^
/home/vaxbuild/repos/gcc/gcc/ifcvt.c:613:3: note: in expansion of macro ‘IFCVT_MODIFY_TESTS’
   IFCVT_MODIFY_TESTS (ce_info, true_expr, false_expr);
   ^
/home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1957:70: error: ‘frv_ifcvt_modify_multiple_tests’ was not declared in this scope
 frv_ifcvt_modify_multiple_tests (CE_INFO, BB, &TRUE_EXPR, &FALSE_EXPR)
                                                                      ^
/home/vaxbuild/repos/gcc/gcc/ifcvt.c:686:4: note: in expansion of macro ‘IFCVT_MODIFY_MULTIPLE_TESTS’
    IFCVT_MODIFY_MULTIPLE_TESTS (ce_info, bb, t, f);
    ^
/home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1974:70: error: ‘frv_ifcvt_modify_cancel’ was not declared in this scope
 #define IFCVT_MODIFY_CANCEL(CE_INFO) frv_ifcvt_modify_cancel (CE_INFO)
                                                                      ^
/home/vaxbuild/repos/gcc/gcc/ifcvt.c:724:7: note: in expansion of macro ‘IFCVT_MODIFY_CANCEL’
       IFCVT_MODIFY_CANCEL (ce_info);
       ^
/home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1969:68: error: ‘frv_ifcvt_modify_final’ was not declared in this scope
 #define IFCVT_MODIFY_FINAL(CE_INFO) frv_ifcvt_modify_final (CE_INFO)
                                                                    ^
/home/vaxbuild/repos/gcc/gcc/ifcvt.c:731:3: note: in expansion of macro ‘IFCVT_MODIFY_FINAL’
   IFCVT_MODIFY_FINAL (ce_info);
   ^
/home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1974:70: error: ‘frv_ifcvt_modify_cancel’ was not declared in this scope
 #define IFCVT_MODIFY_CANCEL(CE_INFO) frv_ifcvt_modify_cancel (CE_INFO)
                                                                      ^
/home/vaxbuild/repos/gcc/gcc/ifcvt.c:761:3: note: in expansion of macro ‘IFCVT_MODIFY_CANCEL’
   IFCVT_MODIFY_CANCEL (ce_info);
   ^
Makefile:1064: recipe for target 'ifcvt.o' failed





This is because the macro-implementing functions are declared in
frv-protos.h iff BB_HEAD is define'd.  Is this okay to apply?



2014-08-23  Jan-Benedict Glaw  <jbglaw@lug-owl.de>
	* config/frv/frv-protos.h (frv_ifcvt_init_extra_fields): Declare
	unconditionally.
	(frv_ifcvt_modify_tests): Ditto.
	(frv_ifcvt_modify_multiple_tests): Ditto.
	(frv_ifcvt_modify_insn): Ditto.
	(frv_ifcvt_modify_final): Ditto.
	(frv_ifcvt_modify_cancel): Ditto.

Comments

David Malcolm Aug. 25, 2014, 2:08 p.m. UTC | #1
On Sat, 2014-08-23 at 20:49 +0200, Jan-Benedict Glaw wrote:
> On Wed, 2014-08-06 13:19:48 -0400, David Malcolm <dmalcolm@redhat.com> wrote:
> > This is further scaffolding; convert the BB_* and SET_BB_* macros
> > into functions.  Convert the BB_* rvalue-style functions into returning
> > rtx_insn * rather than plain rtx.
> [...]
> 
> This gave some fallout for frv-linux (see eg. build
> http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=345281): 
> 
> g++ -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. -I. -I/home/vaxbuild/repos/gcc/gcc -I/home/vaxbuild/repos/gcc/gcc/. -I/home/vaxbuild/repos/gcc/gcc/../include -I/home/vaxbuild/repos/gcc/gcc/../libcpp/include  -I/home/vaxbuild/repos/gcc/gcc/../libdecnumber -I/home/vaxbuild/repos/gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I/home/vaxbuild/repos/gcc/gcc/../libbacktrace    -o ifcvt.o -MT ifcvt.o -MMD -MP -MF ./.deps/ifcvt.TPo /home/vaxbuild/repos/gcc/gcc/ifcvt.c
> In file included from ./tm.h:23:0,
>                  from /home/vaxbuild/repos/gcc/gcc/ifcvt.c:23:
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c: In function ‘int cond_exec_process_insns(ce_if_block*, rtx, rtx, rtx, int, int)’:
> /home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1964:58: error: ‘frv_ifcvt_modify_insn’ was not declared in this scope
>  (PATTERN) = frv_ifcvt_modify_insn (CE_INFO, PATTERN, INSN)
>                                                           ^
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c:408:7: note: in expansion of macro ‘IFCVT_MODIFY_INSN’
>        IFCVT_MODIFY_INSN (ce_info, pattern, insn);
>        ^
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c: In function ‘int cond_exec_process_if_block(ce_if_block*, int)’:
> /home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1948:57: error: ‘frv_ifcvt_modify_tests’ was not declared in this scope
>  frv_ifcvt_modify_tests (CE_INFO, &TRUE_EXPR, &FALSE_EXPR)
>                                                          ^
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c:613:3: note: in expansion of macro ‘IFCVT_MODIFY_TESTS’
>    IFCVT_MODIFY_TESTS (ce_info, true_expr, false_expr);
>    ^
> /home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1957:70: error: ‘frv_ifcvt_modify_multiple_tests’ was not declared in this scope
>  frv_ifcvt_modify_multiple_tests (CE_INFO, BB, &TRUE_EXPR, &FALSE_EXPR)
>                                                                       ^
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c:686:4: note: in expansion of macro ‘IFCVT_MODIFY_MULTIPLE_TESTS’
>     IFCVT_MODIFY_MULTIPLE_TESTS (ce_info, bb, t, f);
>     ^
> /home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1974:70: error: ‘frv_ifcvt_modify_cancel’ was not declared in this scope
>  #define IFCVT_MODIFY_CANCEL(CE_INFO) frv_ifcvt_modify_cancel (CE_INFO)
>                                                                       ^
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c:724:7: note: in expansion of macro ‘IFCVT_MODIFY_CANCEL’
>        IFCVT_MODIFY_CANCEL (ce_info);
>        ^
> /home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1969:68: error: ‘frv_ifcvt_modify_final’ was not declared in this scope
>  #define IFCVT_MODIFY_FINAL(CE_INFO) frv_ifcvt_modify_final (CE_INFO)
>                                                                     ^
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c:731:3: note: in expansion of macro ‘IFCVT_MODIFY_FINAL’
>    IFCVT_MODIFY_FINAL (ce_info);
>    ^
> /home/vaxbuild/repos/gcc/gcc/config/frv/frv.h:1974:70: error: ‘frv_ifcvt_modify_cancel’ was not declared in this scope
>  #define IFCVT_MODIFY_CANCEL(CE_INFO) frv_ifcvt_modify_cancel (CE_INFO)
>                                                                       ^
> /home/vaxbuild/repos/gcc/gcc/ifcvt.c:761:3: note: in expansion of macro ‘IFCVT_MODIFY_CANCEL’
>    IFCVT_MODIFY_CANCEL (ce_info);
>    ^
> Makefile:1064: recipe for target 'ifcvt.o' failed
> 
> 
> 
> 
> 
> This is because the macro-implementing functions are declared in
> frv-protos.h iff BB_HEAD is define'd.  Is this okay to apply?


Bother.   Sorry about this.

FWIW, BB_HEAD becomes a macro again at patch #178 of the series.

In hindsight, how I've been committing the rtx-classes work is, ahem,
suboptimal.  I'm testing and committing individual patches, but this
could have gone into trunk in one go.

I got it into my head that this could either be an svn branch, *or* be
rebased repeatedly in git until it's ready to go in... but I now think
this is a false dichotomy: what I realize now is that I should have
continued rebasing my patches in git until they were ready to go... but
then committed them to a svn branch, with a ChangeLog.rtx-classes, and
*immediately* merged that branch to trunk.  This would have given the
best of both approaches: the flexibility and resilience against bitrot
of the git rebase approach, but also with the atomicity of the merge to
trunk, and with the whole thing scripted.
 
It's too late now to switch to this approach, so in the meantime I've
been working on ways to make my bootstraps as fast as possible.

Sorry again
Dave


> 2014-08-23  Jan-Benedict Glaw  <jbglaw@lug-owl.de>
> 	* config/frv/frv-protos.h (frv_ifcvt_init_extra_fields): Declare
> 	unconditionally.
> 	(frv_ifcvt_modify_tests): Ditto.
> 	(frv_ifcvt_modify_multiple_tests): Ditto.
> 	(frv_ifcvt_modify_insn): Ditto.
> 	(frv_ifcvt_modify_final): Ditto.
> 	(frv_ifcvt_modify_cancel): Ditto.
> 
> diff --git a/gcc/config/frv/frv-protos.h b/gcc/config/frv/frv-protos.h
> index d50ca64..c689813 100644
> --- a/gcc/config/frv/frv-protos.h
> +++ b/gcc/config/frv/frv-protos.h
> @@ -61,7 +61,6 @@ extern rtx frv_split_minmax		(rtx *);
>  extern rtx frv_split_abs		(rtx *);
>  extern void frv_split_double_load	(rtx, rtx);
>  extern void frv_split_double_store	(rtx, rtx);
> -#ifdef BB_HEAD
>  extern void frv_ifcvt_init_extra_fields	(ce_if_block *);
>  extern void frv_ifcvt_modify_tests	(ce_if_block *, rtx *, rtx *);
>  extern void frv_ifcvt_modify_multiple_tests
> @@ -70,7 +69,6 @@ extern void frv_ifcvt_modify_multiple_tests
>  extern rtx frv_ifcvt_modify_insn	(ce_if_block *, rtx, rtx);
>  extern void frv_ifcvt_modify_final	(ce_if_block *);
>  extern void frv_ifcvt_modify_cancel	(ce_if_block *);
> -#endif
>  extern enum reg_class frv_secondary_reload_class
>  					(enum reg_class,
>  					 enum machine_mode, rtx);
>
Mike Stump Aug. 25, 2014, 7:29 p.m. UTC | #2
On Aug 25, 2014, at 7:08 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> I'm testing and committing individual patches, but this
> could have gone into trunk in one go.

Yeah, it’s a hard choice between bit rot and incrementally going in.

> what I realize now is that I should have
> continued rebasing my patches in git until they were ready to go... but
> then committed them to a svn branch, with a ChangeLog.rtx-classes, and
> *immediately* merged that branch to trunk.

If you go all in at once, you forgot to list the build just after checkin.  :-)  Most work doesn’t require it, but for larger work, it is nice to build post checkin and resolve build errors, if any, when there are any changes from trunk post the last build.

> It's too late now to switch to this approach, so in the meantime I've
> been working on ways to make my bootstraps as fast as possible.

-j64 works wonders.  :-)  Though, it is annoying watching the build run at 98% idle.
Steven Bosscher Aug. 25, 2014, 7:45 p.m. UTC | #3
On Mon, Aug 25, 2014 at 9:29 PM, Mike Stump wrote:
> On Aug 25, 2014, at 7:08 AM, David Malcolm wrote:
>> It's too late now to switch to this approach, so in the meantime I've
>> been working on ways to make my bootstraps as fast as possible.
>
> -j64 works wonders.  :-)  Though, it is annoying watching the build run at 98% idle.

And genautomata just sits there... waiting... waiting... wai......

Ciao!
Steven
Mike Stump Aug. 25, 2014, 7:52 p.m. UTC | #4
On Aug 25, 2014, at 12:45 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Mon, Aug 25, 2014 at 9:29 PM, Mike Stump wrote:
>> On Aug 25, 2014, at 7:08 AM, David Malcolm wrote:
>>> It's too late now to switch to this approach, so in the meantime I've
>>> been working on ways to make my bootstraps as fast as possible.
>> 
>> -j64 works wonders.  :-)  Though, it is annoying watching the build run at 98% idle.
> 
> And genautomata just sits there... waiting... waiting... wai……

Sometimes I want it to run first, sometimes I want it to run last.  :-(  When the build will work, I want it first.  When it fails for any other reason, I want it last.  Can anyone else think of a way to order these guys (apart from a filsystem change hook that is wired into make and auto-rebuilds in the background)?  I haven’t yet found anything I like.
diff mbox

Patch

diff --git a/gcc/config/frv/frv-protos.h b/gcc/config/frv/frv-protos.h
index d50ca64..c689813 100644
--- a/gcc/config/frv/frv-protos.h
+++ b/gcc/config/frv/frv-protos.h
@@ -61,7 +61,6 @@  extern rtx frv_split_minmax		(rtx *);
 extern rtx frv_split_abs		(rtx *);
 extern void frv_split_double_load	(rtx, rtx);
 extern void frv_split_double_store	(rtx, rtx);
-#ifdef BB_HEAD
 extern void frv_ifcvt_init_extra_fields	(ce_if_block *);
 extern void frv_ifcvt_modify_tests	(ce_if_block *, rtx *, rtx *);
 extern void frv_ifcvt_modify_multiple_tests
@@ -70,7 +69,6 @@  extern void frv_ifcvt_modify_multiple_tests
 extern rtx frv_ifcvt_modify_insn	(ce_if_block *, rtx, rtx);
 extern void frv_ifcvt_modify_final	(ce_if_block *);
 extern void frv_ifcvt_modify_cancel	(ce_if_block *);
-#endif
 extern enum reg_class frv_secondary_reload_class
 					(enum reg_class,
 					 enum machine_mode, rtx);