Mark static const strings as read-only.

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:
...
...

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

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

OK, too bad that we need to play this back-and-forth game with MEMs.  So the 
original patch is OK (with TREE_READONLY (base) on the next line to mimic what 
is done just above and without the gcc/ prefix in the ChangeLog).  If you have 
some available cycles, you can test and install the build_constant_desc change 
in the same commit, otherwise I'll do it myself.

On 10/10/2011 05:50 PM, Eric Botcazou wrote:
>> So, the patch for build_constant_desc does not have the desired effect.
> 
> OK, too bad that we need to play this back-and-forth game with MEMs.  So the 
> original patch is OK (with TREE_READONLY (base) on the next line to mimic what 
> is done just above and without the gcc/ prefix in the ChangeLog).  If you have 
> some available cycles, you can test and install the build_constant_desc change 
> in the same commit, otherwise I'll do it myself.
> 

I'll include the build_constant_desc change in a bootstrap/reg-test on x86_64.

Thanks,
- Tom