diff mbox

[PR70920] transform (intptr_t) x eq/ne CST to x eq/ne (typeof x) cst

Message ID CAAgBjMmz2tWj4WL2sKtKJtZfY2OjN+xpRGf7QNOFyhTXkw6h1g@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni July 29, 2016, 2:32 p.m. UTC
On 29 July 2016 at 12:42, Richard Biener <rguenther@suse.de> wrote:
> On Fri, 29 Jul 2016, Prathamesh Kulkarni wrote:
>
>> On 28 July 2016 at 19:18, Richard Biener <rguenther@suse.de> wrote:
>> > On Thu, 28 Jul 2016, Prathamesh Kulkarni wrote:
>> >
>> >> On 28 July 2016 at 15:58, Andreas Schwab <schwab@suse.de> wrote:
>> >> > On Mo, Jul 25 2016, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote:
>> >> >
>> >> >> diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c b/gcc/testsuite/gcc.dg/pr70920-4.c
>> >> >> new file mode 100644
>> >> >> index 0000000..dedb895
>> >> >> --- /dev/null
>> >> >> +++ b/gcc/testsuite/gcc.dg/pr70920-4.c
>> >> >> @@ -0,0 +1,21 @@
>> >> >> +/* { dg-do compile } */
>> >> >> +/* { dg-options "-O2 -fdump-tree-ccp-details -Wno-int-to-pointer-cast" } */
>> >> >> +
>> >> >> +#include <stdint.h>
>> >> >> +
>> >> >> +void f1();
>> >> >> +void f2();
>> >> >> +
>> >> >> +void
>> >> >> +foo (int a)
>> >> >> +{
>> >> >> +  void *cst = 0;
>> >> >> +  if ((int *) a == cst)
>> >> >> +    {
>> >> >> +      f1 ();
>> >> >> +      if (a)
>> >> >> +     f2 ();
>> >> >> +    }
>> >> >> +}
>> >> >> +
>> >> >> +/* { dg-final { scan-tree-dump "gimple_simplified to if \\(_\[0-9\]* == 0\\)" "ccp1" } } */
>> >> >
>> >> > This fails on all ilp32 platforms.
>> >> Oops, sorry for the breakage.
>> >> With -m32, the pattern is applied during forwprop1 rather than ccp1.
>> >> I wonder though why ccp1 fails to fold the pattern with -m32 ?
>> >> Looking at the dumps:
>> >>
>> >> without -m32:
>> >> input to ccp1 pass:
>> >>   <bb 2>:
>> >>   cst_4 = 0B;
>> >>   _1 = (long int) a_5(D);
>> >>   _2 = (void *) _1;
>> >>   if (cst_4 == _2)
>> >>     goto <bb 3>;
>> >>   else
>> >>     goto <bb 5>;
>> >>
>> >> cc1 pass dump shows:
>> >> Substituting values and folding statements
>> >>
>> >> Folding statement: _1 = (long int) a_5(D);
>> >> Not folded
>> >> Folding statement: _2 = (void *) _1;
>> >> Not folded
>> >> Folding statement: if (cst_4 == _2)
>> >> which is likely CONSTANT
>> >> Applying pattern match.pd:2537, gimple-match.c:6530
>> >> gimple_simplified to if (_1 == 0)
>> >> Folded into: if (_1 == 0)
>> >>
>> >> with -m32:
>> >> input to ccp1 pass:
>> >>  <bb 2>:
>> >>   cst_3 = 0B;
>> >>   a.0_1 = (void *) a_4(D);
>> >>   if (cst_3 == a.0_1)
>> >>     goto <bb 3>;
>> >>   else
>> >>     goto <bb 5>;
>> >>
>> >> ccp1 pass dump shows:
>> >> Substituting values and folding statements
>> >>
>> >> Folding statement: a.0_1 = (void *) a_4(D);
>> >> Not folded
>> >> Folding statement: if (cst_3 == a.0_1)
>> >> which is likely CONSTANT
>> >> Folded into: if (a.0_1 == 0B)
>> >>
>> >> I am not able to understand why it doesn't fold it to
>> >> if (a_4(D) == 0) ?
>> >> forwprop1 folds a.0_1 == 0B to a_4(D) == 0.
>> >
>> > It's because CCP folds with follow-single-use edges but the
>> > match-and-simplify code uses a single callback to valueize and
>> > decide whether its valid to follow the SSA edge.  I did have some
>> > old patches trying to fix that but never followed up on those.
>> Thanks for the explanation.
>> >
>> >> I suppose the test-case would need to scan ccp1 for non-ilp targets
>> >> and forwprop1 for
>> >> ilp targets. How do update the test-case to reflect this ?
>> >
>> > It's simpler to verify that at some point (forwprop) we have the
>> > expected IL rather than testing for the match debug prints.
>> In forwprop dump,
>> For m32, we have if (a_4(D) == 0)
>> and without m32: if (_1 == 0)
>> So need to match either a default def or anonymous name
>> in the test-case, which I am having a bit of trouble writing regex for.
>> In the patch i simply chose to match "== 0\\)", not sure if that's a good idea.
>> Also how do I update the test-case so that it gets tested twice, once with -m32
>> and once without ?
>
> I don't think just matching == 0 is a good idea.  I suggest to
> restrict the testcase to lp64 targets and maybe add a ilp32 variant.
Hi,
I restricted the test-case to lp64 targets.
Is this OK to commit ?

