Re: Pointer Plus Patch

[Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>]

> > > !   /* Vectorizable, before pointer plus we would get a redundant cast
> > > !      (caused by ponter arithmetics), alias analysis fails to 
> > distinguish
> > > !      between the pointers.  */
> >
> > Who cares.  /* Vectorizable.  */ and that's it.
>
> I wanted to explain why this was changed.

Agreed, but it's a change for the better. :-)

> > > !       /* Since the middle-end checks the type when doing a build2, we
> > > !      need to build the tree in pieces.
> >
> > Not so disgusting, but it would be nicer if you built a
> > non-type-checking code (e.g. MIN_EXPR) and replaced the code, instead of
> > replacing operand 0.
>
> Actually tree codes could have difference sizes (or maybe in the
> future they can) which is why I did it this way.

I don't think this is the case for binary ops.  You can add an assert 
that the length is 2.  This is a call for the C++ maintainers, of course.

> Actually int_const_binop is faster and size_binop could cause an ICE.

Ok, I'll make a note of looking at the last parameter of the function.

> Actually it is not that useless as there really cases where this can
> happen (try using mode with pointers on s390x) and it is not that
> obviously what happens there.

Yes, I understood the problem because of the comment, but most people 
(me included) would not *care* that there is a problem if it works.

> > > !           = convert (sizetype,
> > >                        size_diffop (size_zero_node, gnu_pos));
> >
> > Use fold_convert instead.
>
> This was already using convert.  I did not change this

I know.  But in general it is a bug to use convert.  Same for other 
places in the patch.

> You ask me to remove whitespace changes

Yes, because in GCC comments are *before* the line they refer to, and 
you moved one *after* the line.

> but ask me to change unrelated
> converts to fold_convert.

As you prefer, but since you're touching the code...

> > >     /* If the use of the ADDR_EXPR must be a PLUS_EXPR, or else there
> > >        is nothing to do. */
> > > !   if (TREE_CODE (rhs) != POINTER_PLUS_EXPR)
> > >       return false;
> >
> > If you can, please fix the English in this comment.
> I will change this because I need to change it to be POINTER_PLUS_EXPR 
> anyways:
>  /* If the use of the ADDR_EXPR is not a POINTER_PLUS_EXPR, there
>     is nothing to do. */

Thanks.

Paolo