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: Possible patches for bug 27565


"Joseph S. Myers" <joseph@codesourcery.com> writes:

> Bug 27565 is a 4.1/4.2 regression where use of vectors with SPE enabled 
> yields an ICE in assign_stack_temp_for_type.
> 
>   if (mode == BLKmode)
>     align = BIGGEST_ALIGNMENT;
>   else
>     align = GET_MODE_ALIGNMENT (mode);
> 
>   if (! type)
>     type = lang_hooks.types.type_for_mode (mode, 0);
> 
>   if (type)
>     align = LOCAL_ALIGNMENT (type, align);
> 
> is followed by
> 
>       gcc_assert (mode != BLKmode || align == BIGGEST_ALIGNMENT);
> 
> but LOCAL_ALIGNMENT is defined as
> 
> #define LOCAL_ALIGNMENT(TYPE, ALIGN)                            \
>   ((TARGET_ALTIVEC && TREE_CODE (TYPE) == VECTOR_TYPE) ? 128 :  \
>     (TARGET_E500_DOUBLE && TYPE_MODE (TYPE) == DFmode) ? 64 : \
>     (TARGET_SPE && TREE_CODE (TYPE) == VECTOR_TYPE) ? 64 : ALIGN)
> 
> which reduces the alignment from BIGGEST_ALIGNMENT (128) to 64 so causing 
> an assertion failure.

I don't understand why we check this condition.

In general we assume that the alignment of BLKmode is BITS_PER_UNIT.
There is no other assumption that we can make.  In particular, we
certainly can't assume that a BLKmode value is aligned to
BIGGEST_ALIGNMENT.  So why do we insist on that here?

This patch adjusts the alignment up to BIGGEST_ALIGNMENT for BLKmode:

Thu Feb 18 18:47:09 1999  Jeffrey A Law  (law@cygnus.com)

	* function.c (assign_stack_temp_for_type): Round SIZE before calling
	assign_stack_local for BLKmode slots.

http://gcc.gnu.org/viewcvs/trunk/gcc/function.c?r1=25143&r2=25290&diff_format=h

Unfortunately I can't find the egcs-patches@ or egcs@ e-mail
corresponding to this change.  However, it looks right by itself.

Then it led to this comment:
    http://gcc.gnu.org/ml/gcc-patches/1999-02/msg00639.html
with a patch which eventually led to putting in the assert.

But that requirement doesn't look right to me.  I think Jeff's
original patch did the right thing: in the case where we have an
aligned BLKmode value, we should round up the requested size.  We want
to round the requested size up to BIGGEST_ALIGNMENT because we could
have a screwy BLKmode where the size was larger than the alignment.

An aligned BLKmode value is sufficiently weird that this case isn't
going to arise very often.  But I think the right patch is to change
align to BIGGEST_ALIGNMENT for BLKmode, not to require that it already
be BIGGEST_ALIGNMENT.

I've CC'ed Jeff in case he has any recollection of this.

Ian


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