Pointer Plus Patch

[Paolo Bonzini]

> +   /* Convert the two values into the same type, this is needed because
> +      sizetype causes sign extension even for unsigned types.  */

comma -> period

> ! 	     are varrying.  */

varying

> !   type1 = type;
> !   if (POINTER_TYPE_P (type))
> !     type1 = sizetype;

Quite a common idiom.  More on this later.

> + 
> +     case POINTER_PLUS_EXPR:
> +       return "p+";

Maybe we can print it as "+" since there's no ambiguity based on the types.

> ! 		build1 (NEGATE_EXPR, sizetype, TYPE_SIZE_UNIT (TREE_TYPE (obj))));

negate_expr (TYPE_SIZE_UNIT (TREE_TYPE (obj)))

> ! 	  /* Create (dst p+ strlen (dst)).  */
> ! 	  newdst = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (dst), dst, newdst);
>   
>   	  newdst = builtin_save_expr (newdst);

I'd move the blank line above the new comment.

> + 	  low = fold_build1 (NEGATE_EXPR, sizetype, low);

negate_expr

> !       diff = fold_binary (MINUS_EXPR, ssizetype, ta1, ta);
> !       if (!diff || !integer_onep (diff))
> !         return NULL_TREE;
> !     }
> !   else
> !     {
> !       diff = fold_binary (MINUS_EXPR, typea, a1, a);
> !       if (!diff || !integer_onep (diff))
> !         return NULL_TREE;
> !     }



The second if is shared among the two branches.

> +       /* INT +p INT -> (PTR)(INT + INT), this happens because of stripping types. */

p+ and turn comma into period.

> + /* The variable a cannot be a local variable as we get better aliasing
> +    now and decide that the store to a is dead.  The better aliasing comes
> +    from better representation of pointer arthimetic. */
> + long a = 10;

Just say "This variable is global to avoid that stores to it be 
optimized away".

Actually, I don't understand this change.  It seems to me that the 
compiler is wrong in optimizing away the store to a, unless this is because

   f((long)(&a)-1,0);

creates a pointer outside the object "a"?  If so, you should modify the 
test to use (long)(&a)+1 and correspondingly *((long *)(a-1).

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

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

> !       cookie_ptr = build1 (NEGATE_EXPR, sizetype, size_in_bytes (sizetype));

negate_expr.

> !       if (code == MINUS_EXPR)
> ! 	offset = build1 (NEGATE_EXPR, sizetype, offset);

Likewise.

> !       exp = build2 (POINTER_PLUS_EXPR, TREE_TYPE (exp), exp,
> ! 		    build1 (NEGATE_EXPR, sizetype,
> ! 			    TYPE_SIZE_UNIT (TREE_TYPE (exp))));

Likewise.

> !       offset = fold_convert (sizetype,
> ! 			     int_const_binop (PLUS_EXPR, offset, offset2, 1));

size_binop?  Actually, since you are at it you could try to simplify the 
code because the last parameter to maybe_fold_stmt_indirect (i.e. 
offset) is always integer_zero_node.

> !       /* Pointer adition is done solely using POINTER_PLUS_EXPR.  */

addition

> ! 				    build_int_cst (sizetype, 2000))) + 1;

size_int (there are several other cases in builtins.c).

> +     case POINTER_PLUS_EXPR: 
> +       /* Even though the sizetype mode and the pointer's mode can be different
> +          expand is able to handle this correct and get the correct result out 
> +          from the PLUS_EXPR code.  */

The comment is useless, the code seems pretty obvious even without it 
(it's not to you because you know the history of the code very well).

> ! 	      = convert (sizetype,
>   			 size_diffop (size_zero_node, gnu_pos));

Use fold_convert instead.

>   	      gnu_ptr = convert (gnu_char_ptr_type, gnu_ptr);

Well, I'd use it here too but that's your call.

> ! 	  return build_binary_op (POINTER_PLUS_EXPR, type, expr,
> ! 				  fold (convert (sizetype, byte_diff)));
>   	}

Just use fold_convert here too, as byte_diff is returned from 
size_diffop here too.

> +       /* FALL THROUGH */

lowercase.

>   	    inc = c_size_in_bytes (TREE_TYPE (result_type));
> + 	    inc = convert (sizetype, inc);

Return value of c_size_in_bytes should already be a sizetype.  Use 
fold_convert if this is not the case.

> ! 	    inc = integer_one_node;
> ! 	    inc = convert (argtype, inc);

I wonder if we should use build_int_cst here too, since convert could 
cause overflow.  Testcase:

   struct x {
     signed x:1;
   };

   void f(struct x *y)
   {
     y->x++;
   }

> +       if (arith_code == MINUS_EXPR)
> + 	rhs = fold_build1 (NEGATE_EXPR, TREE_TYPE (rhs), rhs);

Use negate_expr.

> !           && TREE_ADDRESSABLE (decl)
> ! 		    /* XXX: not sufficient: return-by-value structs! */

Irrelevant change, or move the comment together with the previous one. 
There are other whitespace changes in tree-mudflap.c.

>           ofs = convert (bitsizetype, TREE_OPERAND (t, 2));

Please turn this into fold_convert since you are at it.

>           addr = convert (ptr_type_node, addr);

Likewise.

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

> ! 	/* Reuse a formely created dereference.  */

This hunk just introduces a spelling mistake. :-)

> !       if (POINTER_TYPE_P (TREE_TYPE (init_expr)))
> ! 	ni = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (init_expr), 
> ! 			  init_expr, 
> ! 			  fold_convert (sizetype, 
> ! 					fold_build2 (MULT_EXPR, TREE_TYPE (niters),
> ! 						     niters, step_expr)));
> !       else
> ! 	ni = fold_build2 (PLUS_EXPR, TREE_TYPE (init_expr),
> ! 			  fold_build2 (MULT_EXPR, TREE_TYPE (init_expr),
> ! 				       fold_convert (TREE_TYPE (init_expr),
> ! 						     niters),
> ! 				       step_expr),
> ! 			  init_expr);
> ! 
> ! 

I think I had already commented before that this is best placed in a 
separate function.  This function would also include the "common idiom" 
I pointed out above.

> !   /* FIXME: maybe add a POINTER_MINUS_EXPR ???  */
> !   if (resultcode == MINUS_EXPR)
> !     intop = fold_build1 (NEGATE_EXPR, TREE_TYPE (intop), intop);

negate_expr.

I didn't look at the config/ changes.

Paolo