On Mon, Jul 25, 2011 at 7:52 PM, Martin Jambor <mjam...@suse.cz> wrote:
> Hi,
>
> On Thu, Jul 21, 2011 at 11:40:32AM +0200, Martin Jambor wrote:
>> Hi,
>>
>> On Thu, Jul 21, 2011 at 10:34:35AM +0200, Richard Guenther wrote:
>> > On Wed, 20 Jul 2011, Ulrich Weigand wrote:
>> >
>> > > Richard Guenther wrote:
>> > > > On Tue, Jul 19, 2011 at 8:20 PM, Ulrich Weigand <uweig...@de.ibm.com> 
>> > > > wrote:
>> > > > > The problem is that in this expression
>> > > > >   disappear = VIEW_CONVERT_EXPR<struct VecClass>(x_8);
>> > > > > the rhs is considered unaligned and blocks the SRA transformation.
>> > > > >
>> > > > > The check you added for SSA_NAMEs doesn't hit, because the SSA_NAME 
>> > > > > is
>> > > > > encapsulated in a VIEW_CONVERT_EXPR. When get_object_alignment is 
>> > > > > called,
>> > > > > the VIEW_CONVERT_EXPR is stripped off by get_inner_reference and the
>> > > > > SSA_NAME appears, but then get_object_alignment doesn't handle it
>> > > > > and just returns the default alignment of 8 bits.
>> > > > >
>> > > > > Maybe get_object_alignment should itself handle SSA_NAMEs?
>> > > >
>> > > > But what should it return for a rvalue?  There is no "alignment" here.
>> > > > I think SRA should avoid asking for rvalues.
>> > >
>> > > I must admit I do not fully understand what the SRA code is attempting
>> > > to achieve here ...  Could you elaborate on what you mean by "avoid
>> > > asking for rvalues"?  Should the SRA code never check the RHS of an
>> > > assignment for alignment, only the LHS?  Or should it classify the RHS
>> > > tree according to whether the access is rvalue or lvalue (how would
>> > > that work?)?
>> >
>> > Well, it should only ask for stores / loads.  I'm not sure what we'd
>> > want to return as alignment for an rvalue - MAX_ALIGNMENT?  What should
>> > we return for get_object_alignment of an INTEGER_CST for example?
>> >
>>
>> Yeah, we certainly should not be examining alingment of invariants and
>> of conversions of ssa names in.  As far as rvalues in general are
>> concerned, I don't really know which gimple predicate that would be.
>> A comment suggests is_gimple_val but that does not return true for a
>> VIEW_CONVERT_EXPR of an SSA_NAME and would return true for aggregate
>> variables (which perhaps would not be a problem, however they do have
>> an alignment).
>>
>> So at the moment I'd go for stripping all conversions from exp before
>> the first if in tree_non_mode_aligned_mem_p and adding
>> is_gimple_min_invariant to the condition.  Does that make sense?
>
>
> Like this?  Ulrich, can you please verify it works?  I have
> bootstrapped this on x86_64 but there it obvioulsy works and my run of
> compile/testsuite on compile farm sparc64 will take some time (plus,
> the testcase you complained about passes there).
>
> Thanks,
>
> Martin
>
>
> 2011-07-25  Martin Jambor  <mjam...@suse.cz>
>
>        * tree-sra.c (tree_non_mode_aligned_mem_p): Strip conversions and
>        return false for invariants.
>
> Index: src/gcc/tree-sra.c
> ===================================================================
> --- src.orig/gcc/tree-sra.c
> +++ src/gcc/tree-sra.c
> @@ -1075,9 +1075,14 @@ tree_non_mode_aligned_mem_p (tree exp)
>   enum machine_mode mode = TYPE_MODE (TREE_TYPE (exp));
>   unsigned int align;
>
> +  while (CONVERT_EXPR_P (exp)

There can be no convert exprs here, and there can be at most one
VIEW_CONVERT_EXPR.

> +        || TREE_CODE (exp) == VIEW_CONVERT_EXPR)
> +    exp = TREE_OPERAND (exp, 0);
> +
>   if (TREE_CODE (exp) == SSA_NAME
>       || TREE_CODE (exp) == MEM_REF
>       || mode == BLKmode
> +      || is_gimple_min_invariant (exp)
>       || !STRICT_ALIGNMENT)
>     return false;

Otherwise ok.

Thanks,
Richard.

Reply via email to