On Wed, 10 Aug 2011, Ulrich Weigand wrote:
> Richard Guenther wrote:
> > On Fri, 5 Aug 2011, Martin Jambor wrote:
> > > the patch below fixes PR 49923 by checking for misaligned accesses
> > > before doing IPA-SRA (on strict alignment targets). I have checked it
> > > fixes the issue on compile farm sparc64 and I also included this in a
> > > bootstrap and testsuite run on an x86_64-linux just to double check.
> > >
> > > OK for trunk and the 4.6 branch?
> >
> > Ok for now.
> >
> > I think we need to move this to generic middle-end code and also
> > do something about partly strict-alignment targets such as x86_64.
> > Iff expansion would treat expr as if it had non-natural alignment
> > then when building a MEM_REF replacement with non-BLKmode we have
> > to use a properly aligned variant type for it.
> >
> > I think we can trick FRE/PRE to run into exactly the same situation.
> >
> > I'll put it on my TODO.
>
> This caused new failures on spu-elf:
>
> FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr
> cow_.*D.->red with \\*ISRA"
> FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr
> cow_.*D.->green with ISRA"
> FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr
> calf_.*D.->red with \\*ISRA"
> FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr
> calf_.*D.->green with ISRA"
> FAIL: gcc.dg/ipa/ipa-sra-6.c scan-tree-dump-times eipa_sra "foo " 1
>
> Looking at the last case, tree_non_mode_aligned_mem_p gets called with
> expressions like those:
>
> <component_ref 0xf6f4da40
> type <pointer_type 0xf6f602a0
> type <record_type 0xf6f60180 bovid sizes-gimplified type_0 BLK
> size <integer_cst 0xf6f4d680 constant 96>
> unit size <integer_cst 0xf6f4d6c0 constant 12>
> align 32 symtab 0 alias set 2 canonical type 0xf6f60180 fields
> <field_decl 0xf6f601e0 a> context <translation_unit_decl 0xf6ec10a0 D.2004>
> pointer_to_this <pointer_type 0xf6f602a0> chain <type_decl
> 0xf6ec1030 D.1991>>
> sizes-gimplified public unsigned SI
> size <integer_cst 0xf6e10580 constant 32>
> unit size <integer_cst 0xf6e105a0 constant 4>
> align 32 symtab 0 alias set 5 canonical type 0xf6f602a0>
>
> arg 0 <mem_ref 0xf6f4da60 type <record_type 0xf6f60180 bovid>
>
> arg 0 <ssa_name 0xf6f0ccf0 type <pointer_type 0xf6f602a0>
> visited var <parm_decl 0xf6f70000 cow>def_stmt GIMPLE_NOP
>
> version 3
> ptr-info 0xf6e760d0>
> arg 1 <integer_cst 0xf6f4d7a0 constant 0>
>
> /home/uweigand/fsf/gcc-head/gcc/testsuite/gcc.dg/ipa/ipa-sra-6.c:16:10>
> arg 1 <field_decl 0xf6f60300 next type <pointer_type 0xf6f602a0>
> unsigned SI file
> /home/uweigand/fsf/gcc-head/gcc/testsuite/gcc.dg/ipa/ipa-sra-6.c line 8 col
> 17 size <integer_cst 0xf6e10580 32> unit size <integer_cst 0xf6e105a0 4>
> align 32 offset_align 128
> offset <integer_cst 0xf6e105c0 constant 0>
> bit offset <integer_cst 0xf6e10620 constant 64> context <record_type
> 0xf6f60180 bovid>>
> /home/uweigand/fsf/gcc-head/gcc/testsuite/gcc.dg/ipa/ipa-sra-6.c:16:10>
>
> Now, the component reference as such doesn't introduce any misalignment;
> there are no attribute-aligned in place anywhere.
>
> However, the ptr-info of the "cow" parameter is set up to assume nothing
> about alignment. Therefore, get_object_alignment returns 8, and
> tree_non_mode_aligned_mem_p then of course fails.
>
> I must admin I continue to be confused about exactly what it is that
> tree_non_mode_aligned_mem_p is supposed to be testing for. We have:
>
> if (TREE_CODE (exp) == SSA_NAME
> || TREE_CODE (exp) == MEM_REF
> || mode == BLKmode
> || is_gimple_min_invariant (exp)
> || !STRICT_ALIGNMENT)
> return false;
>
> So if we passed in the plain MEM_REF, this would be considered no problem.
> The COMPONENT_REF does not add any additional misalignment, so one would
> hope that this also shouldn't be a problem.
>
> However, just because there *is* a COMPONENT_REF around it, we suddenly
> realize the fact that don't have points-to information for the MEM_REF
> and therefore consider *it* (and consequently the whole COMPONENT_REF)
> to be potentially misaligned ...
Yep. That's because we are totally confused about alignment :/
Martin, what we need to do is get expands idea of what alignment
it woudl assume for a handled_component_ref and compare that
to what it would say if we re-materialize the mem as a MEM_REF.
Unfortunately there isn't a function that you can use to mimic
expands behavior (that of the normal_inner_ref: case), the closest
would be to look for TYPE_PACKED or TYPE_ALIGN in addition to
what get_object_alignment gives us.
Thus something like
align = get_object_alignment (exp);
if (!TYPE_PACKED (TREE_TYPE (exp))
&& (TREE_CODE (exp) != COMPONENT_REF
|| !DECL_PACKED (TREE_OPERAND (exp, 1))))
align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
if (GET_MODE_ALIGNMENT (mode) > align)
return true;
but really the twisted maze of how we compute whether something
is misaligned during expansion should be cleaned up / factored
somehow, including eventually fixing misaligned indirect refs
for STRICT_ALIGNMENT targets ...
Richard.
