Re: [PATCH] Mark static const strings as read-only.

[Tom de Vries <Tom_deVries at mentor dot com>]

On 10/10/2011 03:44 PM, Eric Botcazou wrote:
>> 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.
> 

I tried the patch that you describe:
...
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c (revision 179662)
+++ gcc/varasm.c (working copy)
@@ -3119,7 +3119,7 @@ build_constant_desc (tree exp)
   SET_SYMBOL_REF_DECL (symbol, decl);
   TREE_CONSTANT_POOL_ADDRESS_P (symbol) = 1;

-  rtl = gen_rtx_MEM (TYPE_MODE (TREE_TYPE (exp)), symbol);
+  rtl = gen_const_mem (TYPE_MODE (TREE_TYPE (exp)), symbol);
   set_mem_attributes (rtl, exp, 1);
   set_mem_alias_set (rtl, 0);
   set_mem_alias_set (rtl, const_alias_set);
...

The src_mem in expand_builtin_memcpy is generated in get_memory_rtx:
...
static rtx
get_memory_rtx (tree exp, tree len)
{
  tree orig_exp = exp;
  rtx addr, mem;
  HOST_WIDE_INT off;

  /* When EXP is not resolved SAVE_EXPR, MEM_ATTRS can be still derived
     from its expression, for expr->a.b only <variable>.a.b is recorded.  */
  if (TREE_CODE (exp) == SAVE_EXPR && !SAVE_EXPR_RESOLVED_P (exp))
    exp = TREE_OPERAND (exp, 0);

  addr = expand_expr (orig_exp, NULL_RTX, ptr_mode, EXPAND_NORMAL);
  mem = gen_rtx_MEM (BLKmode, memory_address (BLKmode, addr));
...

With the patch for build_constant_desc, a mem with MEM_READONLY_P set (/u) is
generated in expand_expr_constant during expand_expr:
...
(mem/s/u/c:BLK (symbol_ref/f:SI ("*$LC0") [flags 0x2]  <var_decl 0xf7dc7900
*$LC0>) [1 S4 A32])
...

but the expand_expr returns this:
...
(symbol_ref/f:SI ("*$LC0") [flags 0x2]  <var_decl 0xf7dc7900 *$LC0>)
...

The subsequent gen_rtx_MEM generates this:
...
(mem:BLK (lo_sum:SI (reg:SI 195)
        (symbol_ref/f:SI ("*$LC0") [flags 0x2]  <var_decl 0xf7dc7900 *$LC0>)) [0
A8])
...

and after set_mem_attributes it looks like this:
...
(mem/s/c:BLK (lo_sum:SI (reg:SI 195)
        (symbol_ref/f:SI ("*$LC0") [flags 0x2]  <var_decl 0xf7dc7900 *$LC0>)) [0
S4 A32])
...

So, the patch for build_constant_desc does not have the desired effect.

Thanks,
- Tom