RFA: enable LRA for rs6000 [patch for WRF]

Vladimir Makarov vmakarov@redhat.com
Fri Apr 19 08:09:00 GMT 2013


On 04/17/2013 12:10 PM, Michael Meissner wrote:
> On Wed, Apr 17, 2013 at 10:14:53AM -0400, Vladimir Makarov wrote:
>> Mike, thanks for the patch and all the SPEC2006 data  (which are
>> very useful as I have no access to power machine which can be used
>> for benchmarking).  I guess that may be some benchmark scores are
>> lower because of LRA lacks some micro-optimizations which reload
>> implements through many power hooks (e.g. LRA does not use push
>> reload).  Although sometimes it is not a bad thing (e.g. LRA does
>> not use  SECONDARY_MEMORY_NEEDED_RTX which permits to reuse the
>> stack slots for other useful things).
> SECONDARY_MEMORY_NEEDED_RTX is needed for SDmode on machines before the power7,
> where we need a larger stack slot to hold spilled SDmode values (power6 did not
> have the LFIWZX instruction that is needed to load SDmode values into floating
> point registers).
Thanks for the info.
>> In general I got impression that power7 is the most difficult port
>> for LRA.  If we manage to port it, LRA ports for other targets will
>> be easier.
> I dunno, has LRA been ported to the SH yet?
Not yet.

Sorry for be inaccurate.  I meant 9 targets which I worked on to port LRA.
>> I also reproduced bootstrap failure --with-cpu=power7 and I am going
>> to work on this and after that on SPEC2006 you wrote about.
>
The bootstrap problem was in processing move whose operand was 
substituted by equiv. memory and the move needs secondary reload through 
a provided insn pattern.  The equiv memory was not legitimate and it 
resulted in failure to generated the secondary reload insn.

LRA can fix the wrong address but secondary reload  was done before 
processing addresses.  It could be fixed in rs6000.c code too but it is 
complicated and I found a better (and i think more right) solution by 
moving secondary reload generation after address processing.

Here is the patch for your branch (patch for trunk is a bit different as 
some changes in affected code were done on trunk).

2013-04-18  Vladimir Makarov  <vmakarov@redhat.com>

         * lra-constraints.c (check_and_process_move): Move code for move
         cost check to simple_move_p.  Remove equiv_substitution.
         (simple_move_p): New function.
         (curr_insn_transform): Use the new function.  Move call of
         check_and_process_move after operand equiv substitution and
         address process.

Tomorrow I am going to look at SPEC2006 dealII crash for 32-bit mode.

-------------- next part --------------
Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 198028)
+++ lra-constraints.c	(working copy)
@@ -887,14 +887,6 @@ check_and_process_move (bool *change_p,
   lra_assert (curr_insn_set != NULL_RTX);
   dreg = dest = SET_DEST (curr_insn_set);
   sreg = src = SET_SRC (curr_insn_set);
-  /* Quick check on the right move insn which does not need
-     reloads.  */
-  if ((dclass = get_op_class (dest)) != NO_REGS
-      && (sclass = get_op_class (src)) != NO_REGS
-      /* The backend guarantees that register moves of cost 2 never
-	 need reloads.  */
-      && targetm.register_move_cost (GET_MODE (src), dclass, sclass) == 2)
-    return true;
   if (GET_CODE (dest) == SUBREG)
     dreg = SUBREG_REG (dest);
   if (GET_CODE (src) == SUBREG)
@@ -902,7 +894,6 @@ check_and_process_move (bool *change_p,
   if (! REG_P (dreg) || ! REG_P (sreg))
     return false;
   sclass = dclass = NO_REGS;
-  dreg = get_equiv_substitution (dreg);
   if (REG_P (dreg))
     dclass = get_reg_class (REGNO (dreg));
   if (dclass == ALL_REGS)
@@ -916,7 +907,6 @@ check_and_process_move (bool *change_p,
     return false;
   sreg_mode = GET_MODE (sreg);
   old_sreg = sreg;
-  sreg = get_equiv_substitution (sreg);
   if (REG_P (sreg))
     sclass = get_reg_class (REGNO (sreg));
   if (sclass == ALL_REGS)
@@ -2693,6 +2683,24 @@ emit_inc (enum reg_class new_rclass, rtx
   return result;
 }
 
+/* Return true if the current move insn does not need processing as we
+   already know that it satisfies its constraints.  */
+static bool
+simple_move_p (void)
+{
+  rtx dest, src;
+  enum reg_class dclass, sclass;
+
+  lra_assert (curr_insn_set != NULL_RTX);
+  dest = SET_DEST (curr_insn_set);
+  src = SET_SRC (curr_insn_set);
+  return ((dclass = get_op_class (dest)) != NO_REGS
+	  && (sclass = get_op_class (src)) != NO_REGS
+	  /* The backend guarantees that register moves of cost 2
+	     never need reloads.  */
+	  && targetm.register_move_cost (GET_MODE (src), dclass, sclass) == 2);
+ }
+
 /* Swap operands NOP and NOP + 1. */
 static inline void
 swap_operands (int nop)
@@ -2736,15 +2744,13 @@ curr_insn_transform (void)
   int max_regno_before;
   int reused_alternative_num;
 
+  curr_insn_set = single_set (curr_insn);
+  if (curr_insn_set != NULL_RTX && simple_move_p ())
+    return false;
+
   no_input_reloads_p = no_output_reloads_p = false;
   goal_alt_number = -1;
-
   change_p = sec_mem_p = false;
-  curr_insn_set = single_set (curr_insn);
-  if (curr_insn_set != NULL_RTX
-      && check_and_process_move (&change_p, &sec_mem_p))
-    return change_p;
-
   /* JUMP_INSNs and CALL_INSNs are not allowed to have any output
      reloads; neither are insns that SET cc0.  Insns that use CC0 are
      not allowed to have any input reloads.  */
@@ -2839,6 +2845,10 @@ curr_insn_transform (void)
        we chose previously may no longer be valid.  */
     lra_set_used_insn_alternative (curr_insn, -1);
 
+  if (curr_insn_set != NULL_RTX
+      && check_and_process_move (&change_p, &sec_mem_p))
+    return change_p;
+
  try_swapped:
 
   reused_alternative_num = curr_id->used_insn_alternative;


More information about the Gcc-patches mailing list