diff mbox

RFC: PATCH to pointer_int_sum vs. pointer-arith-10.c

Message ID 562691A7.7000208@redhat.com
State New
Headers show

Commit Message

Jason Merrill Oct. 20, 2015, 7:10 p.m. UTC
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?

Jason

Comments

Richard Biener Oct. 21, 2015, 10:16 a.m. UTC | #1
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.

Richard.

> Jason
>
diff mbox

Patch

commit 798829561671959e9e49965eb8c1b137b48751e0
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);