Pointer Plus Patch

Paolo Bonzini paolo.bonzini@lu.unisi.ch
Thu Jun 14 09:27:00 GMT 2007


>> > !   /* 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



More information about the Java-patches mailing list