[committed] Disable -fmove-loop-invariants for MIPS16

Richard Sandiford richard@codesourcery.com
Fri Sep 7 09:15:00 GMT 2007


I'd noticed a while ago that the CSiBE results for MIPS16 were
significantly better with -fno-move-loop-invariants.  I think the
problems with -fmove-loop-invariants are threefold:

  - It increases register pressure.

  - If the constant it moves is only used once, and is the first
    operand to a two-operand binary MIPS16 instruction, we end
    up replacing a constant load with a constant load and a move.

  - Likewise if the constant forms a register parameter to a call.

These problems (especially the first) are likely to affect speed as
well size, and even if they don't, we should generally be more size-
conscious for MIPS16 than for non-MIPS16, even for non-Os options.

I've attached CSiBE results for four combinations:

  csibe-G8-yes.txt:  -G8 -mcode-readable=yes
  csibe-G0-yes.txt:  -G0 -mcode-readable=yes
  csibe-G8-no.txt:   -G8 -mcode-readable=no
  csibe-G0-no.txt:   -G0 -mcode-readable=no

All tests were run using mipsisa32r2-sde-elf and additionally used
the flags "-Os -mips32r2 -mips16 -fno-common".  To summarise:

-G8 -mcode-readable=yes                        2824521  2816869 :   99.73%
-G0 -mcode-readable=yes                        2836865  2826553 :   99.64%
-G8 -mcode-readable=no                         3016003  3002691 :   99.56%
-G0 -mcode-readable=no                         3214073  3196805 :   99.46%

So I think we should turn flag_move_loop_invariants off by default
for MIPS16.  Tested on mipsisa32r2-sde-elf, applied to mainline.

Richard


gcc/
	* config/mips/mips.c (mips_base_move_loop_invariants): New variable.
	(mips_set_mips16_mode): Restore flag_move_loop_invariants, then set
	to 0 for MIPS16.
	(override_options): Set mips_base_move_loop_invariants.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2007-09-07 00:50:52.000000000 -0700
+++ gcc/config/mips/mips.c	2007-09-07 00:57:14.000000000 -0700
@@ -625,6 +625,7 @@ int mips_abi = MIPS_ABI_DEFAULT;
 /* Similar copies of option settings.  */
 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 */
 static int mips_base_align_loops; /* align_loops */
 static int mips_base_align_jumps; /* align_jumps */
 static int mips_base_align_functions; /* align_functions */
@@ -5596,6 +5597,7 @@ mips_set_mips16_mode (int mips16_p)
   align_functions = mips_base_align_functions;
   flag_schedule_insns = mips_base_schedule_insns;
   flag_reorder_blocks_and_partition = mips_base_reorder_blocks_and_partition;
+  flag_move_loop_invariants = mips_base_move_loop_invariants;
   flag_delayed_branch = mips_flag_delayed_branch;
   
   if (mips16_p) 
@@ -5608,9 +5610,16 @@ mips_set_mips16_mode (int mips16_p)
       flag_schedule_insns = 0;
 
       /* Don't do hot/cold partitioning.  The constant layout code expects
-       the whole function to be in a single section.  */
+	 the whole function to be in a single section.  */
       flag_reorder_blocks_and_partition = 0;
 
+      /* Don't move loop invariants, because it tends to increase
+	 register pressure.  It also introduces an extra move in cases
+	 where the constant is the first operand in a two-operand binary
+	 instruction, or when it forms a register argument to a functon
+	 call.  */
+      flag_move_loop_invariants = 0;
+
       /* Silently disable -mexplicit-relocs since it doesn't apply
 	 to mips16 code.  Even so, it would overly pedantic to warn
 	 about "-mips16 -mexplicit-relocs", especially given that
@@ -6122,6 +6131,7 @@ override_options (void)
   mips_base_target_flags = target_flags;
   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;
   mips_base_align_loops = align_loops;
   mips_base_align_jumps = align_jumps;
   mips_base_align_functions = align_functions;



More information about the Gcc-patches mailing list