RFA: Allow md_reorg to call dbr_schedule

Richard Sandiford rdsandiford@googlemail.com
Sun Jan 4 21:45:00 GMT 2009


...well, OK, MIPS already does this, but it was a hack that no longer
works correctly.

The current approach is to cache the old value of flag_delayed_branch
in mips_base_delayed_branch, so that it can be tested by md_reorg.
We then set flag_delayed_branch itself to 0.

This is incompatible with the optimize attribute, which generally (and
quite reasonably) wants flag variables to reflect their real settings.
Although we _could_ add new target hooks to allow mips_base_delayed_branch
and flag_delayed_branch to be handled in a similar way to they are now,
I think that's the wrong way to go.

The fundamental problem is that md_reorg sometimes needs to have the
last say on the instruction stream, whereas we currently run dbr_schedule
afterwards.  (And shorten_branches too, although that's usually less
of an issue.)

One fix would be to add a second md_reorg pass after dbr_schedule,
but I remember the idea of a second md_reorg being rejected a few
years ago.  There was recently a suggestion on gcc@ that the target
could reorder passes -- and perhaps insert passes of its own -- but if
adding a second md_reorg is controversial, I suppose that will be too.
It also seems way too invasive for this stage of development.

Instead, I'd like to add a crtl boolean to say whether dbr_schedule
has already been run.  If so, it shouldn't run again.  (The pass cannot
run twice anyway; the setup routines would ICE the second time.)

I know this is inelegant, and might seem to set a dangerous precedent.
But the number of passes run after md_reorg is very small -- and should
definitely remain that way -- so I don't think we'd ever have lots of
fields of this kind.  It also seems less hackish than the status quo.

Tested on mips64el-linux-gnu, where it fixes gcc.dg/pr37106-1.c.
OK to install?

Richard


gcc/
	* function.h (rtl_data): Add a dbr_scheduled_p field.
	* reorg.c (dbr_schedule): Set it.
	(gate_handle_delay_slots): Check it.
	* config/mips/mips.c (mips_base_delayed_branch): Delete.
	(mips_reorg): Check flag_delayed_branch instead of
	mips_base_delayed_branch.
	(mips_override_options): Don't set mips_base_delayed_branch
	or flag_delayed_branch.

Index: gcc/function.h
===================================================================
--- gcc/function.h	2009-01-02 11:37:20.000000000 +0000
+++ gcc/function.h	2009-01-03 16:07:45.000000000 +0000
@@ -444,6 +444,9 @@ struct rtl_data GTY(())
   /* Nonzero if function stack realignment has been finalized, namely
      stack_realign_needed flag has been set and finalized after reload.  */
   bool stack_realign_finalized;
+
+  /* True if the dbr_schedule has already been called for this function.  */
+  bool dbr_scheduled_p;
 };
 
 #define return_label (crtl->x_return_label)
Index: gcc/reorg.c
===================================================================
--- gcc/reorg.c	2009-01-02 11:37:20.000000000 +0000
+++ gcc/reorg.c	2009-01-03 16:05:29.000000000 +0000
@@ -4038,6 +4038,7 @@ dbr_schedule (rtx first)
   }
 
 #endif
+  crtl->dbr_scheduled_p = true;
 }
 #endif /* DELAY_SLOTS */
 
@@ -4045,7 +4046,7 @@ dbr_schedule (rtx first)
 gate_handle_delay_slots (void)
 {
 #ifdef DELAY_SLOTS
-  return flag_delayed_branch;
+  return flag_delayed_branch && !crtl->dbr_scheduled_p;
 #else
   return 0;
 #endif
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2009-01-03 15:59:12.000000000 +0000
+++ gcc/config/mips/mips.c	2009-01-03 16:06:07.000000000 +0000
@@ -453,7 +453,6 @@ int mips_abi = MIPS_ABI_DEFAULT;
 bool mips_base_mips16;
 
 /* The ambient values of other global variables.  */
-static int mips_base_delayed_branch; /* flag_delayed_branch */
 static int mips_base_schedule_insns; /* flag_schedule_insns */
 static int mips_base_reorder_blocks_and_partition; /* flag_reorder... */
 static int mips_base_move_loop_invariants; /* flag_move_loop_invariants */
@@ -13297,7 +13296,7 @@ mips_reorg (void)
   mips16_lay_out_constants ();
   if (mips_r10k_cache_barrier != R10K_CACHE_BARRIER_NONE)
     r10k_insert_cache_barriers ();
-  if (mips_base_delayed_branch)
+  if (flag_delayed_branch)
     dbr_schedule (get_insns ());
   mips_reorg_process_insns ();
   if (!TARGET_MIPS16
@@ -14058,7 +14057,6 @@ mips_override_options (void)
 
   /* Save base state of options.  */
   mips_base_target_flags = target_flags;
-  mips_base_delayed_branch = flag_delayed_branch;
   mips_base_schedule_insns = flag_schedule_insns;
   mips_base_reorder_blocks_and_partition = flag_reorder_blocks_and_partition;
   mips_base_move_loop_invariants = flag_move_loop_invariants;
@@ -14071,9 +14069,6 @@ mips_override_options (void)
      Do all CPP-sensitive stuff in non-MIPS16 mode; we'll switch to
      MIPS16 mode afterwards if need be.  */
   mips_set_mips16_mode (false);
-
-  /* We call dbr_schedule from within mips_reorg.  */
-  flag_delayed_branch = 0;
 }
 
 /* Swap the register information for registers I and I + 1, which



More information about the Gcc-patches mailing list