This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: New SPU failures (Re: [PATCH, PR 49923] Check for misaligned accesses before doing SRA)


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]