| Message ID | 562AF779.10003@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Oct 24, 2015 at 5:14 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/21/2015 12:16 AM, Richard Biener wrote: >> >> On Tue, Oct 20, 2015 at 9:10 PM, Jason Merrill <jason@redhat.com> wrote: >>> >>> I made this change on the delayed folding branch and then noticed that it >>> broke pointer-arith-10.c, which you added to the testsuite. The patch >>> changes the -original dump from >>> >>> return (char *) ((sizetype) p + (sizetype) i); >>> >>> to >>> >>> return (char *) i + (sizetype) p; >>> >>> It's not clear to me why the former should be preferred. Any thoughts? >> >> >> We probably regressed for the former and the dump-scanning just didn't >> notice. We wanted to check for >> >> return p + (sizetype) i; >> >> at least GCC 4.4.7 produces that and 4.5.2 regressed. >> >> Some time ago we had a folding that explicitely swapped pointer-ness >> of an integer op like the testcase was supposed to test. But I remember >> I removed this because it's incorrect (pointer arithmetic is more >> constrained >> than unsigned integer arithmetic): >> >> 2009-01-16 Richard Guenther <rguenther@suse.de> >> >> PR tree-optimization/38835 >> PR middle-end/36227 >> * fold-const.c (fold_binary): Remove PTR + INT -> (INT)(PTR p+ >> INT) >> and INT + PTR -> (INT)(PTR p+ INT) folding. >> * tree-ssa-address.c (create_mem_ref): Properly use >> POINTER_PLUS_EXPR. >> >> so I think the testcase should be simply removed. > > > How about this change to the testcase? Hmm, I'd say we want to test for the correctness issue as well, thus pleas also add /* { dg-final { scan-tree-dump-not "return p +" "original" } } */ as we do not want 'p + (sizetype)i' as that is incorrect. Thanks, Richard. > Jason > >
commit 279a991a06a03148d83fc0c188021c845ad18113
Author: Jason Merrill <jason@redhat.com>
Date: Tue Oct 20 00:39:03 2015 -1000
* c-common.c (pointer_int_sum): Fold the MULT_EXPR.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1c75921..f957018 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -4849,9 +4849,8 @@ pointer_int_sum (location_t loc, enum tree_code resultcode,
for the pointer operation and disregard an overflow that occurred only
because of the sign-extension change in the latter conversion. */
{
- tree t = build_binary_op (loc,
- MULT_EXPR, intop,
- convert (TREE_TYPE (intop), size_exp), 1);
+ tree t = fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (intop), intop,
+ convert (TREE_TYPE (intop), size_exp));
intop = convert (sizetype, t);
if (TREE_OVERFLOW_P (intop) && !TREE_OVERFLOW (t))
intop = wide_int_to_tree (TREE_TYPE (intop), intop);
diff --git a/gcc/testsuite/gcc.dg/pointer-arith-10.c b/gcc/testsuite/gcc.dg/pointer-arith-10.c
index 00e7597..4552e7b 100644
--- a/gcc/testsuite/gcc.dg/pointer-arith-10.c
+++ b/gcc/testsuite/gcc.dg/pointer-arith-10.c
@@ -6,4 +6,7 @@ char *foo(char *p, __UINTPTR_TYPE__ i)
return (char *)i + (__UINTPTR_TYPE__)p;
}
-/* { dg-final { scan-tree-dump "p +" "original" } } */
+/* Check that we use a POINTER_PLUS_EXPR, not something like
+ return (char *) ((sizetype) p + (sizetype) i); */
+
+/* { dg-final { scan-tree-dump-not "sizetype.*sizetype" "original" } } */