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

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

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

Hi Richard,
The attached patch tries to fix PR70920.
It adds your pattern from comment 1 in the PR
(with additional gating on INTEGRAL_TYPE_P to avoid regressing finalize_18.f90)
and second pattern, which is reverse of the first transform.
I needed to update ssa-dom-branch-1.c because with patch applied,
jump threading removed the second if (i != 0B) block.
The dumps with and without patch for ssa-dom-branch-1.c start
to differ with forwprop1:

before:
 <bb 3>:
  _1 = temp_16(D)->code;
  _2 = _1 == 42;
  _3 = (int) _2;
  _4 = (long int) _3;
  temp_17 = (struct rtx_def *) _4;
  if (temp_17 != 0B)
    goto <bb 4>;
  else
    goto <bb 8>;

after:
<bb 3>:
  _1 = temp_16(D)->code;
  _2 = _1 == 42;
  _3 = (int) _2;
  _4 = (long int) _2;
  temp_17 = (struct rtx_def *) _4;
  if (_1 == 42)
    goto <bb 4>;
  else
    goto <bb 8>;

I suppose the transform is correct for above test-case ?

Then vrp dump shows:
 Threaded jump 5 --> 9 to 13
  Threaded jump 8 --> 9 to 13
  Threaded jump 3 --> 9 to 13
  Threaded jump 12 --> 9 to 14
Removing basic block 9
basic block 9, loop depth 0
 pred:
if (i1_10(D) != 0B)
  goto <bb 10>;
else
  goto <bb 11>;
 succ:       10
             11

So there remained two instances of if (i1_10 (D) != 0B) in dom2 dump file,
and hence needed to update the test-case.

Bootstrapped and tested on x86_64-unknown-linux-gnu.
OK to commit ?

PS: Writing changelog entries for match.pd is a bit tedious.
Should we add optional names for pattern so we can refer to them by names
in the ChangeLog for the more complicated ones ?
Or maybe just use comments:
(simplify /* name */ ... ) -;)

Thanks,
Prathamesh

[-- Attachment #2: patch-1.diff --]
[-- Type: text/plain, Size: 3737 bytes --]

diff --git a/gcc/match.pd b/gcc/match.pd
index 21bf617..7c736be 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3408,3 +3408,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	 { CONSTRUCTOR_ELT (ctor, idx / k)->value; })
 	(BIT_FIELD_REF { CONSTRUCTOR_ELT (ctor, idx / k)->value; }
 		       @1 { bitsize_int ((idx % k) * width); })))))))))
+
+/* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST.  */
+
+(for cmp (ne eq)
+ (simplify
+  (cmp (convert@2 @0) INTEGER_CST@1)
+  (if (POINTER_TYPE_P (TREE_TYPE (@0))
+       && INTEGRAL_TYPE_P (TREE_TYPE (@2)))
+   (cmp @0 (convert @1)))))
+
+/* Reverse of the above case:
+   x has integral_type, CST is a pointer constant.
+   Transform (typeof CST)x eq/ne CST to x eq/ne (typeof x) CST.  */
+
+(for cmp (ne eq)
+ (simplify
+  (cmp (convert @0) @1)
+  (if (POINTER_TYPE_P (TREE_TYPE (@1))
+       && INTEGRAL_TYPE_P (TREE_TYPE (@0)))
+    (cmp @0 (convert @1)))))
diff --git a/gcc/testsuite/gcc.dg/pr70920-1.c b/gcc/testsuite/gcc.dg/pr70920-1.c
new file mode 100644
index 0000000..9b7e2d0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70920-1.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-gimple" } */
+
+#include <stdint.h>
+
+void f1();
+void f2();
+
+void
+foo (int *a)
+{
+  if ((intptr_t) a == 0)
+    {
+      f1 ();
+      if (a)
+	f2 (); 
+    }
+}
+
+/* { dg-final { scan-tree-dump "if \\(a == 0B\\)" "gimple" } } */
diff --git a/gcc/testsuite/gcc.dg/pr70920-2.c b/gcc/testsuite/gcc.dg/pr70920-2.c
new file mode 100644
index 0000000..2db9897
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70920-2.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-forwprop-details" } */
+
+#include <stdint.h>
+
+void f1();
+void f2();
+
+void
+foo (int *a)
+{
+  int cst = 0;
+  if ((intptr_t) a == cst)
+    {
+      f1 ();
+      if (a) 
+	f2 (); 
+    }
+}
+
+/* { dg-final { scan-tree-dump "gimple_simplified to if \\(a_\[0-9\]*\\(D\\) == 0B\\)" "forwprop1" } } */
diff --git a/gcc/testsuite/gcc.dg/pr70920-3.c b/gcc/testsuite/gcc.dg/pr70920-3.c
new file mode 100644
index 0000000..71e0d8d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70920-3.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Wno-int-to-pointer-cast -fdump-tree-gimple" } */
+
+#include <stdint.h>
+
+void f1();
+void f2();
+
+void
+foo (int a)
+{
+  if ((int *) a == 0)
+    {
+      f1 ();
+      if (a)
+	f2 (); 
+    }
+}
+
+/* { dg-final { scan-tree-dump "if \\(a == 0\\)" "gimple" } } */
diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c b/gcc/testsuite/gcc.dg/pr70920-4.c
new file mode 100644
index 0000000..f92c5a6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70920-4.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-forwprop-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 \\(a_\[0-9\]*\\(D\\) == 0\\)" "forwprop1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c
index 18f9041..d38e3a8 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c
@@ -21,7 +21,7 @@ try_combine (rtx i1, rtx newpat)
 
 /* There should be three tests against i1.  Two from the hash table
    dumps, one in the code itself.  */
