Patchwork RFC: LRA for x86/x86-64 [6/9]

login
register
mail settings
Submitter Vladimir Makarov
Date Sept. 27, 2012, 10:58 p.m.
Message ID <5064DA18.20404@redhat.com>
Download mbox | patch
Permalink /patch/187523/
State New
Headers show

Comments

Vladimir Makarov - Sept. 27, 2012, 10:58 p.m.
The following patch modifies some code in the rest of compiler for
correct work of LRA.  The code works the same way when LRA is not
used.  It is achieved by checking a new variable lra_in_progress.

2012-09-27  Vladimir Makarov  <vmakarov@redhat.com>

     * rtlanal.c (simplify_subreg_regno): Permit ARG_POINTER_REGNUM and
     STACK_POINTER_REGNU for LRA.
     * jump.c (true_regnum): Always use hard_regno for subreg_get_info when
     lra is in progress.
     * expr.c (emit_move_insn_1): Pass an additional argument to
     emit_move_via_integer.  Use emit_move_via_integer for LRA only if
     the insn is recognized.
     * recog.c (general_operand, register_operand): Accept paradoxical 
FLOAD_MODE
     subregs for LRA.
     (scratch_operand): Accept pseudos for LRA.
     * emit-rtl.c (gen_rtx_REG): Add lra_in_progress.
     (validate_subreg): Don't check offset for LRA and
     floating point modes.
     * rtl.h (lra_in_progress): New external.
     * ira.c (lra_in_progress): Define.
Jeff Law - Sept. 28, 2012, 8:07 p.m.
On 09/27/2012 04:58 PM, Vladimir Makarov wrote:
>    The following patch modifies some code in the rest of compiler for
> correct work of LRA.  The code works the same way when LRA is not
> used.  It is achieved by checking a new variable lra_in_progress.
>
> 2012-09-27  Vladimir Makarov <vmakarov@redhat.com>
>
>      * rtlanal.c (simplify_subreg_regno): Permit ARG_POINTER_REGNUM and
>      STACK_POINTER_REGNU for LRA.
>      * jump.c (true_regnum): Always use hard_regno for subreg_get_info when
>      lra is in progress.
>      * expr.c (emit_move_insn_1): Pass an additional argument to
>      emit_move_via_integer.  Use emit_move_via_integer for LRA only if
>      the insn is recognized.
>      * recog.c (general_operand, register_operand): Accept paradoxical
> FLOAD_MODE
>      subregs for LRA.
>      (scratch_operand): Accept pseudos for LRA.
>      * emit-rtl.c (gen_rtx_REG): Add lra_in_progress.
>      (validate_subreg): Don't check offset for LRA and
>      floating point modes.
>      * rtl.h (lra_in_progress): New external.
>      * ira.c (lra_in_progress): Define.
s/FLOAD/FLOAT/ to fix ChangeLog typo.



> Index: jump.c
> ===================================================================
> --- jump.c	(revision 191771)
> +++ jump.c	(working copy)
> @@ -1868,7 +1868,8 @@ true_regnum (const_rtx x)
>   {
>     if (REG_P (x))
>       {
> -      if (REGNO (x) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO (x)] >= 0)
> +      if (REGNO (x) >= FIRST_PSEUDO_REGISTER
> +	  && (lra_in_progress || reg_renumber[REGNO (x)] >= 0))
>   	return reg_renumber[REGNO (x)];
>         return REGNO (x);
>       }
This hunk doesn't make any sense to me, unless you want true_regnum to 
return a negative value during LRA for cases where the pseudo is still 
unassigned.  Is that what's you're intending here?  If that's what you 
want, then I think it's worth a quick comment.



> @@ -1880,7 +1881,8 @@ true_regnum (const_rtx x)
>   	{
>   	  struct subreg_info info;
>
> -	  subreg_get_info (REGNO (SUBREG_REG (x)),
> +	  subreg_get_info (lra_in_progress
> +			   ? (unsigned) base : REGNO (SUBREG_REG (x)),
>   			   GET_MODE (SUBREG_REG (x)),
>   			   SUBREG_BYTE (x), GET_MODE (x), &info);
I'd be good to indicate why you want to do something different for LRA 
here.

>
> Index: rtlanal.c
> ===================================================================
> --- rtlanal.c	(revision 191771)
> +++ rtlanal.c	(working copy)
> @@ -3465,7 +3465,9 @@ simplify_subreg_regno (unsigned int xreg
>     /* Give the backend a chance to disallow the mode change.  */
>     if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT
>         && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT
> -      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode))
> +      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)
> +      /* We can use mode change in LRA for some transformations.  */
> +      && ! lra_in_progress)
>       return -1;
>   #endif
I don't think this change is reflected in the ChangeLog.

