diff mbox

Fix PR67053

Message ID alpine.LSU.2.11.1507291622240.19642@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener July 29, 2015, 2:24 p.m. UTC
The following fixes PR67053 by more closely mirror what fold_binary()s
STRIP_NOPS does to avoid the C++ FE constexpr code to regress.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Yes, I'm thinking on an automated way to more closely mirror
STRIP_[SIGN_]NOPS behavior (on toplevel args).

Richard.

2015-07-29  Richard Biener  <rguenther@suse.de>

	PR middle-end/67053
	* match.pd: Allow both operands to independently have conversion
	when simplifying compares of addresses.

Comments

Marc Glisse Aug. 11, 2015, 12:50 a.m. UTC | #1
On Wed, 29 Jul 2015, Richard Biener wrote:

> The following fixes PR67053 by more closely mirror what fold_binary()s
> STRIP_NOPS does to avoid the C++ FE constexpr code to regress.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Yes, I'm thinking on an automated way to more closely mirror
> STRIP_[SIGN_]NOPS behavior (on toplevel args).

As far as I can see, you are not currently checking that these conversions 
are NOPs. I didn't test, but I am afraid this may simplify 
(char)p1==(char)p2 to false a bit too quickly.

> Richard.
>
> 2015-07-29  Richard Biener  <rguenther@suse.de>
>
> 	PR middle-end/67053
> 	* match.pd: Allow both operands to independently have conversion
> 	when simplifying compares of addresses.
>
> Index: gcc/match.pd
> ===================================================================
> --- gcc/match.pd	(revision 226345)
> +++ gcc/match.pd	(working copy)
> @@ -1814,7 +1814,7 @@ (define_operator_list CBRT BUILT_IN_CBRT
>    enough to make fold_stmt not regress when not dispatching to fold_binary.  */
> (for cmp (simple_comparison)
>  (simplify
> -  (cmp (convert?@2 addr@0) (convert? addr@1))
> +  (cmp (convert1?@2 addr@0) (convert2? addr@1))
>   (with
>    {
>      HOST_WIDE_INT off0, off1;
>
Richard Biener Aug. 11, 2015, 7:47 a.m. UTC | #2
On Tue, 11 Aug 2015, Marc Glisse wrote:

> On Wed, 29 Jul 2015, Richard Biener wrote:
> 
> > The following fixes PR67053 by more closely mirror what fold_binary()s
> > STRIP_NOPS does to avoid the C++ FE constexpr code to regress.
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > 
> > Yes, I'm thinking on an automated way to more closely mirror
> > STRIP_[SIGN_]NOPS behavior (on toplevel args).
> 
> As far as I can see, you are not currently checking that these conversions are
> NOPs. I didn't test, but I am afraid this may simplify (char)p1==(char)p2 to
> false a bit too quickly.

I'm relying on our restrictions on conversions of pointers to integers
which requires same-precision (and thus always NOP-style) here.

Richard.

> > Richard.
> > 
> > 2015-07-29  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR middle-end/67053
> > 	* match.pd: Allow both operands to independently have conversion
> > 	when simplifying compares of addresses.
> > 
> > Index: gcc/match.pd
> > ===================================================================
> > --- gcc/match.pd	(revision 226345)
> > +++ gcc/match.pd	(working copy)
> > @@ -1814,7 +1814,7 @@ (define_operator_list CBRT BUILT_IN_CBRT
> >    enough to make fold_stmt not regress when not dispatching to fold_binary.
> > */
> > (for cmp (simple_comparison)
> >  (simplify
> > -  (cmp (convert?@2 addr@0) (convert? addr@1))
> > +  (cmp (convert1?@2 addr@0) (convert2? addr@1))
> >   (with
> >    {
> >      HOST_WIDE_INT off0, off1;
> >
Marc Glisse Aug. 11, 2015, 8:20 a.m. UTC | #3
On Tue, 11 Aug 2015, Richard Biener wrote:

> On Tue, 11 Aug 2015, Marc Glisse wrote:
>
>> On Wed, 29 Jul 2015, Richard Biener wrote:
>>
>>> The following fixes PR67053 by more closely mirror what fold_binary()s
>>> STRIP_NOPS does to avoid the C++ FE constexpr code to regress.
>>>
>>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>>>
>>> Yes, I'm thinking on an automated way to more closely mirror
>>> STRIP_[SIGN_]NOPS behavior (on toplevel args).
>>
>> As far as I can see, you are not currently checking that these conversions are
>> NOPs. I didn't test, but I am afraid this may simplify (char)p1==(char)p2 to
>> false a bit too quickly.
>
> I'm relying on our restrictions on conversions of pointers to integers
> which requires same-precision (and thus always NOP-style) here.

Indeed. I had tested yesterday with an older gcc that we were generating 
the problematic pattern (with conversion from pointer to char), but I 
can't reproduce today, so I guess I had typoed something :-(
Richard Biener Aug. 11, 2015, 8:45 a.m. UTC | #4
On Tue, 11 Aug 2015, Marc Glisse wrote:

> On Tue, 11 Aug 2015, Richard Biener wrote:
> 
> > On Tue, 11 Aug 2015, Marc Glisse wrote:
> > 
> > > On Wed, 29 Jul 2015, Richard Biener wrote:
> > > 
> > > > The following fixes PR67053 by more closely mirror what fold_binary()s
> > > > STRIP_NOPS does to avoid the C++ FE constexpr code to regress.
> > > > 
> > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > 
> > > > Yes, I'm thinking on an automated way to more closely mirror
> > > > STRIP_[SIGN_]NOPS behavior (on toplevel args).
> > > 
> > > As far as I can see, you are not currently checking that these conversions
> > > are
> > > NOPs. I didn't test, but I am afraid this may simplify (char)p1==(char)p2
> > > to
> > > false a bit too quickly.
> > 
> > I'm relying on our restrictions on conversions of pointers to integers
> > which requires same-precision (and thus always NOP-style) here.
> 
> Indeed. I had tested yesterday with an older gcc that we were generating the
> problematic pattern (with conversion from pointer to char), but I can't
> reproduce today, so I guess I had typoed something :-(

Actually I now see some "special" handling of targets like m32c which
are allowed direct pointer to sizetype conversion even if that is a
truncation.  For m32c IIRC pointer types are 24bit and sizetype is
16bit.  So I suppose I have to add that nop-conversion checks anyway :/

Richard.
diff mbox

Patch

Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 226345)
+++ gcc/match.pd	(working copy)
@@ -1814,7 +1814,7 @@  (define_operator_list CBRT BUILT_IN_CBRT
    enough to make fold_stmt not regress when not dispatching to fold_binary.  */
 (for cmp (simple_comparison)
  (simplify
-  (cmp (convert?@2 addr@0) (convert? addr@1))
+  (cmp (convert1?@2 addr@0) (convert2? addr@1))
   (with
    {
      HOST_WIDE_INT off0, off1;