-/* { dg-final { scan-tree-dump-times "if .i1_" 3 "dom2"} } */
+/* { dg-final { scan-tree-dump-times "if .i1_" 2 "dom2"} } */
 
 /* There should be no actual jump threads realized by DOM.  The
    legitimize jump threads are handled in VRP and those discovered

[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 499 bytes --]

2016-07-25  Richard Biener  <rguenther@suse.de>
	    Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>

	* match.pd ((intptr)x eq/ne CST to x eq/ne (typeof x) CST): New
	pattern.
	((typeof ptr_cst) x eq/ne ptr_cst to x eq/ne (typeof x) ptr_cst): New
	pattern.

testsuite/
	* gcc.dg/pr70920-1.c: New test-case.
	* gcc.dg/pr70902-2.c: Likewise.
	* gcc.dg/pr70920-3.c: Likewise.
	* gcc.dg/pr70920-4.c: Likewise
	* gcc.dg/tree-ssa/ssa-dom-branch-1.c: Change scan-tree-dump-times to
	2 instead of 3. 

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

[Richard Biener]

On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:

> Hi Richard,
> The attached patch tries to fix PR70920.
> It adds your pattern from comment 1 in the PR
> (with additional gating on INTEGRAL_TYPE_P to avoid regressing finalize_18.f90)
> and second pattern, which is reverse of the first transform.
> I needed to update ssa-dom-branch-1.c because with patch applied,
> jump threading removed the second if (i != 0B) block.
> The dumps with and without patch for ssa-dom-branch-1.c start
> to differ with forwprop1:
> 
> before:
>  <bb 3>:
>   _1 = temp_16(D)->code;
>   _2 = _1 == 42;
>   _3 = (int) _2;
>   _4 = (long int) _3;
>   temp_17 = (struct rtx_def *) _4;
>   if (temp_17 != 0B)
>     goto <bb 4>;
>   else
>     goto <bb 8>;
> 
> after:
> <bb 3>:
>   _1 = temp_16(D)->code;
>   _2 = _1 == 42;
>   _3 = (int) _2;
>   _4 = (long int) _2;
>   temp_17 = (struct rtx_def *) _4;
>   if (_1 == 42)
>     goto <bb 4>;
>   else
>     goto <bb 8>;
> 
> I suppose the transform is correct for above test-case ?
> 
> Then vrp dump shows:
>  Threaded jump 5 --> 9 to 13
>   Threaded jump 8 --> 9 to 13
>   Threaded jump 3 --> 9 to 13
>   Threaded jump 12 --> 9 to 14
> Removing basic block 9
> basic block 9, loop depth 0
>  pred:
> if (i1_10(D) != 0B)
>   goto <bb 10>;
> else
>   goto <bb 11>;
>  succ:       10
>              11
> 
> So there remained two instances of if (i1_10 (D) != 0B) in dom2 dump file,
> and hence needed to update the test-case.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> OK to commit ?

--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3408,3 +3408,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
         { CONSTRUCTOR_ELT (ctor, idx / k)->value; })
        (BIT_FIELD_REF { CONSTRUCTOR_ELT (ctor, idx / k)->value; }
                       @1 { bitsize_int ((idx % k) * width); })))))))))
+
+/* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST.  
*/
+
+(for cmp (ne eq)
+ (simplify
+  (cmp (convert@2 @0) INTEGER_CST@1)
+  (if (POINTER_TYPE_P (TREE_TYPE (@0))
+       && INTEGRAL_TYPE_P (TREE_TYPE (@2)))

you can use @1 here and omit @2.

+   (cmp @0 (convert @1)))))
+
+/* Reverse of the above case:
+   x has integral_type, CST is a pointer constant.
+   Transform (typeof CST)x eq/ne CST to x eq/ne (typeof x) CST.  */
+
+(for cmp (ne eq)
+ (simplify
+  (cmp (convert @0) @1)
+  (if (POINTER_TYPE_P (TREE_TYPE (@1))
+       && INTEGRAL_TYPE_P (TREE_TYPE (@0)))
+    (cmp @0 (convert @1)))))