Thanks,
Prathamesh
>
> Richard.
>
>> Thanks,
>> Prathamesh
>> >
>> > Richard.
>> >
>> >> Thanks,
>> >> Prathamesh
>> >> >
>> >> > Andreas.
>> >> >
>> >> > --
>> >> > Andreas Schwab, SUSE Labs, schwab@suse.de
>> >> > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
>> >> > "And now for something completely different."
>> >>
>> >>
>> >
>> > --
>> > Richard Biener <rguenther@suse.de>
>> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
2016-07-29  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

testsuite/
	gcc.dg/pr70920-4.c: Restrict to lp64 targets and make scan-tree-dump
	to scan forwprop1 dump pass.

Comments

Richard Biener July 29, 2016, 3:15 p.m. UTC | #1
On July 29, 2016 4:32:40 PM GMT+02:00, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote:
>On 29 July 2016 at 12:42, Richard Biener <rguenther@suse.de> wrote:
>> On Fri, 29 Jul 2016, Prathamesh Kulkarni wrote:
>>
>>> On 28 July 2016 at 19:18, Richard Biener <rguenther@suse.de> wrote:
>>> > On Thu, 28 Jul 2016, Prathamesh Kulkarni wrote:
>>> >
>>> >> On 28 July 2016 at 15:58, Andreas Schwab <schwab@suse.de> wrote:
>>> >> > On Mo, Jul 25 2016, Prathamesh Kulkarni
><prathamesh.kulkarni@linaro.org> wrote:
>>> >> >
>>> >> >> diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c
>b/gcc/testsuite/gcc.dg/pr70920-4.c
>>> >> >> new file mode 100644
>>> >> >> index 0000000..dedb895
>>> >> >> --- /dev/null
>>> >> >> +++ b/gcc/testsuite/gcc.dg/pr70920-4.c
>>> >> >> @@ -0,0 +1,21 @@
>>> >> >> +/* { dg-do compile } */
>>> >> >> +/* { dg-options "-O2 -fdump-tree-ccp-details
>-Wno-int-to-pointer-cast" } */
>>> >> >> +
>>> >> >> +#include <stdint.h>
>>> >> >> +
>>> >> >> +void f1();
>>> >> >> +void f2();
>>> >> >> +
>>> >> >> +void
>>> >> >> +foo (int a)
>>> >> >> +{
>>> >> >> +  void *cst = 0;
>>> >> >> +  if ((int *) a == cst)
>>> >> >> +    {
>>> >> >> +      f1 ();
>>> >> >> +      if (a)
>>> >> >> +     f2 ();
>>> >> >> +    }
>>> >> >> +}
>>> >> >> +
>>> >> >> +/* { dg-final { scan-tree-dump "gimple_simplified to if
>\\(_\[0-9\]* == 0\\)" "ccp1" } } */
>>> >> >
>>> >> > This fails on all ilp32 platforms.
>>> >> Oops, sorry for the breakage.
>>> >> With -m32, the pattern is applied during forwprop1 rather than
>ccp1.
>>> >> I wonder though why ccp1 fails to fold the pattern with -m32 ?
>>> >> Looking at the dumps:
>>> >>
>>> >> without -m32:
>>> >> input to ccp1 pass:
>>> >>   <bb 2>:
>>> >>   cst_4 = 0B;
>>> >>   _1 = (long int) a_5(D);
>>> >>   _2 = (void *) _1;
>>> >>   if (cst_4 == _2)
>>> >>     goto <bb 3>;
>>> >>   else
>>> >>     goto <bb 5>;
>>> >>
>>> >> cc1 pass dump shows:
>>> >> Substituting values and folding statements
>>> >>
>>> >> Folding statement: _1 = (long int) a_5(D);
>>> >> Not folded
>>> >> Folding statement: _2 = (void *) _1;
>>> >> Not folded
>>> >> Folding statement: if (cst_4 == _2)
>>> >> which is likely CONSTANT
>>> >> Applying pattern match.pd:2537, gimple-match.c:6530
>>> >> gimple_simplified to if (_1 == 0)
>>> >> Folded into: if (_1 == 0)
>>> >>
>>> >> with -m32:
>>> >> input to ccp1 pass:
>>> >>  <bb 2>:
>>> >>   cst_3 = 0B;
>>> >>   a.0_1 = (void *) a_4(D);
>>> >>   if (cst_3 == a.0_1)
>>> >>     goto <bb 3>;
>>> >>   else
>>> >>     goto <bb 5>;
>>> >>
>>> >> ccp1 pass dump shows:
>>> >> Substituting values and folding statements
>>> >>
>>> >> Folding statement: a.0_1 = (void *) a_4(D);
>>> >> Not folded
>>> >> Folding statement: if (cst_3 == a.0_1)
>>> >> which is likely CONSTANT
>>> >> Folded into: if (a.0_1 == 0B)
>>> >>
>>> >> I am not able to understand why it doesn't fold it to
>>> >> if (a_4(D) == 0) ?
>>> >> forwprop1 folds a.0_1 == 0B to a_4(D) == 0.
>>> >
>>> > It's because CCP folds with follow-single-use edges but the
>>> > match-and-simplify code uses a single callback to valueize and
>>> > decide whether its valid to follow the SSA edge.  I did have some
>>> > old patches trying to fix that but never followed up on those.
>>> Thanks for the explanation.
>>> >
>>> >> I suppose the test-case would need to scan ccp1 for non-ilp
>targets
>>> >> and forwprop1 for
>>> >> ilp targets. How do update the test-case to reflect this ?
>>> >
>>> > It's simpler to verify that at some point (forwprop) we have the
>>> > expected IL rather than testing for the match debug prints.
>>> In forwprop dump,
>>> For m32, we have if (a_4(D) == 0)
>>> and without m32: if (_1 == 0)
>>> So need to match either a default def or anonymous name
>>> in the test-case, which I am having a bit of trouble writing regex
>for.
>>> In the patch i simply chose to match "== 0\\)", not sure if that's a
>good idea.
>>> Also how do I update the test-case so that it gets tested twice,
>once with -m32
>>> and once without ?
>>
>> I don't think just matching == 0 is a good idea.  I suggest to
>> restrict the testcase to lp64 targets and maybe add a ilp32 variant.
>Hi,
>I restricted the test-case to lp64 targets.
>Is this OK to commit ?

