Some MIPS movtf fixes

Richard Sandiford rdsandiford@googlemail.com
Sun Jun 1 13:02:00 GMT 2008


This patch fixes a couple of problems I noticed while working on the
movti part of the MIPS HI/LO patch.  Both problems are related to
doubleword accesses.

A doubleword move can in general be decomposed into two word moves,
so we need to make sure that a doubleword address can be decomposed
into two word addresses.  We were failing to do this in two cases:

  - We allowed addresses like (plus BASE_REG (const_int 0x7fff)),
    which led to an out-of-range offset for the second word.

  - We always allowed LO_SUM addresses for doubleword values.
    This was wrong for 128-bit types on o64, which only guarantees
    64-bit alignment, and for which adding UNITS_PER_WORD to the
    LO_SUM may induce a carry.

The current movtf instruction was also wrong for MIPS16.  I'm not
sure if anyone actually uses MIPS16 and 128-bit long doubles together,
but movtf at least ought to work.

The MIPS16 version of movtf needs to use normal memory constraints,
so for consistency with that and with the 32-bit doubleword patterns,
I made the movtf version look more like a normal move pattern.
This included adding the usual predicate checks to the insn condition
and using {,fp}{load,store} constraints.

For completeness, I added a TARGET_64BIT check.  I don't think we'd
ever try to use the pattern on 32-bit targets, but it's better to
have the check anyway.

Finally, I added some extra tests that should cover all movtf
constraints.

Tested on mips64el-linux-gnu and mipsisa64-elfoabi.  Applied.

Richard

PS. I haven't forgotten about the need to revisit mips16_attribute.


gcc/
	* config/mips/mips.c (mips_valid_offset_p): New function.
	(mips_valid_lo_sum_p): Likewise.
	(mips_classify_address): Use them.
	(mips_force_address): New function.
	(mips_legitimize_address): Use it.
	* config/mips/mips.md (MOVE128): New mode iterator.
	(movtf): Require TARGET_64BIT.  Remove empty strings.
	(*movtf_internal): Rename to...
	(*movtf): ...this and require !TARGET_MIPS16.  Use "m" instead
	of "R" and use {,fp}{load,store} attributes instead of "multi".
	Use a separate define_split.
	(*movtf_mips16): New pattern.

gcc/testsuite/
	* gcc.target/mips/fpr-moves-7.c: New test.
	* gcc.target/mips/fpr-moves-8.c: New test.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2008-06-01 11:01:38.000000000 +0100
+++ gcc/config/mips/mips.c	2008-06-01 11:46:17.000000000 +0100
@@ -1809,6 +1809,51 @@ mips_valid_base_register_p (rtx x, enum 
 	  && mips_regno_mode_ok_for_base_p (REGNO (x), mode, strict_p));
 }
 
+/* Return true if, for every base register BASE_REG, (plus BASE_REG X)
+   can address a value of mode MODE.  */
+
+static bool
+mips_valid_offset_p (rtx x, enum machine_mode mode)
+{
+  /* Check that X is a signed 16-bit number.  */
+  if (!const_arith_operand (x, Pmode))
+    return false;
+
+  /* We may need to split multiword moves, so make sure that every word
+     is accessible.  */
+  if (GET_MODE_SIZE (mode) > UNITS_PER_WORD
+      && !SMALL_OPERAND (INTVAL (x) + GET_MODE_SIZE (mode) - UNITS_PER_WORD))
+    return false;
+
+  return true;
+}
+
+/* Return true if a LO_SUM can address a value of mode MODE when the
+   LO_SUM symbol has type SYMBOL_TYPE.  */
+
+static bool
+mips_valid_lo_sum_p (enum mips_symbol_type symbol_type, enum machine_mode mode)
+{
+  /* Check that symbols of type SYMBOL_TYPE can be used to access values
+     of mode MODE.  */
+  if (mips_symbol_insns (symbol_type, mode) == 0)
+    return false;
+
+  /* Check that there is a known low-part relocation.  */
+  if (mips_lo_relocs[symbol_type] == NULL)
+    return false;
+
+  /* We may need to split multiword moves, so make sure that each word
+     can be accessed without inducing a carry.  This is mainly needed
+     for o64, which has historically only guaranteed 64-bit alignment
+     for 128-bit types.  */
+  if (GET_MODE_SIZE (mode) > UNITS_PER_WORD
+      && GET_MODE_BITSIZE (mode) > GET_MODE_ALIGNMENT (mode))
+    return false;
+
+  return true;
+}
+
 /* Return true if X is a valid address for machine mode MODE.  If it is,
    fill in INFO appropriately.  STRICT_P is true if REG_OK_STRICT is in
    effect.  */
