[committed] PR29006: Incorrect zeroing of unaligned 64-bit fields on MIPS targets

[Richard Sandiford]

I found recently that 64-bit MIPS targets generate 32-bit rather than
64-bit accesses when zeroing an unaligned 64-bit field.  This is a
regression from 3.3.  There are two problems:

  (1) mips_expand_unaligned_store checks the mode of the source to see
      whether a 64-bit or 32-bit access is needed.  This is wrong for
      const0_rtx; the function should be checking the mode returned by
      mode_for_size instead.  (It could also check the width directly,
      with only personal taste to choose between them.)

  (2) The 32-bit and 64-bit patterns use the same UNSPEC number.
      This doesn't matter for loads because the mode of the UNSPEC
      decides between the 32-bit and 64-bit versions, but both store
      widths use BLKmode.

      The two obvious fixes are:

         (a) to go back to using separate UNSPEC numbers for
             32-bit and 64-bit accesses

         (b) to check the size of the BLKmode memory is what we
             think it is.

      I think (b) is more robust; it simulates a mode check, and a mode
      check is exactly what we'd use here if we could use integer modes
      for unaligned MEMs.

Tested on mipsisa64-elf.  Applied to trunk, 4.1 and 4.0.

Richard


gcc/
	PR target/29006
	* config/mips/mips-protos.h (mips_mem_fits_mode_p): Declare.
	* config/mips/mips.c (mips_expand_unaligned_store): Use the mode
	returned by mode_for_size, rather than the mode of src itself,
	to choose between 32-bit and 64-bit patterns.
	(mips_mem_fits_mode_p): New function.
	* config/mips/mips.md (mov_<load>l, mov_<load>r): Use it to check
	that the size of the source matches the size of the destination.
	(mov_<store>l, mov_<store>r): Likewise.

gcc/testsuite/
	PR target/29006
	* gcc.c-torture/execute/pr29006.c: New test.

Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc/config/mips/mips-protos.h	(revision 116811)
+++ gcc/config/mips/mips-protos.h	(working copy)
@@ -189,6 +189,7 @@ extern void mips_va_start (tree, rtx);
 
 extern bool mips_expand_unaligned_load (rtx, rtx, unsigned int, int);
 extern bool mips_expand_unaligned_store (rtx, rtx, unsigned int, int);
+extern bool mips_mem_fits_mode_p (enum machine_mode mode, rtx x);
 extern void override_options (void);
 extern void mips_conditional_register_usage (void);
 extern void mips_order_regs_for_local_alloc (void);
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 116811)
+++ gcc/config/mips/mips.c	(working copy)
@@ -4541,13 +4541,15 @@ mips_expand_unaligned_load (rtx dest, rt
 mips_expand_unaligned_store (rtx dest, rtx src, unsigned int width, int bitpos)
 {
   rtx left, right;
+  enum machine_mode mode;
 
   if (!mips_get_unaligned_mem (&dest, width, bitpos, &left, &right))
     return false;
 
-  src = gen_lowpart (mode_for_size (width, MODE_INT, 0), src);
+  mode = mode_for_size (width, MODE_INT, 0);
+  src = gen_lowpart (mode, src);
 
-  if (GET_MODE (src) == DImode)
+  if (mode == DImode)
     {
       emit_insn (gen_mov_sdl (dest, src, left));
       emit_insn (gen_mov_sdr (copy_rtx (dest), copy_rtx (src), right));
@@ -4560,6 +4562,20 @@ mips_expand_unaligned_store (rtx dest, r
   return true;
 }
 
+/* Return true if X is a MEM with the same size as MODE.  */
+
+bool
+mips_mem_fits_mode_p (enum machine_mode mode, rtx x)
+{
+  rtx size;
+
+  if (!MEM_P (x))
+    return false;
+
+  size = MEM_SIZE (x);
+  return size && INTVAL (size) == GET_MODE_SIZE (mode);
+}
+
 /* Return true if (zero_extract OP SIZE POSITION) can be used as the
    source of an "ext" instruction or the destination of an "ins"
    instruction.  OP must be a register operand and the following
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 116811)
+++ gcc/config/mips/mips.md	(working copy)
@@ -2970,7 +2970,7 @@ (define_insn "mov_<load>l"
 	(unspec:GPR [(match_operand:BLK 1 "memory_operand" "m")
 		     (match_operand:QI 2 "memory_operand" "m")]
 		    UNSPEC_LOAD_LEFT))]
-  "!TARGET_MIPS16"
+  "!TARGET_MIPS16 && mips_mem_fits_mode_p (<MODE>mode, operands[1])"
   "<load>l\t%0,%2"
   [(set_attr "type" "load")
    (set_attr "mode" "<MODE>")])
@@ -2981,7 +2981,7 @@ (define_insn "mov_<load>r"
 		     (match_operand:QI 2 "memory_operand" "m")
 		     (match_operand:GPR 3 "register_operand" "0")]
 		    UNSPEC_LOAD_RIGHT))]
-  "!TARGET_MIPS16"
+  "!TARGET_MIPS16 && mips_mem_fits_mode_p (<MODE>mode, operands[1])"
   "<load>r\t%0,%2"
   [(set_attr "type" "load")
    (set_attr "mode" "<MODE>")])
@@ -2991,7 +2991,7 @@ (define_insn "mov_<store>l"
 	(unspec:BLK [(match_operand:GPR 1 "reg_or_0_operand" "dJ")
 		     (match_operand:QI 2 "memory_operand" "m")]
 		    UNSPEC_STORE_LEFT))]
-  "!TARGET_MIPS16"
+  "!TARGET_MIPS16 && mips_mem_fits_mode_p (<MODE>mode, operands[0])"
   "<store>l\t%z1,%2"
   [(set_attr "type" "store")
    (set_attr "mode" "<MODE>")])
@@ -3002,7 +3002,7 @@ (define_insn "mov_<store>r"
 		     (match_operand:QI 2 "memory_operand" "m")
 		     (match_dup 0)]
 		    UNSPEC_STORE_RIGHT))]
-  "!TARGET_MIPS16"
+  "!TARGET_MIPS16 && mips_mem_fits_mode_p (<MODE>mode, operands[0])"
   "<store>r\t%z1,%2"
   [(set_attr "type" "store")
    (set_attr "mode" "<MODE>")])
Index: gcc/testsuite/gcc.c-torture/execute/pr29006.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/pr29006.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/pr29006.c	(revision 0)
@@ -0,0 +1,3 @@
+struct __attribute__((__packed__)) s { char c; unsigned long long x; };
+void __attribute__((__noinline__)) foo (struct s *s) { s->x = 0; }
+int main (void) { struct s s = { 1, ~0ULL }; foo (&s); return s.x != 0; }