I think just the minor ChangeLog updates and clarification of the 
changes to jump.c are all that's needed for this patch to be approved.

jeff
Vladimir Makarov - Sept. 28, 2012, 11:13 p.m.
On 12-09-28 4:07 PM, Jeff Law wrote:
> On 09/27/2012 04:58 PM, Vladimir Makarov wrote:
>>    The following patch modifies some code in the rest of compiler for
>> correct work of LRA.  The code works the same way when LRA is not
>> used.  It is achieved by checking a new variable lra_in_progress.
>>
>> 2012-09-27  Vladimir Makarov <vmakarov@redhat.com>
>>
>>      * rtlanal.c (simplify_subreg_regno): Permit ARG_POINTER_REGNUM and
>>      STACK_POINTER_REGNU for LRA.
>>      * jump.c (true_regnum): Always use hard_regno for 
>> subreg_get_info when
>>      lra is in progress.
>>      * expr.c (emit_move_insn_1): Pass an additional argument to
>>      emit_move_via_integer.  Use emit_move_via_integer for LRA only if
>>      the insn is recognized.
>>      * recog.c (general_operand, register_operand): Accept paradoxical
>> FLOAD_MODE
>>      subregs for LRA.
>>      (scratch_operand): Accept pseudos for LRA.
>>      * emit-rtl.c (gen_rtx_REG): Add lra_in_progress.
>>      (validate_subreg): Don't check offset for LRA and
>>      floating point modes.
>>      * rtl.h (lra_in_progress): New external.
>>      * ira.c (lra_in_progress): Define.
> s/FLOAD/FLOAT/ to fix ChangeLog typo.
>
>
Thanks.  I fixed it will be in the revised versions of the patches.
>
>> Index: jump.c
>> ===================================================================
>> --- jump.c    (revision 191771)
>> +++ jump.c    (working copy)
>> @@ -1868,7 +1868,8 @@ true_regnum (const_rtx x)
>>   {
>>     if (REG_P (x))
>>       {
>> -      if (REGNO (x) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO 
>> (x)] >= 0)
>> +      if (REGNO (x) >= FIRST_PSEUDO_REGISTER
>> +      && (lra_in_progress || reg_renumber[REGNO (x)] >= 0))
>>       return reg_renumber[REGNO (x)];
>>         return REGNO (x);
>>       }
> This hunk doesn't make any sense to me, unless you want true_regnum to 
> return a negative value during LRA for cases where the pseudo is still 
> unassigned.  Is that what's you're intending here?  If that's what you 
> want, then I think it's worth a quick comment.
>
Yes, that was my intention.  LRA works a bit different from reload. It 
needs the current assignment of pseudos (assigned hard register or -1 
even it is not assigned).

I'll add the comment about this in the next version of the patch. The 
code looks a bit strange and could be more clear but I kept in my mind 
removing code for reload in the future.
>
>
>> @@ -1880,7 +1881,8 @@ true_regnum (const_rtx x)
>>       {
>>         struct subreg_info info;
>>
>> -      subreg_get_info (REGNO (SUBREG_REG (x)),
>> +      subreg_get_info (lra_in_progress
>> +               ? (unsigned) base : REGNO (SUBREG_REG (x)),
>>                  GET_MODE (SUBREG_REG (x)),
>>                  SUBREG_BYTE (x), GET_MODE (x), &info);
> I'd be good to indicate why you want to do something different for LRA 
> here.
>
I need to know the current allocation with taking into account that the 
subregister with final hard register will be representable. I'll add the 
comment.
>>
>> Index: rtlanal.c
>> ===================================================================
>> --- rtlanal.c    (revision 191771)
>> +++ rtlanal.c    (working copy)
>> @@ -3465,7 +3465,9 @@ simplify_subreg_regno (unsigned int xreg
>>     /* Give the backend a chance to disallow the mode change. */
>>     if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT
>>         && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT
>> -      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode))
>> +      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)
>> +      /* We can use mode change in LRA for some transformations.  */
>> +      && ! lra_in_progress)
>>       return -1;
>>   #endif
> I don't think this change is reflected in the ChangeLog.
>
You are right.  I added the change description in the ChangeLog. Reload 
has no problem in representation of some decisions because it uses 
internal representation.  It is a problem for LRA using RTL when for 
example two insn operands in different modes should have the same hard 
register according to the constraint.  I use subreg for that even it is 
not a correct in other parts of the compiler.
> I think just the minor ChangeLog updates and clarification of the 
> changes to jump.c are all that's needed for this patch to be approved.
>
Thanks, Jeff.  I really appreciate your help.

