[PATCH] Use sufficient alignment when expanding VCE (PR target/35366)

[PATCH] Use sufficient alignment when expanding VCE (PR target/35366)

[Jakub Jelinek]

Hi!

This is another patch for PR35366, either of the patches fixes the failure,
but IMHO we want both.  If for whatever reason VIEW_CONVERT_EXPR <double, STRING_CST>
(or some other VCE) isn't folded, and the TREE_TYPE of the VCE needs bigger
alignment than the inner operand, the inner constant is emitted with a small
alignment, but it is actually accessed as if the alignment was big enough
for the outer type.  The following patch makes sure an inner constant
is sufficiently aligned.

Ok for trunk?

2008-11-11  Jakub Jelinek  <jakub@redhat.com>

	PR target/35366
	* expr.c (expand_expr_addr_expr_1): If EXP needs bigger alignment
	than INNER and INNER is a constant, forcibly align INNER as much
	as needed.

--- gcc/expr.c.jj	2008-10-29 23:16:25.000000000 +0100
+++ gcc/expr.c	2008-11-11 14:24:56.000000000 +0100
@@ -6862,6 +6862,16 @@ expand_expr_addr_expr_1 (tree exp, rtx t
   gcc_assert (inner != exp);
 
   subtarget = offset || bitpos ? NULL_RTX : target;
+  /* For VIEW_CONVERT_EXPR, where the outer alignment is bigger than
+     inner alignment, force the inner to be sufficiently aligned.  */
+  if (CONSTANT_CLASS_P (inner)
+      && TYPE_ALIGN (TREE_TYPE (inner)) < TYPE_ALIGN (TREE_TYPE (exp)))
+    {
+      inner = copy_node (inner);
+      TREE_TYPE (inner) = copy_node (TREE_TYPE (inner));
+      TYPE_ALIGN (TREE_TYPE (inner)) = TYPE_ALIGN (TREE_TYPE (exp));
+      TYPE_USER_ALIGN (TREE_TYPE (inner)) = 1;
+    }
   result = expand_expr_addr_expr_1 (inner, subtarget, tmode, modifier);
 
   if (offset)

	Jakub

Re: [PATCH] Use sufficient alignment when expanding VCE (PR target/35366)

[Richard Guenther]

On Tue, Nov 11, 2008 at 7:30 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> This is another patch for PR35366, either of the patches fixes the failure,
> but IMHO we want both.  If for whatever reason VIEW_CONVERT_EXPR <double, STRING_CST>
> (or some other VCE) isn't folded, and the TREE_TYPE of the VCE needs bigger
> alignment than the inner operand, the inner constant is emitted with a small
> alignment, but it is actually accessed as if the alignment was big enough
> for the outer type.  The following patch makes sure an inner constant
> is sufficiently aligned.
>
> Ok for trunk?

Is it always possible to force the alignment of the inner object?  I think
the same problem may exist for V_C_E <double> (*p) when we cannot
influence the alignment of *p.  So shouldn't we make sure to have the
correct MEM_ALIGN instead?  Or does it work in that case already?

Richard.

> 2008-11-11  Jakub Jelinek  <jakub@redhat.com>
>
>        PR target/35366
>        * expr.c (expand_expr_addr_expr_1): If EXP needs bigger alignment
>        than INNER and INNER is a constant, forcibly align INNER as much
>        as needed.
>
> --- gcc/expr.c.jj       2008-10-29 23:16:25.000000000 +0100
> +++ gcc/expr.c  2008-11-11 14:24:56.000000000 +0100
> @@ -6862,6 +6862,16 @@ expand_expr_addr_expr_1 (tree exp, rtx t
>   gcc_assert (inner != exp);
>
>   subtarget = offset || bitpos ? NULL_RTX : target;
> +  /* For VIEW_CONVERT_EXPR, where the outer alignment is bigger than
> +     inner alignment, force the inner to be sufficiently aligned.  */
> +  if (CONSTANT_CLASS_P (inner)
> +      && TYPE_ALIGN (TREE_TYPE (inner)) < TYPE_ALIGN (TREE_TYPE (exp)))
> +    {
> +      inner = copy_node (inner);
> +      TREE_TYPE (inner) = copy_node (TREE_TYPE (inner));
> +      TYPE_ALIGN (TREE_TYPE (inner)) = TYPE_ALIGN (TREE_TYPE (exp));
> +      TYPE_USER_ALIGN (TREE_TYPE (inner)) = 1;
> +    }
>   result = expand_expr_addr_expr_1 (inner, subtarget, tmode, modifier);
>
>   if (offset)
>
>        Jakub
>

Re: [PATCH] Use sufficient alignment when expanding VCE (PR target/35366)

[Jakub Jelinek]

On Tue, Nov 11, 2008 at 09:40:16AM -0600, Richard Guenther wrote:
> On Tue, Nov 11, 2008 at 7:30 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > This is another patch for PR35366, either of the patches fixes the failure,
> > but IMHO we want both.  If for whatever reason VIEW_CONVERT_EXPR <double, STRING_CST>
> > (or some other VCE) isn't folded, and the TREE_TYPE of the VCE needs bigger
> > alignment than the inner operand, the inner constant is emitted with a small
> > alignment, but it is actually accessed as if the alignment was big enough
> > for the outer type.  The following patch makes sure an inner constant
> > is sufficiently aligned.
> >
> > Ok for trunk?
> 
> Is it always possible to force the alignment of the inner object?  I think

For constants yes (that's why I use CONSTANT_CLASS_P), otherwise no.

> the same problem may exist for V_C_E <double> (*p) when we cannot
> influence the alignment of *p.  So shouldn't we make sure to have the
> correct MEM_ALIGN instead?  Or does it work in that case already?

If you have an unaligned pointer and V_C_E <double> (*p), then
MEM_ALIGN will be correct I believe (at least, without this patch
it is A8 for LC2, matching its alignment).  Of course whether it
works or not depends on whether the target requires strict alignment
or not, but I'd say if it does and the pointer isn't aligned, it was
a programmer's bug.
V_C_E of a constant is something that is generated by the compiler itself
and is something we can align sufficiently, so we IMHO should.

	Jakub

Re: [PATCH] Use sufficient alignment when expanding VCE (PR target/35366)

[Richard Guenther]

On Tue, Nov 11, 2008 at 12:40 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Nov 11, 2008 at 09:40:16AM -0600, Richard Guenther wrote:
>> On Tue, Nov 11, 2008 at 7:30 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > This is another patch for PR35366, either of the patches fixes the failure,
>> > but IMHO we want both.  If for whatever reason VIEW_CONVERT_EXPR <double, STRING_CST>
>> > (or some other VCE) isn't folded, and the TREE_TYPE of the VCE needs bigger
>> > alignment than the inner operand, the inner constant is emitted with a small
>> > alignment, but it is actually accessed as if the alignment was big enough
>> > for the outer type.  The following patch makes sure an inner constant
>> > is sufficiently aligned.
>> >
>> > Ok for trunk?
>>
>> Is it always possible to force the alignment of the inner object?  I think
>
> For constants yes (that's why I use CONSTANT_CLASS_P), otherwise no.
>
>> the same problem may exist for V_C_E <double> (*p) when we cannot
>> influence the alignment of *p.  So shouldn't we make sure to have the
>> correct MEM_ALIGN instead?  Or does it work in that case already?
>
> If you have an unaligned pointer and V_C_E <double> (*p), then
> MEM_ALIGN will be correct I believe (at least, without this patch
> it is A8 for LC2, matching its alignment).  Of course whether it
> works or not depends on whether the target requires strict alignment
> or not, but I'd say if it does and the pointer isn't aligned, it was
> a programmer's bug.
> V_C_E of a constant is something that is generated by the compiler itself
> and is something we can align sufficiently, so we IMHO should.

Ok.  The patch is ok then.

Thanks,
Richard.


>        Jakub
>

Re: [PATCH] Use sufficient alignment when expanding VCE (PR target/35366)

[Geert Bosch]

On Nov 11, 2008, at 10:40, Richard Guenther wrote:
> Is it always possible to force the alignment of the inner object?  I  
> think
> the same problem may exist for V_C_E <double> (*p) when we cannot
> influence the alignment of *p.  So shouldn't we make sure to have the
> correct MEM_ALIGN instead?  Or does it work in that case already?

V_C_E is about reinterpreting values, not memory objects. So, in the
case where *p has a lesser alignment than a double, V_C_E would load
*p using instructions suitable for the type of *p, like two 32-bit
word loads.

   -Geert