Patchwork [mips] split mips_reorg in pre- and post-dbr_schedule parts

login
register
mail settings
Submitter Steven Bosscher
Date April 14, 2013, 2:20 p.m.
Message ID <CABu31nNszgWG13tTb_hZGw3DTnh0F9XN598+tPPUDKs1Uk1gkA@mail.gmail.com>
Download mbox | patch
Permalink /patch/236446/
State New
Headers show

Comments

Steven Bosscher - April 14, 2013, 2:20 p.m.
Hello,

This patch splits mips_reorg.c in a pre-dbr_schedule part and a new,
machine specific post-dbr_schedule pass. With this patch,
cleanup_barriers and dbr_schedule can be static functions again.

Cross-built&tested mips-sim. OK for trunk?

Ciao!
Steven
* config/mips/mips.c: Include tree-pass.h.
	(mips_reorg): Split in pre- and post-dbr_schedule parts.
	(mips_machine_reorg2): Move mips_reorg post-dbr_schedule parts here.
	(pass_mips_machine_reorg2): New machine specific pass.
	(insert_pass_mips_machine_reorg2): New pass plugin definition.
	(mips_option_override): Register the new pass.
	* rtl.h (cleanup_barriers): Remove prototype.
	(dbr_schedule): Likewise.
	* jump.c (cleanup_barriers): Make static.
	* reorg.c (dbr_schedule): Likewise.
Jeff Law - April 15, 2013, 3:09 p.m.
On 04/14/2013 08:20 AM, Steven Bosscher wrote:
> Hello,
>
> This patch splits mips_reorg.c in a pre-dbr_schedule part and a new,
> machine specific post-dbr_schedule pass. With this patch,
> cleanup_barriers and dbr_schedule can be static functions again.
>
> Cross-built&tested mips-sim. OK for trunk?
>
> Ciao!
> Steven
>
>
> mips_post_dbr_reorg_as_machine_pass.diff.txt
>
>
> 	* config/mips/mips.c: Include tree-pass.h.
> 	(mips_reorg): Split in pre- and post-dbr_schedule parts.
> 	(mips_machine_reorg2): Move mips_reorg post-dbr_schedule parts here.
> 	(pass_mips_machine_reorg2): New machine specific pass.
> 	(insert_pass_mips_machine_reorg2): New pass plugin definition.
> 	(mips_option_override): Register the new pass.
> 	* rtl.h (cleanup_barriers): Remove prototype.
> 	(dbr_schedule): Likewise.
> 	* jump.c (cleanup_barriers): Make static.
> 	* reorg.c (dbr_schedule): Likewise.
The rtl, jump & reorg bits are fine with me.  I don't know enough about 
the MIPS specific bits to comment on them in any meaningful way.

jeff
Steven Bosscher - April 20, 2013, 11:57 a.m.
*ping* MIPS maintainers...

On Mon, Apr 15, 2013 at 5:09 PM, Jeff Law wrote:
> On 04/14/2013 08:20 AM, Steven Bosscher wrote:
>>
>> Hello,
>>
>> This patch splits mips_reorg.c in a pre-dbr_schedule part and a new,
>> machine specific post-dbr_schedule pass. With this patch,
>> cleanup_barriers and dbr_schedule can be static functions again.
>>
>> Cross-built&tested mips-sim. OK for trunk?
>>
>> Ciao!
>> Steven
>>
>>
>> mips_post_dbr_reorg_as_machine_pass.diff.txt
>>
>>
>>         * config/mips/mips.c: Include tree-pass.h.
>>         (mips_reorg): Split in pre- and post-dbr_schedule parts.
>>         (mips_machine_reorg2): Move mips_reorg post-dbr_schedule parts
>> here.
>>         (pass_mips_machine_reorg2): New machine specific pass.
>>         (insert_pass_mips_machine_reorg2): New pass plugin definition.
>>         (mips_option_override): Register the new pass.
>>         * rtl.h (cleanup_barriers): Remove prototype.
>>         (dbr_schedule): Likewise.
>>         * jump.c (cleanup_barriers): Make static.
>>         * reorg.c (dbr_schedule): Likewise.
>
> The rtl, jump & reorg bits are fine with me.  I don't know enough about the
> MIPS specific bits to comment on them in any meaningful way.
>
> jeff
>
Richard Sandiford - April 24, 2013, 6:30 a.m.
Steven Bosscher <stevenb.gcc@gmail.com> writes:
> *ping* MIPS maintainers...

Patch is OK.  Sorry for the slow review, been on holiday.

Richard
Steve Ellcey - May 7, 2013, 3:56 p.m.
Steven,