The second pattern lacks the INTEGER_CST on @1 so it doesn't match
its comment.  Please do not add vertical space between pattern
comment and pattern.

Please place patterns not at the end of match.pd but where similar
transforms are done.  Like after

/* Simplify pointer equality compares using PTA.  */
(for neeq (ne eq)
 (simplify
  (neeq @0 @1)
  (if (POINTER_TYPE_P (TREE_TYPE (@0))
       && ptrs_compare_unequal (@0, @1))
   { neeq == EQ_EXPR ? boolean_false_node : boolean_true_node; })))

please also share the (for ...) for both patterns or merge them
by changing the condition to

  (if ((POINTER_TYPE_P (TREE_TYPE (@0))
        && INTEGRAL_TYPE_P (TREE_TYPE (@1)))
       || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
           && POINTER_TYPE_P (TREE_TYPE (@1))))

Richard.

> PS: Writing changelog entries for match.pd is a bit tedious.
> Should we add optional names for pattern so we can refer to them by names
> in the ChangeLog for the more complicated ones ?
> Or maybe just use comments:
> (simplify /* name */ ... ) -;)

That will add the fun of inventing names ;)

> Thanks,
> Prathamesh
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

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

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 4168 bytes --]

On 25 July 2016 at 14:32, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
>
>> Hi Richard,
>> The attached patch tries to fix PR70920.
>> It adds your pattern from comment 1 in the PR
>> (with additional gating on INTEGRAL_TYPE_P to avoid regressing finalize_18.f90)
>> and second pattern, which is reverse of the first transform.
>> I needed to update ssa-dom-branch-1.c because with patch applied,
>> jump threading removed the second if (i != 0B) block.
>> The dumps with and without patch for ssa-dom-branch-1.c start
>> to differ with forwprop1:
>>
>> before:
>>  <bb 3>:
>>   _1 = temp_16(D)->code;
>>   _2 = _1 == 42;
>>   _3 = (int) _2;
>>   _4 = (long int) _3;
>>   temp_17 = (struct rtx_def *) _4;
>>   if (temp_17 != 0B)
>>     goto <bb 4>;
>>   else
>>     goto <bb 8>;
>>
>> after:
>> <bb 3>:
>>   _1 = temp_16(D)->code;
>>   _2 = _1 == 42;
>>   _3 = (int) _2;
>>   _4 = (long int) _2;
>>   temp_17 = (struct rtx_def *) _4;
>>   if (_1 == 42)
>>     goto <bb 4>;
>>   else
>>     goto <bb 8>;
>>
>> I suppose the transform is correct for above test-case ?
>>
>> Then vrp dump shows:
>>  Threaded jump 5 --> 9 to 13
>>   Threaded jump 8 --> 9 to 13
>>   Threaded jump 3 --> 9 to 13
>>   Threaded jump 12 --> 9 to 14
>> Removing basic block 9
>> basic block 9, loop depth 0
>>  pred:
>> if (i1_10(D) != 0B)
>>   goto <bb 10>;
>> else
>>   goto <bb 11>;
>>  succ:       10
>>              11
>>
>> So there remained two instances of if (i1_10 (D) != 0B) in dom2 dump file,
>> and hence needed to update the test-case.
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> OK to commit ?
>
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3408,3 +3408,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>          { CONSTRUCTOR_ELT (ctor, idx / k)->value; })
>         (BIT_FIELD_REF { CONSTRUCTOR_ELT (ctor, idx / k)->value; }
>                        @1 { bitsize_int ((idx % k) * width); })))))))))
> +
> +/* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST.
> */
> +
> +(for cmp (ne eq)
> + (simplify
> +  (cmp (convert@2 @0) INTEGER_CST@1)
> +  (if (POINTER_TYPE_P (TREE_TYPE (@0))
> +       && INTEGRAL_TYPE_P (TREE_TYPE (@2)))
>
> you can use @1 here and omit @2.
>
> +   (cmp @0 (convert @1)))))
> +
> +/* Reverse of the above case:
> +   x has integral_type, CST is a pointer constant.
> +   Transform (typeof CST)x eq/ne CST to x eq/ne (typeof x) CST.  */
> +
> +(for cmp (ne eq)
> + (simplify
> +  (cmp (convert @0) @1)
> +  (if (POINTER_TYPE_P (TREE_TYPE (@1))
> +       && INTEGRAL_TYPE_P (TREE_TYPE (@0)))
> +    (cmp @0 (convert @1)))))
>
> The second pattern lacks the INTEGER_CST on @1 so it doesn't match
> its comment.  Please do not add vertical space between pattern
> comment and pattern.
>
> Please place patterns not at the end of match.pd but where similar
> transforms are done.  Like after
>
> /* Simplify pointer equality compares using PTA.  */
> (for neeq (ne eq)
>  (simplify
>   (neeq @0 @1)
>   (if (POINTER_TYPE_P (TREE_TYPE (@0))
>        && ptrs_compare_unequal (@0, @1))
>    { neeq == EQ_EXPR ? boolean_false_node : boolean_true_node; })))
>
> please also share the (for ...) for both patterns or merge them
> by changing the condition to
>
>   (if ((POINTER_TYPE_P (TREE_TYPE (@0))
>         && INTEGRAL_TYPE_P (TREE_TYPE (@1)))
>        || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
>            && POINTER_TYPE_P (TREE_TYPE (@1))))
>
Hi,
Done suggested changes in this version.
pr70920-4.c (test-case in patch) is now folded during  ccp instead of
forwprop after merging the
two patterns.
Passes bootstrap+test on x86_64-unknown-linux-gnu.
OK for trunk ?

