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: [patch] Fix PR tree-optimization/49771


On Mon, Jul 25, 2011 at 3:22 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Richard Guenther wrote:
>
>> >>>>>> Well, the back-end assumes a pointer to vector type is always
>> >>>>>> naturally aligned, and therefore the data it points to can be
>> >>>>>> accessed via a simple load, with no extra rotate needed.
>> >>>>>
>> >>>>> I can't see any use of VECTOR_TYPE in config/spu/, and assuming
>> >>>>> anything about alignment just because of the kind of the pointer
>> >>>>> is bogus - the scalar code does a scalar read using that pointer.
>> >>>>> So the backend better should look at the memory operation, not
>> >>>>> at the pointer type. ?That said, I can't find any code that looks
>> >>>>> suspicious in the spu backend.
>> >>>>>
>> >>>>>> It seems what happened here is that somehow, a pointer to int
>> >>>>>> gets replaced by a pointer to vector, even though their alignment
>> >>>>>> properties are different.
>> >>>>>
>> >>>>> No, they are not. ?They get replaced if they are value-equivalent
>> >>>>> in which case they are also alignment-equivalent. ?But well, the
>> >>>>> dump snippet wasn't complete and I don't feel like building a
>> >>>>> SPU cross to verify myself.
>
>> > Seems perfectly valid to me. ?Or well - I suppose we might run into
>> > the issue that the vectorizer sets alignment data at the wrong spot?
>> > You can check alignment info when dumping with the -alias flag.
>> > Building a spu cross now.
>>
>> Nope, all perfectly valid.
>
> Ah, I guess I see what's happening here. ?When the SPU back-end is called
> to expand the load, the source operand is passed as:
>
> (mem:SI (reg/f:SI 226 [ vect_pa.9 ])
> ? ? ? ?[2 MEM[base: vect_pa.9_44, offset: 0B]+0 S4 A32])
>
> Now this does say the MEM is only guaranteed to be aligned to 32 bits.
>
> However, spu_expand_load then goes and looks at the components of the
> address in detail, in order to figure out how to best perform the access.
> In doing so, it looks at the REGNO_POINTER_ALIGN values of the base
> registers involved in the address.
>
> In this case, REGNO_POINTER_ALIGN (226) is set to 128, and therefore
> the back-end thinks it can use an aligned access after all.
>
> Now, the reason why REGNO_POINTER_ALIGN (226) is 128 is that the register
> is the DECL_RTL for the variable vect_pa.9, and that variable has a
> pointer-to-vector type (with target alignment 128).
>
> When expanding that variable, expand_one_register_var does:
>
> ?if (POINTER_TYPE_P (type))
> ? ?mark_reg_pointer (x, TYPE_ALIGN (TREE_TYPE (type)));
>
> All this is normally completely correct -- a variable of type pointer
> to vector type *must* hold only properly aligned values.

No, this is indeed completely bogus code ;)  it should instead
use get_pointer_alignment.

Richard.


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