OK

Thanks,
Richard.

>Thanks,
>Prathamesh
>>
>> Richard.
>>
>>> Thanks,
>>> Prathamesh
>>> >
>>> > Richard.
>>> >
>>> >> Thanks,
>>> >> Prathamesh
>>> >> >
>>> >> > Andreas.
>>> >> >
>>> >> > --
>>> >> > Andreas Schwab, SUSE Labs, schwab@suse.de
>>> >> > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3
>0EEA B9D7
>>> >> > "And now for something completely different."
>>> >>
>>> >>
>>> >
>>> > --
>>> > Richard Biener <rguenther@suse.de>
>>> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham
>Norton, HRB 21284 (AG Nuernberg)
>>>
>>
>> --
>> Richard Biener <rguenther@suse.de>
>> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham
>Norton, HRB 21284 (AG Nuernberg)
Matthew Wahab Aug. 3, 2016, 11:57 a.m. UTC | #2
On 29/07/16 15:32, Prathamesh Kulkarni wrote:
> On 29 July 2016 at 12:42, Richard Biener <rguenther@suse.de> wrote:
>> On Fri, 29 Jul 2016, Prathamesh Kulkarni wrote:
>>
>>> On 28 July 2016 at 19:18, Richard Biener <rguenther@suse.de> wrote:
>>>> On Thu, 28 Jul 2016, Prathamesh Kulkarni wrote:
>>>>
>>>>> On 28 July 2016 at 15:58, Andreas Schwab <schwab@suse.de> wrote:
>>>>>> On Mo, Jul 25 2016, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote:
>>>>>>
>>>>>>> diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c b/gcc/testsuite/gcc.dg/pr70920-4.c
>>>>>>> new file mode 100644
>>>>>>> index 0000000..dedb895
>>>>>>> --- /dev/null
>>>>>>> +++ b/gcc/testsuite/gcc.dg/pr70920-4.c
>>>>>>> @@ -0,0 +1,21 @@
>>>>>>> +/* { dg-do compile } */
>>>>>>> +/* { dg-options "-O2 -fdump-tree-ccp-details -Wno-int-to-pointer-cast" } */
>>>>>>> +
>>>>>>> +#include <stdint.h>
>>>>>>> +
>>>>>>> +void f1();
>>>>>>> +void f2();
>>>>>>> +
>>>>>>> +void
>>>>>>> +foo (int a)
>>>>>>> +{
>>>>>>> +  void *cst = 0;
>>>>>>> +  if ((int *) a == cst)
>>>>>>> +    {
>>>>>>> +      f1 ();
>>>>>>> +      if (a)
>>>>>>> +     f2 ();
>>>>>>> +    }
>>>>>>> +}
>>>>>>> +
>>>>>>> +/* { dg-final { scan-tree-dump "gimple_simplified to if \\(_\[0-9\]* == 0\\)" "ccp1" } } */
>>>>>>
>>>>>> This fails on all ilp32 platforms.
[..]
>>
>> I don't think just matching == 0 is a good idea.  I suggest to
>> restrict the testcase to lp64 targets and maybe add a ilp32 variant.
> Hi,
> I restricted the test-case to lp64 targets.
> Is this OK to commit ?

Hello,

The test case is failing for arm-none-linux-gnueabihf.

It is correctly skipped if the 'dg-require-effective-target lp64' you added is 
moved to the end of the directives (after the dg-options).

Matthew
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c b/gcc/testsuite/gcc.dg/pr70920-4.c
index dedb895..ab2748b 100644
--- a/gcc/testsuite/gcc.dg/pr70920-4.c
+++ b/gcc/testsuite/gcc.dg/pr70920-4.c
@@ -1,5 +1,6 @@ 
+/* { dg-require-effective-target lp64 } */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-ccp-details -Wno-int-to-pointer-cast" } */
+/* { dg-options "-O2 -fdump-tree-forwprop-details -Wno-int-to-pointer-cast" } */
 
 #include <stdint.h>
 
@@ -18,4 +19,4 @@  foo (int a)
     }
 }
 
-/* { dg-final { scan-tree-dump "gimple_simplified to if \\(_\[0-9\]* == 0\\)" "ccp1" } } */
+/* { dg-final { scan-tree-dump "if \\(_\[0-9\]* == 0\\)" "forwprop1" } } */