This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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 } } */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]