diff mbox

0006-GCSE-complex-constants.patch

Message ID 4C18F4BD.5000906@codesourcery.com
State New
Headers show

Commit Message

Maxim Kuvyrkov June 16, 2010, 3:58 p.m. UTC 
Certain architectures (e.g., ARM) cannot easily operate with constants, 
they need to emit sequences of several instructions to load constants 
into registers.  The common procedure to do this is to emit a (parallel 
[(set) (clobber (reg1)) ... (clobber (regN))]) instruction which later 
splits into several instructions using pseudos (regX) to store 
intermediate values.

Currently PRE and hoist do not GCSE constants, and there is a good 
reason for that, to avoid increasing register pressure; interestingly, 
symbol_refs are allowed to be GCSE'ed, is this intentional or by accident?

In any case, it seems like a good idea to GCSE constants and symbol_refs 
that need something beyond a simple (set) to get into a register, and 
not GCSE them otherwise.

This patch adds a simple heuristic to gcse.c:want_to_gcse_p() that 
adjusts preferences for CONST_INTs and SYMBOL_REFs.

OK to apply?

Thank you,

Comments

Jeff Law June 16, 2010, 4:54 p.m. UTC | #1 
> Certain architectures (e.g., ARM) cannot easily operate with 
> constants, they need to emit sequences of several instructions to load 
> constants into registers.  The common procedure to do this is to emit 
> a (parallel [(set) (clobber (reg1)) ... (clobber (regN))]) instruction 
> which later splits into several instructions using pseudos (regX) to 
> store intermediate values.
>
> Currently PRE and hoist do not GCSE constants, and there is a good 
> reason for that, to avoid increasing register pressure; interestingly, 
> symbol_refs are allowed to be GCSE'ed, is this intentional or by 
> accident?
It's intentional; a SYMBOL_REF if often be rather expensive.  Some 
CONST_INTs can have that same property.  One could argue that an 
expensive CONST_INT shouldn't be appearing in RTL, but certainly some 
ports have chosen to handle splitting insns with expensive constants 
later in the pipeline.

>
> In any case, it seems like a good idea to GCSE constants and 
> symbol_refs that need something beyond a simple (set) to get into a 
> register, and not GCSE them otherwise.
Rather than triggering this on the PARALLEL it might be better to 
trigger it on the cost of the RTX.  Triggering on the PARALLEL looks 
like a hack to me -- IMHO we'd be better off fixing the costing 
mechanism and using costing as the trigger.

Jeff
diff mbox

Patch

diff --git a/gcc/gcse.c b/gcc/gcse.c
index a7c7237..c81d71c 100644
--- a/gcc/gcse.c
+++ b/gcc/gcse.c
@@ -431,7 +431,7 @@  static void hash_scan_insn (rtx, struct hash_table_d *);
 static void hash_scan_set (rtx, rtx, struct hash_table_d *);
 static void hash_scan_clobber (rtx, rtx, struct hash_table_d *);
 static void hash_scan_call (rtx, rtx, struct hash_table_d *);
-static int want_to_gcse_p (rtx);
+static int want_to_gcse_p (rtx, rtx);
 static bool gcse_constant_p (const_rtx);
 static int oprs_unchanged_p (const_rtx, const_rtx, int);
 static int oprs_anticipatable_p (const_rtx, const_rtx);
@@ -751,10 +751,10 @@  static basic_block current_bb;
 
 
 /* See whether X, the source of a set, is something we want to consider for
-   GCSE.  */
+   GCSE in instruction INSN.  */
 
 static int
-want_to_gcse_p (rtx x)
+want_to_gcse_p (rtx x, rtx insn)
 {
 #ifdef STACK_REGS
   /* On register stack architectures, don't GCSE constants from the
@@ -768,13 +768,35 @@  want_to_gcse_p (rtx x)
     {
     case REG:
     case SUBREG:
-    case CONST_INT:
     case CONST_DOUBLE:
     case CONST_FIXED:
     case CONST_VECTOR:
     case CALL:
       return 0;
 
+    case CONST_INT:
+    case SYMBOL_REF:
+      /* If it takes a PARALLEL to set a constant or a symbol, try to gcse it.
+	 Usually, (clobber (reg)) is the second part of the parallel.
+	 We rely on rematerialization of constants to avoid excessive
+	 register pressure.
+
+	 ??? We would also like to GCSE/hoist non-parallel-looking constants
+	 and symbol_refs on architectures which require load from constant
+	 pools to get a constant into register, e.g., ARM.
+	 We do not currently do that because IRA overestimates cost
+	 of allocating a constant to memory, thus unnecessarily increasing
+	 register pressure and causing spills.  One side of this problem
+	 is IRA using rtx_costs which are not particularly precise for
+	 constants when optimizing for size.  For a good example see
+	 300.twolf:ucxxo1.c from SPEC2000.
+
+	 ??? Should we handle CONST_DOUBLE and CONST_FIXED similarly?
+	 Will rematerialization handle them?  */
+      if (GET_CODE (PATTERN (insn)) != PARALLEL)
+	return 0;
+      /* FALLTHRU */
+
     default:
       return can_assign_to_reg_without_clobbers_p (x);
     }
@@ -1328,7 +1350,7 @@  hash_scan_set (rtx pat, rtx insn, struct hash_table_d *table)
 	  && !REG_P (src)
 	  && (table->set_p
 	      ? gcse_constant_p (XEXP (note, 0))
-	      : want_to_gcse_p (XEXP (note, 0))))
+	      : want_to_gcse_p (XEXP (note, 0), insn)))
 	src = XEXP (note, 0), pat = gen_rtx_SET (VOIDmode, dest, src);
 
       /* Only record sets of pseudo-regs in the hash table.  */
@@ -1343,7 +1365,7 @@  hash_scan_set (rtx pat, rtx insn, struct hash_table_d *table)
 	     can't do the same thing at the rtl level.  */
 	  && !can_throw_internal (insn)
 	  /* Is SET_SRC something we want to gcse?  */
-	  && want_to_gcse_p (src)
+	  && want_to_gcse_p (src, insn)
 	  /* Don't CSE a nop.  */
 	  && ! set_noop_p (pat)
 	  /* Don't GCSE if it has attached REG_EQUIV note.
@@ -1404,7 +1426,7 @@  hash_scan_set (rtx pat, rtx insn, struct hash_table_d *table)
 	      do that easily for EH edges so disable GCSE on these for now.  */
 	   && !can_throw_internal (insn)
 	   /* Is SET_DEST something we want to gcse?  */
-	   && want_to_gcse_p (dest)
+	   && want_to_gcse_p (dest, insn)
 	   /* Don't CSE a nop.  */
 	   && ! set_noop_p (pat)
 	   /* Don't GCSE if it has attached REG_EQUIV note.