diff mbox series

Simplify ptr - 0

Message ID alpine.DEB.2.02.1711221809380.11945@stedding.saclay.inria.fr
State New
Headers show
Series Simplify ptr - 0 | expand

Commit Message

Marc Glisse Nov. 22, 2017, 5:34 p.m. UTC
Hello,

I hadn't implemented this simplification because I think it is invalid 
code, but apparently people do write it, so we might as well handle it 
sensibly. This also happens to work around PR 83104 (already fixed).

bootstrap+regtest on powerpc64le-unknown-linux-gnu.

2017-11-22  Marc Glisse  <marc.glisse@inria.fr>

 	* match.pd (ptr-0): New transformation.

Comments

Jakub Jelinek Nov. 22, 2017, 5:43 p.m. UTC | #1
On Wed, Nov 22, 2017 at 06:34:08PM +0100, Marc Glisse wrote:
> Hello,
> 
> I hadn't implemented this simplification because I think it is invalid code,
> but apparently people do write it, so we might as well handle it sensibly.
> This also happens to work around PR 83104 (already fixed).
> 
> bootstrap+regtest on powerpc64le-unknown-linux-gnu.

I guess we'll need to disable it for -fsanitize=pointer-subtraction
once/if it makes it in, because it should be still diagnosed as invalid.

> 2017-11-22  Marc Glisse  <marc.glisse@inria.fr>
> 
> 	* match.pd (ptr-0): New transformation.
> 
> -- 
> Marc Glisse

	Jakub
Marc Glisse Nov. 22, 2017, 7:23 p.m. UTC | #2
On Wed, 22 Nov 2017, Jakub Jelinek wrote:

> On Wed, Nov 22, 2017 at 06:34:08PM +0100, Marc Glisse wrote:
>> Hello,
>>
>> I hadn't implemented this simplification because I think it is invalid code,
>> but apparently people do write it, so we might as well handle it sensibly.
>> This also happens to work around PR 83104 (already fixed).
>>
>> bootstrap+regtest on powerpc64le-unknown-linux-gnu.
>
> I guess we'll need to disable it for -fsanitize=pointer-subtraction
> once/if it makes it in, because it should be still diagnosed as invalid.

I am not sure you'll need to disable it. From what I've seen, the patch 
inserts a call to BUILT_IN_ASAN_POINTER_SUBTRACT before the 
POINTER_DIFF_EXPR is created, and I don't see why simplifying 
POINTER_DIFF_EXPR would affect that call. But then again, I don't know how 
the sanitizers work, so I can easily misinterpret things.
Richard Biener Nov. 23, 2017, 12:39 p.m. UTC | #3
On Wed, Nov 22, 2017 at 6:34 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> I hadn't implemented this simplification because I think it is invalid code,
> but apparently people do write it, so we might as well handle it sensibly.
> This also happens to work around PR 83104 (already fixed).
>
> bootstrap+regtest on powerpc64le-unknown-linux-gnu.

Ok.

What about 0 - ptr?  (ok, that's even more weird)

Richard.

> 2017-11-22  Marc Glisse  <marc.glisse@inria.fr>
>
>         * match.pd (ptr-0): New transformation.
>
> --
> Marc Glisse
Marc Glisse Nov. 23, 2017, 1:25 p.m. UTC | #4
On Thu, 23 Nov 2017, Richard Biener wrote:

> On Wed, Nov 22, 2017 at 6:34 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> I hadn't implemented this simplification because I think it is invalid code,
>> but apparently people do write it, so we might as well handle it sensibly.
>> This also happens to work around PR 83104 (already fixed).
>>
>> bootstrap+regtest on powerpc64le-unknown-linux-gnu.
>
> Ok.
>
> What about 0 - ptr?  (ok, that's even more weird)

I thought about it, but it seemed less likely (I can imagine the idiom 
ptr-0 being used to let the compiler guess the right type). I'll add 0-ptr 
if you think that may be useful.
Marc Glisse Nov. 24, 2017, 9:18 a.m. UTC | #5
On Thu, 23 Nov 2017, Richard Biener wrote:

> What about 0 - ptr?  (ok, that's even more weird)

 	* match.pd (0-ptr): New transformation.

Regtested on gcc112.
Richard Biener Nov. 24, 2017, 12:04 p.m. UTC | #6
On Fri, Nov 24, 2017 at 10:18 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Thu, 23 Nov 2017, Richard Biener wrote:
>
>> What about 0 - ptr?  (ok, that's even more weird)
>
>
>         * match.pd (0-ptr): New transformation.
>
> Regtested on gcc112.

Ok.

Thanks,
Richard.

> --
> Marc Glisse
> Index: gcc/match.pd
> ===================================================================
> --- gcc/match.pd        (revision 255118)
> +++ gcc/match.pd        (working copy)
> @@ -2449,23 +2449,27 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>      (minus @0 { tem; })))))
>
>  /* Convert x+x into x*2.  */
>  (simplify
>   (plus @0 @0)
>   (if (SCALAR_FLOAT_TYPE_P (type))
>    (mult @0 { build_real (type, dconst2); })
>    (if (INTEGRAL_TYPE_P (type))
>     (mult @0 { build_int_cst (type, 2); }))))
>
> +/* 0 - X  ->  -X.  */
>  (simplify
>   (minus integer_zerop @1)
>   (negate @1))
> +(simplify
> + (pointer_diff integer_zerop @1)
> + (negate (convert @1)))
>
>  /* (ARG0 - ARG1) is the same as (-ARG1 + ARG0).  So check whether
>     ARG0 is zero and X + ARG0 reduces to X, since that would mean
>     (-ARG1 + ARG0) reduces to -ARG1.  */
>  (simplify
>   (minus real_zerop@0 @1)
>   (if (fold_real_zero_addition_p (type, @0, 0))
>    (negate @1)))
>
>  /* Transform x * -1 into -x.  */
>
diff mbox series

Patch

Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 255052)
+++ gcc/match.pd	(working copy)
@@ -95,20 +95,25 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (for op (plus pointer_plus minus bit_ior bit_xor)
   (simplify
     (op @0 integer_zerop)
     (non_lvalue @0)))
 
 /* 0 +p index -> (type)index */
 (simplify
  (pointer_plus integer_zerop @1)
  (non_lvalue (convert @1)))
 
+/* ptr - 0 -> (type)ptr */
+(simplify
+ (pointer_diff @0 integer_zerop)
+ (convert @0))
+
 /* See if ARG1 is zero and X + ARG1 reduces to X.
    Likewise if the operands are reversed.  */
 (simplify
  (plus:c @0 real_zerop@1)
  (if (fold_real_zero_addition_p (type, @1, 0))
   (non_lvalue @0)))
 
 /* See if ARG1 is zero and X - ARG1 reduces to X.  */
 (simplify
  (minus @0 real_zerop@1)