Patch

Index: rtl.h
===================================================================
--- rtl.h	(revision 191771)
+++ rtl.h	(working copy)
@@ -2369,6 +2369,9 @@  extern int epilogue_completed;
 
 extern int reload_in_progress;
 
+/* Set to 1 while in lra.  */
+extern int lra_in_progress;
+
 /* This macro indicates whether you may create a new
    pseudo-register.  */
 
Index: ira.c
===================================================================
--- ira.c	(revision 191771)
+++ ira.c	(working copy)
@@ -4308,6 +4308,9 @@  bool ira_conflicts_p;
 /* Saved between IRA and reload.  */
 static int saved_flag_ira_share_spill_slots;
 
+/* Set to 1 while in lra.  */
+int lra_in_progress = 0;
+
 /* This is the main entry of IRA.  */
 static void
 ira (FILE *f)
Index: jump.c
===================================================================
--- jump.c	(revision 191771)
+++ jump.c	(working copy)
@@ -1868,7 +1868,8 @@  true_regnum (const_rtx x)
 {
   if (REG_P (x))
     {
-      if (REGNO (x) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO (x)] >= 0)
+      if (REGNO (x) >= FIRST_PSEUDO_REGISTER
+	  && (lra_in_progress || reg_renumber[REGNO (x)] >= 0))
 	return reg_renumber[REGNO (x)];
       return REGNO (x);
     }
@@ -1880,7 +1881,8 @@  true_regnum (const_rtx x)
 	{
 	  struct subreg_info info;
 
-	  subreg_get_info (REGNO (SUBREG_REG (x)),
+	  subreg_get_info (lra_in_progress
+			   ? (unsigned) base : REGNO (SUBREG_REG (x)),
 			   GET_MODE (SUBREG_REG (x)),
 			   SUBREG_BYTE (x), GET_MODE (x), &info);
 
Index: rtlanal.c
===================================================================
--- rtlanal.c	(revision 191771)
+++ rtlanal.c	(working copy)
@@ -3465,7 +3465,9 @@  simplify_subreg_regno (unsigned int xreg
   /* Give the backend a chance to disallow the mode change.  */
   if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT
       && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT
-      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode))
+      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)
+      /* We can use mode change in LRA for some transformations.  */
+      && ! lra_in_progress)
     return -1;
 #endif
 
@@ -3475,10 +3477,16 @@  simplify_subreg_regno (unsigned int xreg
     return -1;
 
   if (FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM
-      && xregno == ARG_POINTER_REGNUM)
+      /* We should convert arg register in LRA after the elimination
+	 if it is possible.  */
+      && xregno == ARG_POINTER_REGNUM
+      && ! lra_in_progress)
     return -1;
 
-  if (xregno == STACK_POINTER_REGNUM)
+  if (xregno == STACK_POINTER_REGNUM
+      /* We should convert hard stack register in LRA if it is
+	 possible.  */
+      && ! lra_in_progress)
     return -1;
 
   /* Try to get the register offset.  */