Thanks,
Prathamesh
> Richard.
>
>> PS: Writing changelog entries for match.pd is a bit tedious.
>> Should we add optional names for pattern so we can refer to them by names
>> in the ChangeLog for the more complicated ones ?
>> Or maybe just use comments:
>> (simplify /* name */ ... ) -;)
>
> That will add the fun of inventing names ;)
>
>> Thanks,
>> Prathamesh
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

[-- Attachment #2: patch-2.diff --]
[-- Type: text/plain, Size: 3626 bytes --]

diff --git a/gcc/match.pd b/gcc/match.pd
index 21bf617..6c2ec82 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -2513,6 +2513,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        && ptrs_compare_unequal (@0, @1))
    { neeq == EQ_EXPR ? boolean_false_node : boolean_true_node; })))
 
+/* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST.
+   and (typeof ptr_cst) x eq/ne ptr_cst to x eq/ne (typeof x) CST */
+(for cmp (ne eq)
+ (simplify
+  (cmp (convert @0) INTEGER_CST@1)
+  (if ((POINTER_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P (TREE_TYPE (@1)))
+        || (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && POINTER_TYPE_P (TREE_TYPE (@1))))
+   (cmp @0 (convert @1)))))
+
 /* Non-equality compare simplifications from fold_binary  */
 (for cmp (lt gt le ge)
  /* Comparisons with the highest or lowest possible integer of
diff --git a/gcc/testsuite/gcc.dg/pr70920-1.c b/gcc/testsuite/gcc.dg/pr70920-1.c
new file mode 100644
index 0000000..9b7e2d0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70920-1.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-gimple" } */
+
+#include <stdint.h>
+
+void f1();
+void f2();
+
+void
+foo (int *a)
+{
+  if ((intptr_t) a == 0)
+    {
+      f1 ();
+      if (a)
+	f2 (); 
+    }
+}
+
+/* { dg-final { scan-tree-dump "if \\(a == 0B\\)" "gimple" } } */
diff --git a/gcc/testsuite/gcc.dg/pr70920-2.c b/gcc/testsuite/gcc.dg/pr70920-2.c
new file mode 100644
index 0000000..2db9897
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70920-2.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-forwprop-details" } */
+
+#include <stdint.h>
+
+void f1();
+void f2();
+
+void
+foo (int *a)
+{
+  int cst = 0;
+  if ((intptr_t) a == cst)
+    {
+      f1 ();
+      if (a) 
+	f2 (); 
+    }
+}
+
+/* { dg-final { scan-tree-dump "gimple_simplified to if \\(a_\[0-9\]*\\(D\\) == 0B\\)" "forwprop1" } } */
diff --git a/gcc/testsuite/gcc.dg/pr70920-3.c b/gcc/testsuite/gcc.dg/pr70920-3.c
new file mode 100644
index 0000000..8b24cbc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70920-3.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-gimple -Wno-int-to-pointer-cast" } */
+
+#include <stdint.h>
+
+void f1();
+void f2();
+
+void
+foo (int a)
+{
+  if ((int *) a == 0)
+    {
+      f1 ();
+      if (a)
+	f2 (); 
+    }
+}
+
+/* { dg-final { scan-tree-dump "if \\(a == 0\\)" "gimple" } } */
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" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c
index 18f9041..d38e3a8 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c
@@ -21,7 +21,7 @@ try_combine (rtx i1, rtx newpat)
 
 /* There should be three tests against i1.  Two from the hash table
    dumps, one in the code itself.  */
-/* { dg-final { scan-tree-dump-times "if .i1_" 3 "dom2"} } */
+/* { dg-final { scan-tree-dump-times "if .i1_" 2 "dom2"} } */
 
 /* There should be no actual jump threads realized by DOM.  The
    legitimize jump threads are handled in VRP and those discovered

[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 520 bytes --]

2016-07-25  Richard Biener  <rguenther@suse.de>
	    Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>

	PR middle-end/70920
	* match.pd ((intptr)x eq/ne CST to x eq/ne (typeof x) CST): New
	pattern.
	((typeof ptr_cst) x eq/ne ptr_cst to x eq/ne (typeof x) ptr_cst): New
	pattern.

testsuite/
	* gcc.dg/pr70920-1.c: New test-case.
	* gcc.dg/pr70902-2.c: Likewise.
	* gcc.dg/pr70920-3.c: Likewise.
	* gcc.dg/pr70920-4.c: Likewise
	* gcc.dg/tree-ssa/ssa-dom-branch-1.c: Change scan-tree-dump-times to
	2 instead of 3. 

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

[Richard Biener]

On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:

> On 25 July 2016 at 14:32, Richard Biener <rguenther@suse.de> wrote:
> > On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
> >
> >> Hi Richard,
> >> The attached patch tries to fix PR70920.
> >> It adds your pattern from comment 1 in the PR
> >> (with additional gating on INTEGRAL_TYPE_P to avoid regressing finalize_18.f90)
> >> and second pattern, which is reverse of the first transform.
> >> I needed to update ssa-dom-branch-1.c because with patch applied,
> >> jump threading removed the second if (i != 0B) block.
> >> The dumps with and without patch for ssa-dom-branch-1.c start
> >> to differ with forwprop1:
> >>
> >> before:
> >>  <bb 3>:
> >>   _1 = temp_16(D)->code;
> >>   _2 = _1 == 42;
> >>   _3 = (int) _2;
> >>   _4 = (long int) _3;
> >>   temp_17 = (struct rtx_def *) _4;
> >>   if (temp_17 != 0B)
> >>     goto <bb 4>;
> >>   else
> >>     goto <bb 8>;
> >>
> >> after:
> >> <bb 3>:
> >>   _1 = temp_16(D)->code;
> >>   _2 = _1 == 42;
> >>   _3 = (int) _2;
> >>   _4 = (long int) _2;
> >>   temp_17 = (struct rtx_def *) _4;
> >>   if (_1 == 42)
> >>     goto <bb 4>;
> >>   else
> >>     goto <bb 8>;
> >>
> >> I suppose the transform is correct for above test-case ?
> >>
> >> Then vrp dump shows:
> >>  Threaded jump 5 --> 9 to 13
> >>   Threaded jump 8 --> 9 to 13
> >>   Threaded jump 3 --> 9 to 13
> >>   Threaded jump 12 --> 9 to 14
> >> Removing basic block 9
> >> basic block 9, loop depth 0
> >>  pred:
> >> if (i1_10(D) != 0B)
> >>   goto <bb 10>;
> >> else
> >>   goto <bb 11>;
> >>  succ:       10
> >>              11
> >>
> >> So there remained two instances of if (i1_10 (D) != 0B) in dom2 dump file,
> >> and hence needed to update the test-case.
> >>
> >> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> OK to commit ?
> >
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -3408,3 +3408,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >          { CONSTRUCTOR_ELT (ctor, idx / k)->value; })
> >         (BIT_FIELD_REF { CONSTRUCTOR_ELT (ctor, idx / k)->value; }
> >                        @1 { bitsize_int ((idx % k) * width); })))))))))
> > +
> > +/* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST.
> > */
> > +
> > +(for cmp (ne eq)
> > + (simplify
> > +  (cmp (convert@2 @0) INTEGER_CST@1)
> > +  (if (POINTER_TYPE_P (TREE_TYPE (@0))
> > +       && INTEGRAL_TYPE_P (TREE_TYPE (@2)))
> >
> > you can use @1 here and omit @2.
> >
> > +   (cmp @0 (convert @1)))))
> > +
> > +/* Reverse of the above case:
> > +   x has integral_type, CST is a pointer constant.
> > +   Transform (typeof CST)x eq/ne CST to x eq/ne (typeof x) CST.  */
> > +
> > +(for cmp (ne eq)
> > + (simplify
> > +  (cmp (convert @0) @1)
> > +  (if (POINTER_TYPE_P (TREE_TYPE (@1))
> > +       && INTEGRAL_TYPE_P (TREE_TYPE (@0)))
> > +    (cmp @0 (convert @1)))))
> >
> > The second pattern lacks the INTEGER_CST on @1 so it doesn't match
> > its comment.  Please do not add vertical space between pattern
> > comment and pattern.
> >
> > Please place patterns not at the end of match.pd but where similar
> > transforms are done.  Like after
> >
> > /* Simplify pointer equality compares using PTA.  */
> > (for neeq (ne eq)
> >  (simplify
> >   (neeq @0 @1)
> >   (if (POINTER_TYPE_P (TREE_TYPE (@0))
> >        && ptrs_compare_unequal (@0, @1))
> >    { neeq == EQ_EXPR ? boolean_false_node : boolean_true_node; })))
> >
> > please also share the (for ...) for both patterns or merge them
> > by changing the condition to
> >
> >   (if ((POINTER_TYPE_P (TREE_TYPE (@0))
> >         && INTEGRAL_TYPE_P (TREE_TYPE (@1)))
> >        || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> >            && POINTER_TYPE_P (TREE_TYPE (@1))))
> >
> Hi,
> Done suggested changes in this version.
> pr70920-4.c (test-case in patch) is now folded during  ccp instead of
> forwprop after merging the
> two patterns.
> Passes bootstrap+test on x86_64-unknown-linux-gnu.
> OK for trunk ?

(please paste in ChangeLog entries rather than attaching them).

In gcc.dg/tree-ssa/ssa-dom-branch-1.c you need to adjust the comment
before the dump-scan you adjust.

Ok with that change.

Thanks,
Richard.

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

[Prathamesh Kulkarni]

On 26 July 2016 at 17:28, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
>
>> On 25 July 2016 at 14:32, Richard Biener <rguenther@suse.de> wrote:
>> > On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
>> >
>> >> Hi Richard,
>> >> The attached patch tries to fix PR70920.
>> >> It adds your pattern from comment 1 in the PR
>> >> (with additional gating on INTEGRAL_TYPE_P to avoid regressing finalize_18.f90)
>> >> and second pattern, which is reverse of the first transform.
>> >> I needed to update ssa-dom-branch-1.c because with patch applied,
>> >> jump threading removed the second if (i != 0B) block.
>> >> The dumps with and without patch for ssa-dom-branch-1.c start
>> >> to differ with forwprop1:
>> >>
>> >> before:
>> >>  <bb 3>:
>> >>   _1 = temp_16(D)->code;
>> >>   _2 = _1 == 42;
>> >>   _3 = (int) _2;
>> >>   _4 = (long int) _3;
>> >>   temp_17 = (struct rtx_def *) _4;
>> >>   if (temp_17 != 0B)
>> >>     goto <bb 4>;
>> >>   else
>> >>     goto <bb 8>;
>> >>
>> >> after:
>> >> <bb 3>:
>> >>   _1 = temp_16(D)->code;
>> >>   _2 = _1 == 42;
>> >>   _3 = (int) _2;
>> >>   _4 = (long int) _2;
>> >>   temp_17 = (struct rtx_def *) _4;
>> >>   if (_1 == 42)
>> >>     goto <bb 4>;
>> >>   else
>> >>     goto <bb 8>;
>> >>
>> >> I suppose the transform is correct for above test-case ?
>> >>
>> >> Then vrp dump shows:
>> >>  Threaded jump 5 --> 9 to 13
>> >>   Threaded jump 8 --> 9 to 13
>> >>   Threaded jump 3 --> 9 to 13
>> >>   Threaded jump 12 --> 9 to 14
>> >> Removing basic block 9
>> >> basic block 9, loop depth 0
>> >>  pred:
>> >> if (i1_10(D) != 0B)
>> >>   goto <bb 10>;
>> >> else
>> >>   goto <bb 11>;
>> >>  succ:       10
>> >>              11
>> >>
>> >> So there remained two instances of if (i1_10 (D) != 0B) in dom2 dump file,
>> >> and hence needed to update the test-case.
>> >>
>> >> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >> OK to commit ?
>> >
>> > --- a/gcc/match.pd
>> > +++ b/gcc/match.pd
>> > @@ -3408,3 +3408,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>> >          { CONSTRUCTOR_ELT (ctor, idx / k)->value; })
>> >         (BIT_FIELD_REF { CONSTRUCTOR_ELT (ctor, idx / k)->value; }
>> >                        @1 { bitsize_int ((idx % k) * width); })))))))))
>> > +
>> > +/* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST.
>> > */
>> > +
>> > +(for cmp (ne eq)
>> > + (simplify
>> > +  (cmp (convert@2 @0) INTEGER_CST@1)
>> > +  (if (POINTER_TYPE_P (TREE_TYPE (@0))
>> > +       && INTEGRAL_TYPE_P (TREE_TYPE (@2)))
>> >
>> > you can use @1 here and omit @2.
>> >
>> > +   (cmp @0 (convert @1)))))
>> > +
>> > +/* Reverse of the above case:
>> > +   x has integral_type, CST is a pointer constant.
>> > +   Transform (typeof CST)x eq/ne CST to x eq/ne (typeof x) CST.  */
>> > +
>> > +(for cmp (ne eq)
>> > + (simplify
>> > +  (cmp (convert @0) @1)
>> > +  (if (POINTER_TYPE_P (TREE_TYPE (@1))
>> > +       && INTEGRAL_TYPE_P (TREE_TYPE (@0)))
>> > +    (cmp @0 (convert @1)))))
>> >
>> > The second pattern lacks the INTEGER_CST on @1 so it doesn't match
>> > its comment.  Please do not add vertical space between pattern
>> > comment and pattern.
>> >
>> > Please place patterns not at the end of match.pd but where similar
>> > transforms are done.  Like after
>> >
>> > /* Simplify pointer equality compares using PTA.  */
>> > (for neeq (ne eq)
>> >  (simplify
>> >   (neeq @0 @1)
>> >   (if (POINTER_TYPE_P (TREE_TYPE (@0))
>> >        && ptrs_compare_unequal (@0, @1))
>> >    { neeq == EQ_EXPR ? boolean_false_node : boolean_true_node; })))
>> >
>> > please also share the (for ...) for both patterns or merge them
>> > by changing the condition to
>> >
>> >   (if ((POINTER_TYPE_P (TREE_TYPE (@0))
>> >         && INTEGRAL_TYPE_P (TREE_TYPE (@1)))
>> >        || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> >            && POINTER_TYPE_P (TREE_TYPE (@1))))
>> >
>> Hi,
>> Done suggested changes in this version.
>> pr70920-4.c (test-case in patch) is now folded during  ccp instead of
>> forwprop after merging the
>> two patterns.
>> Passes bootstrap+test on x86_64-unknown-linux-gnu.
>> OK for trunk ?
>
> (please paste in ChangeLog entries rather than attaching them).
Will do henceforth.
>
> In gcc.dg/tree-ssa/ssa-dom-branch-1.c you need to adjust the comment
> before the dump-scan you adjust.
>
> Ok with that change.
Thanks, committed as r238754 after adjusting the comment in ssa-dom-branch-1.c.

