This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Pointer Plus Patch
- From: Paolo Bonzini <bonzini at gnu dot org>
- To: Andrew Pinski <pinskia at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, java-patches at gcc dot gnu dot org, Fortran List <fortran at gcc dot gnu dot org>
- Date: Wed, 13 Jun 2007 12:11:57 +0200
- Subject: Re: Pointer Plus Patch
- References: <de8d50360706130147x75373c0eie94f95eb7964769a@mail.gmail.com>
+ /* 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