@@ -1831,7 +1876,7 @@ mips_classify_address (struct mips_addre
       info->reg = XEXP (x, 0);
       info->offset = XEXP (x, 1);
       return (mips_valid_base_register_p (info->reg, mode, strict_p)
-	      && const_arith_operand (info->offset, VOIDmode));
+	      && mips_valid_offset_p (info->offset, mode));
 
     case LO_SUM:
       info->type = ADDRESS_LO_SUM;
@@ -1849,8 +1894,7 @@ mips_classify_address (struct mips_addre
       info->symbol_type
 	= mips_classify_symbolic_expression (info->offset, SYMBOL_CONTEXT_MEM);
       return (mips_valid_base_register_p (info->reg, mode, strict_p)
-	      && mips_symbol_insns (info->symbol_type, mode) > 0
-	      && mips_lo_relocs[info->symbol_type] != 0);
+	      && mips_valid_lo_sum_p (info->symbol_type, mode));
 
     case CONST_INT:
       /* Small-integer addresses don't occur very often, but they
@@ -2473,6 +2517,16 @@ mips_legitimize_tls_address (rtx loc)
   return dest;
 }
 
+/* If X is not a valid address for mode MODE, force it into a register.  */
+
+static rtx
+mips_force_address (rtx x, enum machine_mode mode)
+{
+  if (!mips_legitimate_address_p (mode, x, false))
+    x = force_reg (Pmode, x);
+  return x;
+}
+
 /* This function is used to implement LEGITIMIZE_ADDRESS.  If *XLOC can
    be legitimized in a way that the generic machinery might not expect,
    put the new address in *XLOC and return true.  MODE is the mode of
@@ -2481,7 +2535,7 @@ mips_legitimize_tls_address (rtx loc)
 bool
 mips_legitimize_address (rtx *xloc, enum machine_mode mode)
 {
-  rtx base;
+  rtx base, addr;
   HOST_WIDE_INT offset;
 
   if (mips_tls_symbol_p (*xloc))
@@ -2491,8 +2545,11 @@ mips_legitimize_address (rtx *xloc, enum
     }
 
   /* See if the address can split into a high part and a LO_SUM.  */
-  if (mips_split_symbol (NULL, *xloc, mode, xloc))
-    return true;
+  if (mips_split_symbol (NULL, *xloc, mode, &addr))
+    {
+      *xloc = mips_force_address (addr, mode);
+      return true;
+    }
 
   /* Handle BASE + OFFSET using mips_add_offset.  */
   mips_split_plus (*xloc, &base, &offset);