Thanks,
Prathamesh
>
> Thanks,
> Richard.

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

[Andreas Schwab]

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.

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."

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

[Prathamesh Kulkarni]

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.

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 ?

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."

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

[Richard Biener]

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.

> 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.

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)

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

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 3748 bytes --]

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 ?

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)

[-- Attachment #2: foo.diff --]
[-- Type: text/plain, Size: 610 bytes --]

diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c b/gcc/testsuite/gcc.dg/pr70920-4.c
index dedb895..035c3cb 100644
--- a/gcc/testsuite/gcc.dg/pr70920-4.c
+++ b/gcc/testsuite/gcc.dg/pr70920-4.c
@@ -1,5 +1,5 @@
 /* { 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 +18,4 @@ foo (int a)
     }
 }
 
-/* { dg-final { scan-tree-dump "gimple_simplified to if \\(_\[0-9\]* == 0\\)" "ccp1" } } */
+/* { dg-final { scan-tree-dump "== 0\\)" "forwprop1" } } */

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

[Richard Biener]

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.

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)

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

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 4637 bytes --]

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)

[-- Attachment #2: foo.txt --]
[-- Type: text/plain, Size: 849 bytes --]

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.

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" } } */

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

[Richard Biener]

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)

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

[Matthew Wahab]

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

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

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

