Re: 4.2 reload infrastructure (PR 25027)

[Bernd Schmidt <bernds_cb1 at t-online dot de>]

John David Anglin wrote:
> Joern, could you help with fixing PR 25027?  At first look, it seems
> your change to the reload infrastructure has introduced several problems
> on the PA.

Ok, so it seems that the backwards compatibility code isn't 100% 
backwards compatible.  The new code expects that the scratch registers 
returned by SECONDARY_RELOAD_CLASS match the constraints on the 
reload_inxx patterns, while the old code was a little more lenient. 
This is somewhat unfortunate, but not disastrous since the mismatch is 
detected at runtime.  We should just convert all ports to use the new 
target hook.

It seems relatively straightforward to adapt the PA to the new 
infrastructure; I've attached a patch which could serve as a starting 
point (hardly tested, but it seems to fix the testcase).  I'm pretty 
sure that if you start using the ability to choose a specific 
reload_xxxx pattern, you could simplify a bit of code, e.g. in 
emit_move_sequence (that is one ugly function you have there).

Given how little I know about the PA, and how convoluted the code 
especially in emit_move_sequence is, you better double-check that this 
does what you need.


Bernd
Index: pa.md
===================================================================
--- pa.md	(revision 107470)
+++ pa.md	(working copy)
@@ -2258,6 +2258,21 @@
 ;; Reloading an SImode or DImode value requires a scratch register if
 ;; going in to or out of float point registers.
 