@@ -2500,7 +2557,8 @@ mips_legitimize_address (rtx *xloc, enum
     {
       if (!mips_valid_base_register_p (base, mode, false))
 	base = copy_to_mode_reg (Pmode, base);
-      *xloc = mips_add_offset (NULL, base, offset);
+      addr = mips_add_offset (NULL, base, offset);
+      *xloc = mips_force_address (addr, mode);
       return true;
     }
   return false;
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	2008-06-01 11:01:38.000000000 +0100
+++ gcc/config/mips/mips.md	2008-06-01 13:53:07.000000000 +0100
@@ -496,6 +496,9 @@ (define_mode_iterator MOVECC [SI (DI "TA
 (define_mode_iterator MOVE64
   [DI DF (V2SF "TARGET_HARD_FLOAT && TARGET_PAIRED_SINGLE_FLOAT")])
 
+;; 128-bit modes for which we provide move patterns on 64-bit targets.
+(define_mode_iterator MOVE128 [TF])
+
 ;; This mode iterator allows the QI and HI extension patterns to be
 ;; defined from the same template.
 (define_mode_iterator SHORT [QI HI])
@@ -4064,28 +4067,36 @@ (define_insn "*movdf_mips16"
 ;; 128-bit floating point moves
 
 (define_expand "movtf"
-  [(set (match_operand:TF 0 "")
-	(match_operand:TF 1 ""))]
-  ""
+  [(set (match_operand:TF 0)
+	(match_operand:TF 1))]
+  "TARGET_64BIT"
 {
   if (mips_legitimize_move (TFmode, operands[0], operands[1]))
     DONE;
 })
 
 ;; This pattern handles both hard- and soft-float cases.
-(define_insn_and_split "*movtf_internal"
-  [(set (match_operand:TF 0 "nonimmediate_operand" "=d,R,f,dR")
-	(match_operand:TF 1 "move_operand" "dGR,dG,dGR,f"))]
-  ""
+(define_insn "*movtf"
+  [(set (match_operand:TF 0 "nonimmediate_operand" "=d,d,m,f,d,f,m")
+	(match_operand:TF 1 "move_operand" "dG,m,dG,dG,f,m,f"))]
+  "TARGET_64BIT
+   && !TARGET_MIPS16
+   && (register_operand (operands[0], TFmode)
+       || reg_or_0_operand (operands[1], TFmode))"
   "#"
-  "&& reload_completed"
-  [(const_int 0)]
-{
-  mips_split_doubleword_move (operands[0], operands[1]);
-  DONE;
-}
-  [(set_attr "type" "multi")
-   (set_attr "length" "16")])
+  [(set_attr "type" "multi,load,store,multi,multi,fpload,fpstore")
+   (set_attr "length" "8,*,*,8,8,*,*")])
+
+(define_insn "*movtf_mips16"
+  [(set (match_operand:TF 0 "nonimmediate_operand" "=d,y,d,d,m")
+	(match_operand:TF 1 "move_operand" "d,d,y,m,d"))]
+  "TARGET_64BIT
+   && TARGET_MIPS16
+   && (register_operand (operands[0], TFmode)
+       || register_operand (operands[1], TFmode))"
+  "#"
+  [(set_attr "type" "multi,multi,multi,load,store")
+   (set_attr "length" "8,8,8,*,*")])
 
 (define_split
   [(set (match_operand:MOVE64 0 "nonimmediate_operand")
@@ -4098,6 +4109,16 @@ (define_split
   DONE;
 })
 
+(define_split
+  [(set (match_operand:MOVE128 0 "nonimmediate_operand")
+	(match_operand:MOVE128 1 "move_operand"))]
+  "TARGET_64BIT && reload_completed"
+  [(const_int 0)]
+{
+  mips_split_doubleword_move (operands[0], operands[1]);
+  DONE;
+})
+
 ;; When generating mips16 code, split moves of negative constants into
 ;; a positive "li" followed by a negation.
 (define_split
Index: gcc/testsuite/gcc.target/mips/fpr-moves-7.c
===================================================================
--- /dev/null	2008-05-31 10:25:53.548096750 +0100
+++ gcc/testsuite/gcc.target/mips/fpr-moves-7.c	2008-06-01 11:21:09.000000000 +0100
@@ -0,0 +1,38 @@
+/* { dg-do compile { target mips16_attribute } } */
+/* { dg-mips-options "-mabi=64 -msoft-float -O2 -EL" } */
+/* { dg-add-options mips16_attribute } */
+
+extern long double g[16];
+extern unsigned char gstuff[0x10000];
+
+NOMIPS16 long double
+foo (long double i1, long double i2, long double i3, long double i4,
+     long double *x, unsigned char *lstuff)
+{
+  g[0] = i1;
+  g[1] = i2;
+  g[2] = i3;
+  g[3] = i4;
+  x[0] = x[4];
+  x[1] = 0;
+  x[2] = 1.0;
+  x[3] = g[4];
+  x[4] = *(long double *) (lstuff + 0x7fff);
+  return *(long double *) (gstuff + 0x7fff);
+}
+
+MIPS16 long double
+bar (long double i1, long double i2, long double i3, long double i4,
+     long double *x, unsigned char *lstuff)
+{
+  g[0] = i1;
+  g[1] = i2;
+  g[2] = i3;
+  g[3] = i4;
+  x[0] = x[4];
+  x[1] = 0;
+  x[2] = 1.0;
+  x[3] = g[4];
+  x[4] = *(long double *) (lstuff + 0x7fff);
+  return *(long double *) (gstuff + 0x7fff);
+}
Index: gcc/testsuite/gcc.target/mips/fpr-moves-8.c
===================================================================
--- /dev/null	2008-05-31 10:25:53.548096750 +0100
+++ gcc/testsuite/gcc.target/mips/fpr-moves-8.c	2008-06-01 11:21:09.000000000 +0100
@@ -0,0 +1,38 @@
+/* { dg-do compile { target mips16_attribute } } */
+/* { dg-mips-options "-mabi=64 -msoft-float -O2 -EB" } */
+/* { dg-add-options mips16_attribute } */
+
+extern long double g[16];
+extern unsigned char gstuff[0x10000];
+
+NOMIPS16 long double
+foo (long double i1, long double i2, long double i3, long double i4,
+     long double *x, unsigned char *lstuff)
+{
+  g[0] = i1;
+  g[1] = i2;
+  g[2] = i3;
+  g[3] = i4;
+  x[0] = x[4];
+  x[1] = 0;
+  x[2] = 1.0;
+  x[3] = g[4];
+  x[4] = *(long double *) (lstuff + 0x7fff);
+  return *(long double *) (gstuff + 0x7fff);
+}
+
+MIPS16 long double
+bar (long double i1, long double i2, long double i3, long double i4,
+     long double *x, unsigned char *lstuff)
+{
+  g[0] = i1;
+  g[1] = i2;
+  g[2] = i3;
+  g[3] = i4;
+  x[0] = x[4];
+  x[1] = 0;
+  x[2] = 1.0;
+  x[3] = g[4];
+  x[4] = *(long double *) (lstuff + 0x7fff);
+  return *(long double *) (gstuff + 0x7fff);
+}



More information about the Gcc-patches mailing list