On Mon, Jul 25, 2011 at 3:22 PM, Ulrich Weigand <uweig...@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.
