diff mbox

Mark static const strings as read-only.

Message ID 4E92B854.5070208@mentor.com
State New
Headers show

Commit Message

Tom de Vries Oct. 10, 2011, 9:18 a.m. UTC 
Eric,

without this patch, the mips code for the memcpy test-case in the patch looks
like this. The loadbytes (lbu) and the storebytes (sb) are considered to alias,
so the scheduler cannot rearrange them in a more optimal order.
...
	lui	$2,%hi($LC0)
	lbu	$3,%lo($LC0)($2)
	addiu	$2,$2,%lo($LC0)
	sb	$3,0($4)
	lbu	$3,1($2)
	nop
	sb	$3,1($4)
	lbu	$2,2($2)
	j	$31
	sb	$2,2($4)
...

With the patch, the loadbytes and the storebytes are considered not to alias
because the static const string src argument is marked as read-only, and the
scheduler rearranges them.
...
	lui	$3,%hi($LC0)
	addiu	$2,$3,%lo($LC0)
	lbu	$5,%lo($LC0)($3)
	lbu	$3,1($2)
	lbu	$2,2($2)
	sb	$5,0($4)
	sb	$3,1($4)
	j	$31
	sb	$2,2($4)
...

The patch makes sure that the src_mem computed here in expand_builtin_memcpy is
marked as MEM_READONLY_P:
...
      src_mem = get_memory_rtx (src, len);
...

build and reg-tested on i686, arm, and mips.

OK for trunk?

Thanks,
- Tom

2011-10-10  Tom de Vries  <tom@codesourcery.com>

	* gcc/emit-rtl.c (set_mem_attributes_minus_bitpos): Set MEM_READONLY_P
	for static const strings.

	* gcc/testsuite/gcc.dg/memcpy-3.c: New test.

Comments

Eric Botcazou Oct. 10, 2011, 1:44 p.m. UTC | #1 
> The patch makes sure that the src_mem computed here in
> expand_builtin_memcpy is marked as MEM_READONLY_P:
> ...
>       src_mem = get_memory_rtx (src, len);
> ...
>
> build and reg-tested on i686, arm, and mips.
>
> OK for trunk?

Do you get the same result by calling gen_const_mem from build_constant_desc 
instead of gen_rtx_MEM?  If so, this would be better I think.
diff mbox

Patch

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c (revision 179662)
+++ gcc/emit-rtl.c (working copy)
@@ -1696,6 +1696,11 @@  set_mem_attributes_minus_bitpos (rtx ref
 	  && !TREE_THIS_VOLATILE (base))
 	MEM_READONLY_P (ref) = 1;
 
+      /* Mark static const strings readonly as well.  */
+      if (base && TREE_CODE (base) == STRING_CST && TREE_READONLY (base)
+	  && TREE_STATIC (base))
+	MEM_READONLY_P (ref) = 1;
+
       /* If this expression uses it's parent's alias set, mark it such
 	 that we won't change it.  */
       if (component_uses_parent_alias_set (t))
Index: gcc/testsuite/gcc.dg/memcpy-4.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/memcpy-4.c (revision 0)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-rtl-expand" } */
+
+void
+f1 (char *p)
+{
+  __builtin_memcpy (p, "123", 3);
+}
+
+/* { dg-final { scan-rtl-dump-times "mem/s/u" 3 "expand" { target mips*-*-* } } } */
+/* { dg-final { cleanup-rtl-dump "expand" } } */