On 3 August 2016 at 17:27, Matthew Wahab <matthew.wahab@foss.arm.com> wrote:
> 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.
Oops, sorry about that.
>
> 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).
Indeed, it is skipped after moving to end.
Is it OK to commit the attached patch ?

Thanks,
Prathamesh
>
> Matthew
>

[-- Attachment #2: foo.diff --]
[-- Type: text/plain, Size: 425 bytes --]

diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c b/gcc/testsuite/gcc.dg/pr70920-4.c
index ab2748b..c83ebf9 100644
--- a/gcc/testsuite/gcc.dg/pr70920-4.c
+++ b/gcc/testsuite/gcc.dg/pr70920-4.c
@@ -1,6 +1,6 @@
-/* { dg-require-effective-target lp64 } */
 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-forwprop-details -Wno-int-to-pointer-cast" } */
+/* { dg-require-effective-target lp64 } */
 
 #include <stdint.h>
 

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

[Richard Biener]

On Thu, 4 Aug 2016, Prathamesh Kulkarni wrote:

> On 3 August 2016 at 17:27, Matthew Wahab <matthew.wahab@foss.arm.com> wrote:
> > 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.
> Oops, sorry about that.
> >
> > 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).
> Indeed, it is skipped after moving to end.
> Is it OK to commit the attached patch ?