This patch has broken the GCC build for my mips-mti-linux-gnu and mips-mti-elf builds.  The GCC build dies
when trying to configure libgcc.  Do you have any idea what might be going on?

Steve Ellcey
sellcey@imgtec.com


checking for mips-mti-linux-gnu-gcc... /local/home/sellcey/nightly2/obj-mips-mti-linux-gnu/gcc/initial/./gcc/xgcc -B/local/home/sellcey/nightly2/obj-mips-mti-linux-gnu/gcc/initial/./gcc/ -B/local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/bin/ -B/local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/lib/ -isystem /local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/include -isystem /local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/sys-include
checking for suffix of object files... configure: error: in `/local/home/sellcey/nightly2/obj-mips-mti-linux-gnu/gcc/initial/mips-mti-linux-gnu/libgcc':
configure: error: cannot compute suffix of object files: cannot compile
See `config.log' for more details.
make[1]: *** [configure-target-libgcc] Error 1

From the libgcc/config.log file:

configure:3376: /local/home/sellcey/nightly2/obj-mips-mti-linux-gnu/gcc/initial/./gcc/xgcc -B/local/home/sellcey/nightly2/obj-mips-mti-linux-gnu/gcc/initial/./gcc/ -B/local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/bin/ -B/local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/lib/ -isystem /local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/include -isystem /local/home/sellcey/nightly2/install-mips-mti-linux-gnu/mips-mti-linux-gnu/sys-include    -o conftest -g -O2 -minterlink-mips16   conftest.c  >&5
conftest.c: In function 'main':
conftest.c:16:1: internal compiler error: in update_ssa, at tree-into-ssa.c:3141
 }
 ^
0x95d7e0 update_ssa(unsigned int)
        /local/home/sellcey/nightly2/src/gcc/gcc/tree-into-ssa.c:3141
0x8433fa execute_function_todo
        /local/home/sellcey/nightly2/src/gcc/gcc/passes.c:1942
0x8455bd execute_todo
        /local/home/sellcey/nightly2/src/gcc/gcc/passes.c:2002
Please submit a full bug report,with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Steven Bosscher - May 7, 2013, 4:39 p.m.
On Tue, May 7, 2013 at 5:56 PM, Steve Ellcey wrote:
>
> Steven,
>
> This patch has broken the GCC build for my mips-mti-linux-gnu and mips-mti-elf builds.  The GCC build dies
> when trying to configure libgcc.  Do you have any idea what might be going on?

Hello,

Are you sure this is caused by my patch? It doesn't change anything
for targets other than VR4130.

Ciao!
Steven
Steve Ellcey - May 7, 2013, 4:50 p.m.
On Tue, 2013-05-07 at 18:39 +0200, Steven Bosscher wrote:
> On Tue, May 7, 2013 at 5:56 PM, Steve Ellcey wrote:
> >
> > Steven,
> >
> > This patch has broken the GCC build for my mips-mti-linux-gnu and mips-mti-elf builds.  The GCC build dies
> > when trying to configure libgcc.  Do you have any idea what might be going on?
> 
> Hello,
> 
> Are you sure this is caused by my patch? It doesn't change anything
> for targets other than VR4130.
> 
> Ciao!
> Steven

Yes, I did two builds, one with the git version just before this checkin
and one with the checkin.  This is the patch I am refering to, it
doesn't seem VR4130 specific.

Steve Ellcey
sellcey@imgtec.com

2013-05-06  Steven Bosscher  <steven@gcc.gnu.org>


	* config/mips/mips.c: Include tree-pass.h.
	(mips_reorg): Split in pre- and post-dbr_schedule parts.
	(mips_machine_reorg2): Move mips_reorg post-dbr_schedule parts here.
	(pass_mips_machine_reorg2): New machine specific pass.
	(insert_pass_mips_machine_reorg2): New pass plugin definition.
	(mips_option_override): Register the new pass.
	* rtl.h (cleanup_barriers): Remove prototype.
	(dbr_schedule): Likewise.
	* jump.c (cleanup_barriers): Make static.
	* reorg.c (dbr_schedule): Likewise.
Steven Bosscher - May 7, 2013, 5:14 p.m.
On Tue, May 7, 2013 at 6:50 PM, Steve Ellcey wrote:
> On Tue, 2013-05-07 at 18:39 +0200, Steven Bosscher wrote:
> Yes, I did two builds, one with the git version just before this checkin
> and one with the checkin.  This is the patch I am refering to, it
> doesn't seem VR4130 specific.

Hmm, I can't reproduce it with a cross build. Can you open a PR and
attach a test case and all the other info I need to reproduce this?