+(define_expand "reload_insi_pic_const"
+  [(set (match_operand:SI 0 "register_operand" "=Z")
+	(match_operand:SI 1 "non_hard_reg_operand" ""))
+   (clobber (match_operand:SI 2 "register_operand" "=&a"))]
+  ""
+  "
+{
+  if (emit_move_sequence (operands, SImode, operands[2]))
+    DONE;
+
+  /* We don't want the clobber emitted, so handle this ourselves.  */
+  emit_insn (gen_rtx_SET (VOIDmode, operands[0], operands[1]));
+  DONE;
+}")
+
 (define_expand "reload_insi"
   [(set (match_operand:SI 0 "register_operand" "=Z")
 	(match_operand:SI 1 "non_hard_reg_operand" ""))
@@ -4044,6 +4059,21 @@
     DONE;
 }")
 
+(define_expand "reload_indi_pic_const"
+  [(set (match_operand:DI 0 "register_operand" "=Z")
+	(match_operand:DI 1 "non_hard_reg_operand" ""))
+   (clobber (match_operand:SI 2 "register_operand" "=&a"))]
+  ""
+  "
+{
+  if (emit_move_sequence (operands, DImode, operands[2]))
+    DONE;
+
+  /* We don't want the clobber emitted, so handle this ourselves.  */
+  emit_insn (gen_rtx_SET (VOIDmode, operands[0], operands[1]));
+  DONE;
+}")
+
 (define_expand "reload_indi"
   [(set (match_operand:DI 0 "register_operand" "=Z")
 	(match_operand:DI 1 "non_hard_reg_operand" ""))
Index: pa.c
===================================================================
--- pa.c	(revision 107470)
+++ pa.c	(working copy)
@@ -150,6 +150,9 @@
 static int pa_arg_partial_bytes (CUMULATIVE_ARGS *, enum machine_mode,
 				 tree, bool);
 static struct machine_function * pa_init_machine_status (void);
+static enum reg_class pa_secondary_reload (bool, rtx, enum reg_class,
+					   enum machine_mode,
+					   secondary_reload_info *);
 
 
 /* Save the operands last given to a compare for use when we
@@ -299,6 +302,9 @@
 #undef TARGET_CANNOT_FORCE_CONST_MEM
 #define TARGET_CANNOT_FORCE_CONST_MEM pa_tls_referenced_p
 
+#undef TARGET_SECONDARY_RELOAD
+#define TARGET_SECONDARY_RELOAD pa_secondary_reload
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 /* Parse the -mfixed-range= option string.  */
@@ -5565,18 +5571,9 @@
   fputc ('\n', asm_out_file);
 }
 
-/* Return the class of any secondary reload register that is needed to
-   move IN into a register in class CLASS using mode MODE.
-
-   Profiling has showed this routine and its descendants account for
-   a significant amount of compile time (~7%).  So it has been
-   optimized to reduce redundant computations and eliminate useless
-   function calls.
-
-   It might be worthwhile to try and make this a leaf function too.  */
-
-enum reg_class
-pa_secondary_reload_class (enum reg_class class, enum machine_mode mode, rtx in)
+static enum reg_class
+pa_secondary_reload (bool in_p, rtx x, enum reg_class class,
+		     enum machine_mode mode, secondary_reload_info *sri)
 {
   int regno, is_symbolic;
 
@@ -5585,28 +5582,33 @@
   if (flag_pic
       && GET_MODE_CLASS (mode) == MODE_INT
       && FP_REG_CLASS_P (class)
-      && (GET_CODE (in) == CONST_INT || GET_CODE (in) == CONST_DOUBLE))
-    return R1_REGS;
+      && (GET_CODE (x) == CONST_INT || GET_CODE (x) == CONST_DOUBLE))
+    {
+      gcc_assert (mode == SImode || mode == DImode);
+      sri->icode = (mode == SImode ? CODE_FOR_reload_insi_pic_const
+		    : CODE_FOR_reload_indi_pic_const);
+      return NO_REGS;
+    }
 
   /* Profiling showed the PA port spends about 1.3% of its compilation
      time in true_regnum from calls inside pa_secondary_reload_class.  */
 
-  if (GET_CODE (in) == REG)
+  if (GET_CODE (x) == REG)
     {
-      regno = REGNO (in);
+      regno = REGNO (x);
       if (regno >= FIRST_PSEUDO_REGISTER)
-	regno = true_regnum (in);
+	regno = true_regnum (x);
     }
-  else if (GET_CODE (in) == SUBREG)
-    regno = true_regnum (in);
+  else if (GET_CODE (x) == SUBREG)
+    regno = true_regnum (x);
   else
     regno = -1;
 
   /* If we have something like (mem (mem (...)), we can safely assume the
      inner MEM will end up in a general register after reloading, so there's
      no need for a secondary reload.  */
-  if (GET_CODE (in) == MEM
-      && GET_CODE (XEXP (in, 0)) == MEM)
+  if (GET_CODE (x) == MEM
+      && GET_CODE (XEXP (x, 0)) == MEM)
     return NO_REGS;
 
   /* Handle out of range displacement for integer mode loads/stores of
@@ -5615,24 +5617,30 @@
        && GET_MODE_CLASS (mode) == MODE_INT
        && FP_REG_CLASS_P (class))
       || (class == SHIFT_REGS && (regno <= 0 || regno >= 32)))
-    return GENERAL_REGS;
+    {
+      sri->icode = in_p ? reload_in_optab[mode] : reload_out_optab[mode];
+      return NO_REGS;
+    }
 
   /* A SAR<->FP register copy requires a secondary register (GPR) as
      well as secondary memory.  */
   if (regno >= 0 && regno < FIRST_PSEUDO_REGISTER
       && ((REGNO_REG_CLASS (regno) == SHIFT_REGS && FP_REG_CLASS_P (class))
 	  || (class == SHIFT_REGS && FP_REG_CLASS_P (REGNO_REG_CLASS (regno)))))
-    return GENERAL_REGS;
+    {
+      sri->icode = in_p ? reload_in_optab[mode] : reload_out_optab[mode];
+      return NO_REGS;
+    }
 
-  if (GET_CODE (in) == HIGH)
-    in = XEXP (in, 0);
+  if (GET_CODE (x) == HIGH)
+    x = XEXP (x, 0);
 
   /* Profiling has showed GCC spends about 2.6% of its compilation
      time in symbolic_operand from calls inside pa_secondary_reload_class.
 
      We use an inline copy and only compute its return value once to avoid
      useless work.  */
-  switch (GET_CODE (in))
+  switch (GET_CODE (x))
     {
       rtx tmp;
 
@@ -5641,7 +5649,7 @@
         is_symbolic = 1;
         break;
       case CONST:
-	tmp = XEXP (in, 0);
+	tmp = XEXP (x, 0);
 	is_symbolic = ((GET_CODE (XEXP (tmp, 0)) == SYMBOL_REF
 			|| GET_CODE (XEXP (tmp, 0)) == LABEL_REF)
 		       && GET_CODE (XEXP (tmp, 1)) == CONST_INT);
@@ -5654,11 +5662,16 @@
 
   if (!flag_pic
       && is_symbolic
-      && read_only_operand (in, VOIDmode))
+      && read_only_operand (x, VOIDmode))
     return NO_REGS;
 
   if (class != R1_REGS && is_symbolic)
-    return R1_REGS;
+    {
+      gcc_assert (mode == SImode || mode == DImode);
+      sri->icode = (mode == SImode ? CODE_FOR_reload_insi_pic_const
+		    : CODE_FOR_reload_indi_pic_const);
+      return NO_REGS;
+    }
 
   return NO_REGS;
 }
Index: pa.h
===================================================================
--- pa.h	(revision 107470)
+++ pa.h	(working copy)
@@ -527,16 +527,6 @@
    in some cases it is preferable to use a more restrictive class.  */
 #define PREFERRED_RELOAD_CLASS(X,CLASS) (CLASS)
 
-/* Return the register class of a scratch register needed to copy
-   IN into a register in CLASS in MODE, or a register in CLASS in MODE
-   to IN.  If it can be done directly NO_REGS is returned. 
-
-  Avoid doing any work for the common case calls.  */
-#define SECONDARY_RELOAD_CLASS(CLASS,MODE,IN) \
-  ((CLASS == BASE_REG_CLASS && GET_CODE (IN) == REG		\
-    && REGNO (IN) < FIRST_PSEUDO_REGISTER)			\
-   ? NO_REGS : pa_secondary_reload_class (CLASS, MODE, IN))
-
 #define MAYBE_FP_REG_CLASS_P(CLASS) \
   reg_classes_intersect_p ((CLASS), FP_REGS)