diff mbox

RFC: LRA for x86/x86-64 [1/9]

Message ID 5064D9B8.4070401@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov Sept. 27, 2012, 10:56 p.m. UTC 
The following patch adds a new argument for function alter_subreg.  
LRA will sometime call alter_subreg with different argument value.

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

     * output.h (alter_subreg): Add new argument.
     * dbxout.c (dbxout_symbol_location): Pass new argument to
     alter_subreg.
     * sdbout.c (sdbout_symbol): Pass new argument to alter_subreg.
     * final.c (final_scan_insn, cleanup_subreg_operands): Pass new
     argument to alter_subreg.
     (walk_alter_subreg, output_operand): Ditto.
     (alter_subreg): Add new argument.
     * config/m32r/m32r.c (gen_split_move_double): Pass new argument to
     alter_subreg.
     * config/sh/sh.md: Ditto.
     * config/xtensa/xtensa.c (fixup_subreg_mem): Ditto.
     * config/m68k/m68k.c (emit_move_sequence): Ditto.
     * config/arm/arm.c (load_multiple_sequence): Ditto.
     (store_multiple_sequence): Ditto.
     * config/pa/pa.c (pa_emit_move_sequence): Ditto.
     * config/v850/v850.c (v850_reorg): Ditto.

Comments

Jeff Law Sept. 28, 2012, 4:07 p.m. UTC | #1 
On 09/27/2012 04:56 PM, Vladimir Makarov wrote:
>    The following patch adds a new argument for function alter_subreg.
> LRA will sometime call alter_subreg with different argument value.
>
> 2012-09-27  Vladimir Makarov  <vmakarov@redhat.com>
>
>      * output.h (alter_subreg): Add new argument.
>      * dbxout.c (dbxout_symbol_location): Pass new argument to
>      alter_subreg.
>      * sdbout.c (sdbout_symbol): Pass new argument to alter_subreg.
>      * final.c (final_scan_insn, cleanup_subreg_operands): Pass new
>      argument to alter_subreg.
>      (walk_alter_subreg, output_operand): Ditto.
>      (alter_subreg): Add new argument.
>      * config/m32r/m32r.c (gen_split_move_double): Pass new argument to
>      alter_subreg.
>      * config/sh/sh.md: Ditto.
>      * config/xtensa/xtensa.c (fixup_subreg_mem): Ditto.
>      * config/m68k/m68k.c (emit_move_sequence): Ditto.
>      * config/arm/arm.c (load_multiple_sequence): Ditto.
>      (store_multiple_sequence): Ditto.
>      * config/pa/pa.c (pa_emit_move_sequence): Ditto.
>      * config/v850/v850.c (v850_reorg): Ditto.
This is OK.

jeff
Richard Sandiford Sept. 30, 2012, 9:10 a.m. UTC | #2 
Hi Vlad,

