[Patch 1/2] Don't put out a call to memcpy for volatile struct operations

[James Greenhalgh <james dot greenhalgh at arm dot com>]

Hi,

Andrew is quite right, we break the contract for volatile struct copies
if we start double copying bytes.

But, the generic code will call memcpy - at which point anything could
happen. So, we must not put out a call to memcpy if either our source or
destination operands are volatile. The same is true of memset, so also
disable that call for volatile operands, and add a fallback loop
implementation.

Bootstrapped on x86, Arm and AArch64 with no issues.

OK?

Thanks,
James

---
gcc/

2014-08-21  James Greenhalgh  <james.greenhalgh@arm.com>

	* expr.c (set_storage_via_loop): New.
	(emit_block_move_hints): Do not call memcpy with volatile operands.
	(emit_block_move_via_movmem): Clarify that targets do have to care
	about volatile operands.
	(clear_storage_hints): Do not call memset for volatile operands,
	fall back to a loop implementation.

gcc/testsuite/

2014-08-21  James Greenhalgh  <james.greenhalgh@arm.com>

	* gcc.dg/large-volatile-struct-copy-1.c: New.
	* gcc.dg/large-volatile-struct-set-1.c: Likewise.

diff --git a/gcc/expr.c b/gcc/expr.c
index 920d47b..764525f 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -134,6 +134,7 @@ static void store_by_pieces_1 (struct store_by_pieces_d *, unsigned int);
 static void store_by_pieces_2 (insn_gen_fn, machine_mode,
 			       struct store_by_pieces_d *);
 static tree clear_storage_libcall_fn (int);
+static void set_storage_via_loop (rtx, rtx, rtx, unsigned int);
 static rtx_insn *compress_float_constant (rtx, rtx);
 static rtx get_subtarget (rtx);
 static void store_constructor_field (rtx, unsigned HOST_WIDE_INT,
@@ -1139,6 +1140,10 @@ emit_block_move_hints (rtx x, rtx y, rtx size, enum block_op_methods method,
   x = adjust_address (x, BLKmode, 0);
   y = adjust_address (y, BLKmode, 0);
 
+  /* memcpy is not guaranteed to be safe for volatile operands.  */
+  may_use_call &= (!MEM_VOLATILE_P (x)
+		   && !MEM_VOLATILE_P (y));
+
   /* Set MEM_SIZE as appropriate for this block copy.  The main place this
      can be incorrect is coming from __builtin_memcpy.  */
   if (CONST_INT_P (size))
@@ -2788,15 +2793,62 @@ clear_storage_hints (rtx object, rtx size, enum block_op_methods method,
 				   expected_align, expected_size,
 				   min_size, max_size, probable_max_size))
     ;
-  else if (ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (object)))
+  else if (ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (object))
+	   && !MEM_VOLATILE_P (object))
     return set_storage_via_libcall (object, size, const0_rtx,
 				    method == BLOCK_OP_TAILCALL);
   else
-    gcc_unreachable ();
+    set_storage_via_loop (object, size, const0_rtx, align);
 
   return NULL;
 }
 
+/* A subroutine of clear_storage.  Set the data via an explicit
+   loop.  This is used only when libcalls are forbidden.  */
+/* ??? It'd be nice to set in hunks larger than QImode.  */
+
+static void
+set_storage_via_loop (rtx object, rtx size, rtx val,
+		      unsigned int align ATTRIBUTE_UNUSED)
+{
+  rtx cmp_label, top_label, iter, object_addr, tmp;
+  enum machine_mode object_addr_mode = get_address_mode (object);
+  enum machine_mode iter_mode;
+
+  iter_mode = GET_MODE (size);
+  if (iter_mode == VOIDmode)
+    iter_mode = word_mode;
+
+  top_label = gen_label_rtx ();
+  cmp_label = gen_label_rtx ();
+  iter = gen_reg_rtx (iter_mode);
+
+  emit_move_insn (iter, const0_rtx);
+
+  object_addr = force_operand (XEXP (object, 0), NULL_RTX);
+  do_pending_stack_adjust ();
+
+  emit_jump (cmp_label);
+  emit_label (top_label);
+
+  tmp = convert_modes (object_addr_mode, iter_mode, iter, true);
+  object_addr = simplify_gen_binary (PLUS, object_addr_mode, object_addr, tmp);
+
+  object = change_address (object, QImode, object_addr);
+
+  emit_move_insn (object, val);
+
+  tmp = expand_simple_binop (iter_mode, PLUS, iter, const1_rtx, iter,
+			     true, OPTAB_LIB_WIDEN);
+  if (tmp != iter)
+    emit_move_insn (iter, tmp);
+
+  emit_label (cmp_label);
+
+  emit_cmp_and_jump_insns (iter, size, LT, NULL_RTX, iter_mode,
+			   true, top_label, REG_BR_PROB_BASE * 90 / 100);
+}
+
 rtx
 clear_storage (rtx object, rtx size, enum block_op_methods method)
 {
diff --git a/gcc/testsuite/gcc.dg/large-volatile-struct-copy-1.c b/gcc/testsuite/gcc.dg/large-volatile-struct-copy-1.c
new file mode 100644
index 0000000..32e4bdf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/large-volatile-struct-copy-1.c
@@ -0,0 +1,37 @@
+/* { dg-do run } */
+/* { dg-options "-O3 --save-temps" } */
+
+#define SIZE 1000
+
+extern void abort (void);
+
+struct foo
+{
+  char data[SIZE];
+};
+
+void __attribute__ ((noinline))
+func (struct foo volatile *x, struct foo volatile *y)
+{
+  *x = *y;
+}
+
+int
+main (int argc, char** argv)
+{
+  /* We just need something to copy, it doesn't much matter what it is.  */
+  volatile struct foo y = { 1, 2, 3 };
+  volatile struct foo x;
+  int i = 0;
+
+  func (&x, &y);
+
+  for (i = 0; i < SIZE; ++i)
+    if (x.data[i] != y.data[i])
+      abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "memcpy" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/gcc.dg/large-volatile-struct-set-1.c b/gcc/testsuite/gcc.dg/large-volatile-struct-set-1.c
new file mode 100644
index 0000000..a41909c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/large-volatile-struct-set-1.c
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O3 --save-temps" } */
+
+#define SIZE 1000
+
+extern void abort (void);
+
+struct foo
+{
+  char data[SIZE];
+};
+
+void __attribute__ ((__noinline__))
+func (struct foo volatile * x)
+{
+  *x = (volatile struct foo) {{0}};
+}
+
+int
+main (int argc, char** argv)
+{
+  volatile struct foo x;
+  int i = 0;
+  func (&x);
+
+  for (i = 0; i < SIZE; ++i)
+    if (x.data[i])
+      abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "memset" } } */
+/* { dg-final { cleanup-saved-temps } } */