On Mon, Jan 2, 2012 at 8:00 PM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
> Eric Botcazou <ebotca...@adacore.com> writes:
>>> You are basically (but non-optimally) locally re-implementing
>>> what expr.c:get_object_or_type_alignment does, for the
>>> bare MEM_REF case (you don't consider offsets that
>>> do not change the alignment, nor alignment information
>>> present on the SSA name).
>>
>> Adjusted version attached, regression-tested on SPARC/Solaris. I'll do a
>> full
>> testing cycle if it is OK for mainline.
>>
>> get_object_or_type_alignment doesn't exist on the 4.6 branch, the expanders
>> for
>> MEM_REF and TARGET_MEM_REF do:
>>
>> MAX (TYPE_ALIGN (TREE_TYPE (exp)),
>> get_object_alignment (exp, BIGGEST_ALIGNMENT)))
>>
>> instead, so I presume I should do the same for them in
>> tree_non_aligned_mem_p?
>>
>>
>> 2011-12-07 Eric Botcazou <ebotca...@adacore.com>
>>
>> PR tree-optimization/51315
>> * tree.h (get_object_or_type_alignment): Declare.
>> * expr.c (get_object_or_type_alignment): Move to...
>> * builtins.c (get_object_or_type_alignment): ...here. Add assertion.
>> * tree-sra.c (tree_non_mode_aligned_mem_p): Rename to...
>> (tree_non_aligned_mem_p): ...this. Add ALIGN parameter. Look into
>> MEM_REFs and use get_object_or_type_alignment for them.
>> (build_accesses_from_assign): Adjust for above change.
>> (access_precludes_ipa_sra_p): Likewise.
>
> Sorry for the late notice, but this regressed memcpy-1.c for n32 and n64
> on mips64-linux-gnu. I realise memcpy-1.c isn't viewed as being a proper
> SRA test, but in this case I think it really is showing a genuine problem.
> We have:
>
> struct a {int a,b,c;} a;
> int test(struct a a)
> {
> struct a nasty_local;
> __builtin_memcpy (&nasty_local,&a, sizeof(a));
> return nasty_local.a;
> }
>
> We apply LOCAL_ALIGNMENT to nasty_local during estimated_stack_frame_size,
> so we have a VAR_DECL (nasty_local) with 64-bit alignment and a PARM_DECL
> (a) with 32-bit alignment. This fails the condition:
>
> if (STRICT_ALIGNMENT
> && tree_non_aligned_mem_p (rhs, get_object_alignment (lhs)))
> lacc->grp_unscalarizable_region = 1;
>
> because LHS has 64-bit alignment but RHS has 32-bit alignment.
Shouldn't we go back and unconditionally return false if lhs and (or?) rhs
are BLKmode? I suppose they are, in this case. Or should we truncate
the alignment from get_object_alignment to TYPE_ALIGN if it is bigger
than that to avoid this situation?
I suppose Eric will know which case the expander would handle correctly.
Richard.
> Richard
