Re: [patch] Fix PR tree-optimization/49771

["Ulrich Weigand" <uweigand@de.ibm.com>]

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.

I guess the vectorizer deliberatly loads a (potentially) unaligned
value into a vector pointer variable.  It then generates a check
whether the value is really aligned; and uses it only if so.

But if that pointer variable "escapes" into the other branch because
DOM thinks it can re-use the value, the REGNO_POINTER_ALIGN value
carried for its DECL_RTL register is now incorrect ...

Maybe the vectorizer ought to declare that variable with a non-default
type alignment setting?  Or else, perform the assignment to the
variable only *inside* the "if" that checks for correct alignment?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com