Index: emit-rtl.c
===================================================================
--- emit-rtl.c	(revision 191771)
+++ emit-rtl.c	(working copy)
@@ -581,7 +581,7 @@  gen_rtx_REG (enum machine_mode mode, uns
      Also don't do this when we are making new REGs in reload, since
      we don't want to get confused with the real pointers.  */
 
-  if (mode == Pmode && !reload_in_progress)
+  if (mode == Pmode && !reload_in_progress && !lra_in_progress)
     {
       if (regno == FRAME_POINTER_REGNUM
 	  && (!reload_completed || frame_pointer_needed))
@@ -723,7 +723,14 @@  validate_subreg (enum machine_mode omode
      (subreg:SI (reg:DF) 0) isn't.  */
   else if (FLOAT_MODE_P (imode) || FLOAT_MODE_P (omode))
     {
-      if (isize != osize)
+      if (! (isize == osize
+	     /* LRA can use subreg to store a floating point value in
+		an integer mode.  Although the floating point and the
+		integer modes need the same number of hard registers,
+		the size of floating point mode can be less than the
+		integer mode.  LRA also uses subregs for a register
+		should be used in different mode in on insn.  */
+	     || lra_in_progress))
 	return false;
     }
 
@@ -756,7 +763,8 @@  validate_subreg (enum machine_mode omode
      of a subword.  A subreg does *not* perform arbitrary bit extraction.
      Given that we've already checked mode/offset alignment, we only have
      to check subword subregs here.  */
-  if (osize < UNITS_PER_WORD)
+  if (osize < UNITS_PER_WORD
+      && ! (lra_in_progress && (FLOAT_MODE_P (imode) || FLOAT_MODE_P (omode))))
     {
       enum machine_mode wmode = isize > UNITS_PER_WORD ? word_mode : imode;
       unsigned int low_off = subreg_lowpart_offset (omode, wmode);
Index: expr.c
===================================================================
--- expr.c	(revision 191771)
+++ expr.c	(working copy)
@@ -3434,9 +3434,13 @@  emit_move_insn_1 (rtx x, rtx y)
      fits within a HOST_WIDE_INT.  */
   if (!CONSTANT_P (y) || GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT)
     {
-      rtx ret = emit_move_via_integer (mode, x, y, false);
+      rtx ret = emit_move_via_integer (mode, x, y, lra_in_progress);
+
       if (ret)
-	return ret;
+	{
+	  if (! lra_in_progress || recog (PATTERN (ret), ret, 0) >= 0)
+	    return ret;
+	}
     }
 
   return emit_move_multi_word (mode, x, y);
Index: recog.c
===================================================================
--- recog.c	(revision 191771)
+++ recog.c	(working copy)
@@ -993,6 +993,12 @@  general_operand (rtx op, enum machine_mo
       /* FLOAT_MODE subregs can't be paradoxical.  Combine will occasionally
 	 create such rtl, and we must reject it.  */
       if (SCALAR_FLOAT_MODE_P (GET_MODE (op))
+	  /* LRA can use subreg to store a floating point value in an
+	     integer mode.  Although the floating point and the
+	     integer modes need the same number of hard registers, the
+	     size of floating point mode can be less than the integer
+	     mode.  */
+	  && ! lra_in_progress 
 	  && GET_MODE_SIZE (GET_MODE (op)) > GET_MODE_SIZE (GET_MODE (sub)))
 	return 0;
 
@@ -1068,6 +1074,12 @@  register_operand (rtx op, enum machine_m
       /* FLOAT_MODE subregs can't be paradoxical.  Combine will occasionally
 	 create such rtl, and we must reject it.  */
       if (SCALAR_FLOAT_MODE_P (GET_MODE (op))
+	  /* LRA can use subreg to store a floating point value in an
+	     integer mode.  Although the floating point and the
+	     integer modes need the same number of hard registers, the
+	     size of floating point mode can be less than the integer
+	     mode.  */
+	  && ! lra_in_progress 
 	  && GET_MODE_SIZE (GET_MODE (op)) > GET_MODE_SIZE (GET_MODE (sub)))
 	return 0;
 
@@ -1099,7 +1111,7 @@  scratch_operand (rtx op, enum machine_mo
 
   return (GET_CODE (op) == SCRATCH
 	  || (REG_P (op)
-	      && REGNO (op) < FIRST_PSEUDO_REGISTER));
+	      && (lra_in_progress || REGNO (op) < FIRST_PSEUDO_REGISTER)));
 }
 
 /* Return 1 if OP is a valid immediate operand for mode MODE.