Vladimir Makarov <vmakarov@redhat.com> writes:
> @@ -2973,11 +2973,11 @@ cleanup_subreg_operands (rtx insn)
>      df_insn_rescan (insn);
>  }
>  
> -/* If X is a SUBREG, replace it with a REG or a MEM,
> -   based on the thing it is a subreg of.  */
> +/* If X is a SUBREG, try to replace it with a REG or a MEM, based on
> +   the thing it is a subreg of.  Do it anyway if FINAL_P.  */
>  
>  rtx
> -alter_subreg (rtx *xp)
> +alter_subreg (rtx *xp, bool final_p)
>  {
>    rtx x = *xp;
>    rtx y = SUBREG_REG (x);
> @@ -3001,16 +3001,19 @@ alter_subreg (rtx *xp)
>              offset += difference % UNITS_PER_WORD;
>          }
>  
> -      *xp = adjust_address (y, GET_MODE (x), offset);
> +      if (final_p)
> +	*xp = adjust_address (y, GET_MODE (x), offset);
> +      else
> +	*xp = adjust_address_nv (y, GET_MODE (x), offset);
>      }
>    else
>      {
>        rtx new_rtx = simplify_subreg (GET_MODE (x), y, GET_MODE (y),
> -				 SUBREG_BYTE (x));
> +				     SUBREG_BYTE (x));
>  
>        if (new_rtx != 0)
>  	*xp = new_rtx;
> -      else if (REG_P (y))
> +      else if (final_p && REG_P (y))
>  	{
>  	  /* Simplify_subreg can't handle some REG cases, but we have to.  */
>  	  unsigned int regno;

Could you add a bit more commentary to explain the MEM case?
The REG handling obviously matches the comment at the head of the function:
if FINAL_P, we replace a (subreg (reg)) with a (reg) even in cases where
simplify_subreg wouldn't.  If !FINAL_P we leave things be.

But in the MEM case it's more a verify vs. don't verify thing.  I assume
the idea is that LRA wants to see the rtl for invalid addresses and have
an opportunity to make them valid (because LRA works on rtl rather an
internal representation, like you said elsewhere).  It would be nice to
make that more explicit here.

Thanks,
Richard
diff mbox

Patch

Index: dbxout.c
===================================================================
--- dbxout.c	(revision 191771)
+++ dbxout.c	(working copy)
@@ -2994,7 +2994,7 @@  dbxout_symbol_location (tree decl, tree
 	  if (REGNO (value) >= FIRST_PSEUDO_REGISTER)
 	    return 0;
 	}
-      home = alter_subreg (&home);
+      home = alter_subreg (&home, true);
     }
   if (REG_P (home))
     {
Index: config/pa/pa.c
===================================================================
--- config/pa/pa.c	(revision 191771)
+++ config/pa/pa.c	(working copy)
@@ -1616,7 +1616,7 @@  pa_emit_move_sequence (rtx *operands, en
       rtx temp = gen_rtx_SUBREG (GET_MODE (operand0),
 				 reg_equiv_mem (REGNO (SUBREG_REG (operand0))),
 				 SUBREG_BYTE (operand0));
-      operand0 = alter_subreg (&temp);
+      operand0 = alter_subreg (&temp, true);
     }
 
   if (scratch_reg
@@ -1633,7 +1633,7 @@  pa_emit_move_sequence (rtx *operands, en
       rtx temp = gen_rtx_SUBREG (GET_MODE (operand1),
 				 reg_equiv_mem (REGNO (SUBREG_REG (operand1))),
 				 SUBREG_BYTE (operand1));
-      operand1 = alter_subreg (&temp);
+      operand1 = alter_subreg (&temp, true);
     }
 
   if (scratch_reg && reload_in_progress && GET_CODE (operand0) == MEM
Index: config/v850/v850.c
===================================================================
--- config/v850/v850.c	(revision 191771)
+++ config/v850/v850.c	(working copy)
@@ -1301,11 +1301,11 @@  v850_reorg (void)
 	      if (GET_CODE (dest) == SUBREG
 		  && (GET_CODE (SUBREG_REG (dest)) == MEM
 		      || GET_CODE (SUBREG_REG (dest)) == REG))
-		alter_subreg (&dest);
+		alter_subreg (&dest, true);
 	      if (GET_CODE (src) == SUBREG
 		  && (GET_CODE (SUBREG_REG (src)) == MEM
 		      || GET_CODE (SUBREG_REG (src)) == REG))
-		alter_subreg (&src);
+		alter_subreg (&src, true);
 
 	      if (GET_CODE (dest) == MEM && GET_CODE (src) == MEM)
 		mem = NULL_RTX;
Index: config/sh/sh.md
===================================================================
--- config/sh/sh.md	(revision 191771)
+++ config/sh/sh.md	(working copy)
@@ -7275,7 +7275,7 @@  label:
 	  rtx regop = operands[store_p], word0 ,word1;
 
 	  if (GET_CODE (regop) == SUBREG)
-	    alter_subreg (&regop);
+	    alter_subreg (&regop, true);
 	  if (REGNO (XEXP (addr, 0)) == REGNO (XEXP (addr, 1)))
 	    offset = 2;
 	  else
@@ -7283,9 +7283,9 @@  label:
 	  mem = copy_rtx (mem);
 	  PUT_MODE (mem, SImode);
 	  word0 = gen_rtx_SUBREG (SImode, regop, 0);
-	  alter_subreg (&word0);
+	  alter_subreg (&word0, true);
 	  word1 = gen_rtx_SUBREG (SImode, regop, 4);
-	  alter_subreg (&word1);
+	  alter_subreg (&word1, true);
 	  if (store_p || ! refers_to_regno_p (REGNO (word0),
 					      REGNO (word0) + 1, addr, 0))
 	    {
@@ -7743,7 +7743,7 @@  label:
       else
 	{
 	  x = gen_rtx_SUBREG (V2SFmode, operands[0], i * 8);
-	  alter_subreg (&x);
+	  alter_subreg (&x, true);
 	}
 
       if (MEM_P (operands[1]))
@@ -7752,7 +7752,7 @@  label:
       else
 	{
 	  y = gen_rtx_SUBREG (V2SFmode, operands[1], i * 8);
-	  alter_subreg (&y);
+	  alter_subreg (&y, true);
 	}
 
       emit_insn (gen_movv2sf_i (x, y));
Index: config/xtensa/xtensa.c
===================================================================
--- config/xtensa/xtensa.c	(revision 191771)
+++ config/xtensa/xtensa.c	(working copy)
@@ -1087,7 +1087,7 @@  fixup_subreg_mem (rtx x)
 	gen_rtx_SUBREG (GET_MODE (x),
 			reg_equiv_mem (REGNO (SUBREG_REG (x))),
 			SUBREG_BYTE (x));
-      x = alter_subreg (&temp);
+      x = alter_subreg (&temp, true);
     }
   return x;
 }
Index: config/m68k/m68k.c
===================================================================
--- config/m68k/m68k.c	(revision 191771)
+++ config/m68k/m68k.c	(working copy)
@@ -3658,7 +3658,7 @@  emit_move_sequence (rtx *operands, enum
       rtx temp = gen_rtx_SUBREG (GET_MODE (operand0),
 				 reg_equiv_mem (REGNO (SUBREG_REG (operand0))),
 				 SUBREG_BYTE (operand0));
-      operand0 = alter_subreg (&temp);
+      operand0 = alter_subreg (&temp, true);
     }
 
   if (scratch_reg
@@ -3675,7 +3675,7 @@  emit_move_sequence (rtx *operands, enum
       rtx temp = gen_rtx_SUBREG (GET_MODE (operand1),
 				 reg_equiv_mem (REGNO (SUBREG_REG (operand1))),
 				 SUBREG_BYTE (operand1));
-      operand1 = alter_subreg (&temp);
+      operand1 = alter_subreg (&temp, true);
     }
 
   if (scratch_reg && reload_in_progress && GET_CODE (operand0) == MEM
Index: config/m32r/m32r.c
===================================================================
--- config/m32r/m32r.c	(revision 191771)
+++ config/m32r/m32r.c	(working copy)
@@ -1030,9 +1030,9 @@  gen_split_move_double (rtx operands[])
      subregs to make this code simpler.  It is safe to call
      alter_subreg any time after reload.  */
   if (GET_CODE (dest) == SUBREG)
-    alter_subreg (&dest);
+    alter_subreg (&dest, true);
   if (GET_CODE (src) == SUBREG)
-    alter_subreg (&src);
+    alter_subreg (&src, true);
 
   start_sequence ();
   if (REG_P (dest))
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 191771)
+++ config/arm/arm.c	(working copy)
@@ -10316,7 +10316,7 @@  load_multiple_sequence (rtx *operands, i
 
       /* Convert a subreg of a mem into the mem itself.  */
       if (GET_CODE (operands[nops + i]) == SUBREG)
-	operands[nops + i] = alter_subreg (operands + (nops + i));
+	operands[nops + i] = alter_subreg (operands + (nops + i), true);
 
       gcc_assert (MEM_P (operands[nops + i]));
 
@@ -10468,7 +10468,7 @@  store_multiple_sequence (rtx *operands,
 
       /* Convert a subreg of a mem into the mem itself.  */
       if (GET_CODE (operands[nops + i]) == SUBREG)
-	operands[nops + i] = alter_subreg (operands + (nops + i));
+	operands[nops + i] = alter_subreg (operands + (nops + i), true);
 
       gcc_assert (MEM_P (operands[nops + i]));
 
Index: sdbout.c
===================================================================
--- sdbout.c	(revision 191771)
+++ sdbout.c	(working copy)
@@ -767,7 +767,7 @@  sdbout_symbol (tree decl, int local)
 	      if (REGNO (value) >= FIRST_PSEUDO_REGISTER)
 		return;
 	    }
-	  regno = REGNO (alter_subreg (&value));
+	  regno = REGNO (alter_subreg (&value, true));
 	  SET_DECL_RTL (decl, value);
 	}
       /* Don't output anything if an auto variable
Index: final.c
===================================================================
--- final.c	(revision 191771)
+++ final.c	(working copy)
@@ -2534,7 +2534,7 @@  final_scan_insn (rtx insn, FILE *file, i
 	      {
 		rtx src1, src2;
 		if (GET_CODE (SET_SRC (set)) == SUBREG)
-		  SET_SRC (set) = alter_subreg (&SET_SRC (set));
+		  SET_SRC (set) = alter_subreg (&SET_SRC (set), true);
 
 		src1 = SET_SRC (set);
 		src2 = NULL_RTX;
@@ -2542,10 +2542,10 @@  final_scan_insn (rtx insn, FILE *file, i
 		  {
 		    if (GET_CODE (XEXP (SET_SRC (set), 0)) == SUBREG)
 		      XEXP (SET_SRC (set), 0)
-			= alter_subreg (&XEXP (SET_SRC (set), 0));
+			= alter_subreg (&XEXP (SET_SRC (set), 0), true);
 		    if (GET_CODE (XEXP (SET_SRC (set), 1)) == SUBREG)
 		      XEXP (SET_SRC (set), 1)
-			= alter_subreg (&XEXP (SET_SRC (set), 1));
+			= alter_subreg (&XEXP (SET_SRC (set), 1), true);
 		    if (XEXP (SET_SRC (set), 1)
 			== CONST0_RTX (GET_MODE (XEXP (SET_SRC (set), 0))))
 		      src2 = XEXP (SET_SRC (set), 0);
@@ -2948,7 +2948,7 @@  cleanup_subreg_operands (rtx insn)
 	 expression directly.  */
       if (GET_CODE (*recog_data.operand_loc[i]) == SUBREG)
 	{
-	  recog_data.operand[i] = alter_subreg (recog_data.operand_loc[i]);
+	  recog_data.operand[i] = alter_subreg (recog_data.operand_loc[i], true);
 	  changed = true;
 	}
       else if (GET_CODE (recog_data.operand[i]) == PLUS
@@ -2961,7 +2961,7 @@  cleanup_subreg_operands (rtx insn)
     {
       if (GET_CODE (*recog_data.dup_loc[i]) == SUBREG)
 	{
-	  *recog_data.dup_loc[i] = alter_subreg (recog_data.dup_loc[i]);
+	  *recog_data.dup_loc[i] = alter_subreg (recog_data.dup_loc[i], true);
 	  changed = true;
 	}
       else if (GET_CODE (*recog_data.dup_loc[i]) == PLUS
@@ -2973,11 +2973,11 @@  cleanup_subreg_operands (rtx insn)
     df_insn_rescan (insn);
 }
 
-/* If X is a SUBREG, replace it with a REG or a MEM,
-   based on the thing it is a subreg of.  */
+/* If X is a SUBREG, try to replace it with a REG or a MEM, based on
+   the thing it is a subreg of.  Do it anyway if FINAL_P.  */
 
 rtx
-alter_subreg (rtx *xp)
+alter_subreg (rtx *xp, bool final_p)
 {
   rtx x = *xp;
   rtx y = SUBREG_REG (x);
@@ -3001,16 +3001,19 @@  alter_subreg (rtx *xp)
             offset += difference % UNITS_PER_WORD;
         }
 
-      *xp = adjust_address (y, GET_MODE (x), offset);
+      if (final_p)
+	*xp = adjust_address (y, GET_MODE (x), offset);
+      else
+	*xp = adjust_address_nv (y, GET_MODE (x), offset);
     }
   else
     {
       rtx new_rtx = simplify_subreg (GET_MODE (x), y, GET_MODE (y),
-				 SUBREG_BYTE (x));
+				     SUBREG_BYTE (x));
 
       if (new_rtx != 0)
 	*xp = new_rtx;
-      else if (REG_P (y))
+      else if (final_p && REG_P (y))
 	{
 	  /* Simplify_subreg can't handle some REG cases, but we have to.  */
 	  unsigned int regno;
@@ -3050,7 +3053,7 @@  walk_alter_subreg (rtx *xp, bool *change
 
     case SUBREG:
       *changed = true;
-      return alter_subreg (xp);
+      return alter_subreg (xp, true);
 
     default:
       break;
@@ -3656,7 +3659,7 @@  void
 output_operand (rtx x, int code ATTRIBUTE_UNUSED)
 {
   if (x && GET_CODE (x) == SUBREG)
-    x = alter_subreg (&x);
+    x = alter_subreg (&x, true);
 
   /* X must not be a pseudo reg.  */
   gcc_assert (!x || !REG_P (x) || REGNO (x) < FIRST_PSEUDO_REGISTER);
Index: output.h
===================================================================
--- output.h	(revision 191771)
+++ output.h	(working copy)
@@ -76,7 +76,7 @@  extern rtx final_scan_insn (rtx, FILE *,
 
 /* Replace a SUBREG with a REG or a MEM, based on the thing it is a
    subreg of.  */
-extern rtx alter_subreg (rtx *);
+extern rtx alter_subreg (rtx *, bool);
 
 /* Print an operand using machine-dependent assembler syntax.  */
 extern void output_operand (rtx, int);