Ciao!
Steven
Graham Stott - May 7, 2013, 5:17 p.m.
Steven,

The new execute mips_machine_reorrg2 part of the patch  doesn't have a return value!

Graham

Patch

Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c	(revision 197944)
+++ config/mips/mips.c	(working copy)
@@ -55,6 +55,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "target-globals.h"
 #include "opts.h"
+#include "tree-pass.h"
 
 /* True if X is an UNSPEC wrapper around a SYMBOL_REF or LABEL_REF.  */
 #define UNSPEC_ADDRESS_P(X)					\
@@ -16356,12 +16357,14 @@  mips_reorg (void)
       mips_df_reorg ();
       free_bb_for_insn ();
     }
+}
 
-  if (optimize > 0 && flag_delayed_branch)
-    {
-      cleanup_barriers ();
-      dbr_schedule (get_insns ());
-    }
+/* We use a machine specific pass to do a second machine dependent reorg
+   pass after delay branch scheduling.  */
+
+static unsigned int
+mips_machine_reorg2 (void)
+{
   mips_reorg_process_insns ();
   if (!TARGET_MIPS16
       && TARGET_EXPLICIT_RELOCS
@@ -16374,6 +16377,34 @@  mips_reorg (void)
     mips_reorg_process_insns ();
   mips16_split_long_branches ();
 }
+
+struct rtl_opt_pass pass_mips_machine_reorg2 =
+{
+ {
+  RTL_PASS,
+  "mach2",				/* name */
+  OPTGROUP_NONE,			/* optinfo_flags */
+  NULL,					/* gate */
+  mips_machine_reorg2,			/* execute */
+  NULL,					/* sub */
+  NULL,					/* next */
+  0,					/* static_pass_number */
+  TV_MACH_DEP,				/* tv_id */
+  0,					/* properties_required */
+  0,					/* properties_provided */
+  0,					/* properties_destroyed */
+  0,					/* todo_flags_start */
+  TODO_verify_rtl_sharing,		/* todo_flags_finish */
+ }
+};
+
+struct register_pass_info insert_pass_mips_machine_reorg2 =
+{
+  &pass_mips_machine_reorg2.pass,	/* pass */
+  "dbr",				/* reference_pass_name */
+  1,					/* ref_pass_instance_number */
+  PASS_POS_INSERT_AFTER			/* po_op */
+};
 
 /* Implement TARGET_ASM_OUTPUT_MI_THUNK.  Generate rtl rather than asm text
    in order to avoid duplicating too much logic from elsewhere.  */
@@ -17149,6 +17180,11 @@  mips_option_override (void)
      Do all CPP-sensitive stuff in uncompressed mode; we'll switch modes
      later if required.  */
   mips_set_compression_mode (0);
+
+  /* We register a second machine specific reorg pass after delay slot
+     filling.  Registering the pass must be done at start up.  It's
+     convenient to do it here.  */
+  register_pass (&insert_pass_mips_machine_reorg2);
 }
 
 /* Swap the register information for registers I and I + 1, which
Index: rtl.h
===================================================================
--- rtl.h	(revision 197944)
+++ rtl.h	(working copy)
@@ -1924,7 +1924,6 @@  extern enum rtx_code swap_condition (enum rtx_code
 extern enum rtx_code unsigned_condition (enum rtx_code);
 extern enum rtx_code signed_condition (enum rtx_code);
 extern void mark_jump_label (rtx, rtx, int);
-extern unsigned int cleanup_barriers (void);
 
 /* In jump.c */
 extern rtx delete_related_insns (rtx);
@@ -2668,9 +2667,6 @@  extern void reg_scan (rtx, unsigned int);
 extern void fix_register (const char *, int, int);
 extern bool invalid_mode_change_p (unsigned int, enum reg_class);
 
-/* In reorg.c */
-extern void dbr_schedule (rtx);
-
 /* In reload1.c */
 extern int function_invariant_p (const_rtx);
 
Index: jump.c
===================================================================
--- jump.c	(revision 197944)
+++ jump.c	(working copy)
@@ -118,7 +118,7 @@  rebuild_jump_labels_chain (rtx chain)
    This simple pass moves barriers and removes duplicates so that the
    old code is happy.
  */
-unsigned int
+static unsigned int
 cleanup_barriers (void)
 {
   rtx insn, next, prev;
Index: reorg.c
===================================================================
--- reorg.c	(revision 197944)
+++ reorg.c	(working copy)
@@ -3641,7 +3641,7 @@  make_return_insns (rtx first)
 
 /* Try to find insns to place in delay slots.  */
 
-void
+static void
 dbr_schedule (rtx first)
 {
   rtx insn, next, epilogue_insn = 0;