I believe the canonical place is after do-do but before dg-options.
Can you check if that works, too?

Richard.

> Thanks,
> Prathamesh
> >
> > Matthew
> >
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

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

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

On 4 August 2016 at 12:39, Richard Biener <rguenther@suse.de> wrote:
> On Thu, 4 Aug 2016, Prathamesh Kulkarni wrote:
>
>> On 3 August 2016 at 17:27, Matthew Wahab <matthew.wahab@foss.arm.com> wrote:
>> > 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.
>> Oops, sorry about that.
>> >
>> > 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).
>> Indeed, it is skipped after moving to end.
>> Is it OK to commit the attached patch ?
>
> I believe the canonical place is after do-do but before dg-options.
> Can you check if that works, too?
Yes that works. Should I commit the attached patch ?

Thanks,
Prathamesh
>
> Richard.
>
>> Thanks,
>> Prathamesh
>> >
>> > Matthew
>> >
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

[-- Attachment #2: foo.diff --]
[-- Type: text/plain, Size: 423 bytes --]

diff --git a/gcc/testsuite/gcc.dg/pr70920-4.c b/gcc/testsuite/gcc.dg/pr70920-4.c
index ab2748b..e9c2b95 100644
--- a/gcc/testsuite/gcc.dg/pr70920-4.c
+++ b/gcc/testsuite/gcc.dg/pr70920-4.c
@@ -1,5 +1,5 @@
-/* { dg-require-effective-target lp64 } */
 /* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options "-O2 -fdump-tree-forwprop-details -Wno-int-to-pointer-cast" } */
 
 #include <stdint.h>

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

[Richard Biener]

On Thu, 4 Aug 2016, Prathamesh Kulkarni wrote:

> On 4 August 2016 at 12:39, Richard Biener <rguenther@suse.de> wrote:
> > On Thu, 4 Aug 2016, Prathamesh Kulkarni wrote:
> >
> >> On 3 August 2016 at 17:27, Matthew Wahab <matthew.wahab@foss.arm.com> wrote:
> >> > 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.
> >> Oops, sorry about that.
> >> >
> >> > 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).
> >> Indeed, it is skipped after moving to end.
> >> Is it OK to commit the attached patch ?
> >
> > I believe the canonical place is after do-do but before dg-options.
> > Can you check if that works, too?
> Yes that works. Should I commit the attached patch ?

Yes.

Richard.

> Thanks,
> Prathamesh
> >
> > Richard.
> >
> >> Thanks,
> >> Prathamesh
> >> >
> >> > Matthew
> >> >
> >>
> >
> > --
> > 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)