On Fri, 9 Sep 2011, Martin Jambor wrote:
> Hi,
>
> On Wed, Aug 10, 2011 at 04:29:40PM +0200, Richard Guenther wrote:
> > On Wed, 10 Aug 2011, Ulrich Weigand wrote:
> >
>
> ...
>
> > >
> > > 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
>
> This has recvently been reported as PR 50052.
>
> ...
>
> > >
> > > 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;
> >
>
> This fixed the failing testcase ipa-sra-2.c but it creates a
> misaligned access for the testcase below so I changed it to:
>
> align = get_object_alignment (exp);
> if (!TYPE_PACKED (TREE_TYPE (exp)))
> {
> bool encountered_packed = false;
> tree t = exp;
>
> while (TREE_CODE (t) == COMPONENT_REF)
that should be handled_component_p (t) then to catch p->a.b[i].c
with packed a.
But the normal_inner_ref: case doesn't suggest that we need to dive
into handled-components at all ... but it uses DECL_MODE of
an outermost COMPONENT_REF instead of TYPE_MODE. Maybe the easiest
would be to simply call get_inner_reference like the normal_inner_ref
case does to obtain the mode, perform the packed_p check and
decide based on that (supposed that the mode difference is what
makes the new testcase fail). Also look for the predicates that
guard the extract_bit_field call ... (yeah, I know ...)
> if (DECL_PACKED (TREE_OPERAND (t, 1)))
> {
> encountered_packed = true;
> break;
> }
> else
> t = TREE_OPERAND (t, 0);
>
> if (!encountered_packed)
> align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
> }
> if (GET_MODE_ALIGNMENT (mode) > align)
> return true;
>
> I'm currently bootstrapping this on a sparc64 on the compile farm. Is
> the patch OK if it passes (it has passed bootstrap and testing on
> x86_64-linux but that is no surprise)?
>
> Thanks,
>
> Martin
>
>
> 2011-09-08 Martin Jambor <[email protected]>
>
> PR tree-optimization/50052
> * tree-sra.c (tree_non_mode_aligned_mem_p): Look at TYPE_ALIGN if
> there is no packed field decl in exp.
>
> * gcc.dg/tree-ssa/pr49094-2.c: New test.
>
> Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
> @@ -0,0 +1,44 @@
> +/* { dg-do run } */
> +/* { dg-options "-O" } */
> +
> +struct in_addr {
> + unsigned int s_addr;
> +};
> +
> +struct ip {
> + struct in_addr ip_src,ip_dst;
> + unsigned char ip_p;
> + unsigned short ip_sum;
> +};
> +
> +struct ip2 {
> + unsigned char blah;
> + unsigned short sheep;
> + struct ip ip;
> +} __attribute__ ((aligned(1), packed));
> +
> +struct ip2 ip_fw_fwd_addr;
> +
> +int test_alignment( char *m )
> +{
> + struct ip2 *ip = (struct ip2 *) m;
> + struct in_addr pkt_dst;
> + pkt_dst = ip->ip.ip_dst ;
> + if( pkt_dst.s_addr == 0 )
> + return 1;
> + else
> + return 0;
> +}
> +
> +int __attribute__ ((noinline, noclone))
> +intermediary (char *p)
> +{
> + return test_alignment (p);
> +}
> +
> +int
> +main (int argc, char *argv[])
> +{
> + ip_fw_fwd_addr.ip.ip_dst.s_addr = 1;
> + return intermediary ((void *) &ip_fw_fwd_addr);
> +}
> Index: src/gcc/tree-sra.c
> ===================================================================
> --- src.orig/gcc/tree-sra.c
> +++ src/gcc/tree-sra.c
> @@ -1086,6 +1086,23 @@ tree_non_mode_aligned_mem_p (tree exp)
> return false;
>
> align = get_object_alignment (exp);
> + if (!TYPE_PACKED (TREE_TYPE (exp)))
> + {
> + bool encountered_packed = false;
> + tree t = exp;
> +
> + while (TREE_CODE (t) == COMPONENT_REF)
> + if (DECL_PACKED (TREE_OPERAND (t, 1)))
> + {
> + encountered_packed = true;
> + break;
> + }
> + else
> + t = TREE_OPERAND (t, 0);
> +
> + if (!encountered_packed)
> + align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
> + }
> if (GET_MODE_ALIGNMENT (mode) > align)
> return true;
>
>
>
--
Richard Guenther <[email protected]>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
