public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [tree-optimization/71947] Avoid unwanted propagations
@ 2016-10-10 21:06 Jeff Law
  2016-10-12 14:37 ` Christophe Lyon
  2016-10-12 15:00 ` Georg-Johann Lay
  0 siblings, 2 replies; 5+ messages in thread
From: Jeff Law @ 2016-10-10 21:06 UTC (permalink / raw)
  To: gcc-patches

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



So if we have an equality conditional between A & B, we record into our 
const/copy tables A = B and B = A.

This helps us discover some of the more obscure equivalences. But it 
also creates problems with an expression like

A ^ B

Where we might cprop the first operand generating

B ^ B

Then the second generating

B ^ A

ANd we've lost the folding opportunity.  At first I'd tried folding 
after each propagation step, but that turns into a bit of a nightmare 
because of changes in the underlying structure of the gimple statement 
and cycles that may develop if we re-build the operand cache after folding.

This approach is simpler and should catch all these cases for binary 
operators.  We just track the last copy propagated argument and refuse 
to ping-pong propagations.

It fixes the tests from 71947 and 77647 without regressing (obviously). 
I've included an xfailed test for a more complex situation that we don't 
currently handle (would require backtracking from the equality 
comparison through the logicals that feed the equality comparison).

Bootstrapped and regression tested on x86_64.  Applied to the trunk.


[-- Attachment #2: P --]
[-- Type: text/plain, Size: 6177 bytes --]

commit 6223e6e425b6de916f0330b9dbe5698765d4a73c
Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Oct 10 20:40:59 2016 +0000

            PR tree-optimization/71947
    	* tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B, then
    	B with A within a single statement.
    
    	PR tree-optimization/71947
    	* gcc.dg/tree-ssa/pr71947-1.c: New test.
    	* gcc.dg/tree-ssa/pr71947-2.c: New test.
    	* gcc.dg/tree-ssa/pr71947-3.c: New test.
    	* gcc.dg/tree-ssa/pr71947-4.c: New test.
    	* gcc.dg/tree-ssa/pr71947-5.c: New test.
    	* gcc.dg/tree-ssa/pr71947-6.c: New test.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240947 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 1738bc7..16e25bf 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2016-10-10  Jeff Law  <law@redhat.com>
+
+        PR tree-optimization/71947
+	* tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B, then
+	B with A within a single statement.
+
 2016-10-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
 
 	PR tree-optimization/77824
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 04966cf..e31bcc6 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,13 @@
+2016-10-10  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/71947
+	* gcc.dg/tree-ssa/pr71947-1.c: New test.
+	* gcc.dg/tree-ssa/pr71947-2.c: New test.
+	* gcc.dg/tree-ssa/pr71947-3.c: New test.
+	* gcc.dg/tree-ssa/pr71947-4.c: New test.
+	* gcc.dg/tree-ssa/pr71947-5.c: New test.
+	* gcc.dg/tree-ssa/pr71947-6.c: New test.
+
 2016-10-10  Thomas Koenig  <tkoenig@gcc.gnu.org>
 
 	PR fortran/77915
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
new file mode 100644
index 0000000..b033495
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
+
+
+int f(int x, int y)
+{
+   int ret;
+
+   if (x == y)
+     ret = x ^ y;
+   else
+     ret = 1;
+
+   return ret;
+}
+
+/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } } */
+
+
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
new file mode 100644
index 0000000..de8f88b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
+
+
+int f(int x, int y)
+{
+  int ret;
+  if (x == y)
+    ret = x - y;
+  else
+    ret = 1;
+
+  return ret;
+}
+
+/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
new file mode 100644
index 0000000..e79847f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
+
+int f(int x, int y)
+{
+  int ret = 10;
+  if (x == y)
+    ret = x  -  y;
+  return ret;
+}
+
+/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
new file mode 100644
index 0000000..a881f0d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
+
+
+
+static inline long load(long *p)
+{
+        long ret;
+        asm ("movq      %1,%0\n\t" : "=r" (ret) : "m" (*p));
+        if (ret != *p)
+                __builtin_unreachable();
+        return ret;
+}
+
+long foo(long *mem)
+{
+        long ret;
+        ret = load(mem);
+        return ret + *mem;
+}
+
+/* { dg-final { scan-tree-dump "Folded to: _\[0-9\]+ = _\[0-9\]+ \\* 2"  "dom2" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c
new file mode 100644
index 0000000..fa679f0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
+
+
+static inline long load(long *p)
+{
+        long ret;
+        asm ("movq      %1,%0\n\t" : "=r" (ret) : "m" (*p));
+        if (ret != *p)
+                __builtin_unreachable();
+        return ret;
+}
+
+long foo(long *mem)
+{
+        long ret;
+        ret = load(mem);
+        return ret - *mem;
+}
+
+/* { dg-final { scan-tree-dump "Folded to: _\[0-9\]+ = 0;"  "dom2" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c
new file mode 100644
index 0000000..9cb89cb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
+
+
+int f(int x, int y, int a, int b)
+{
+  int ret = 10;
+  if (a == x
+      && b == y
+      && a == b)
+    ret = x - y;
+
+  return ret;
+}
+
+/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" { xfail *-*-* } } } */
diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index b007388..5376ff9 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1731,9 +1731,26 @@ cprop_into_stmt (gimple *stmt)
 {
   use_operand_p op_p;
   ssa_op_iter iter;
+  tree last_copy_propagated_op = NULL;
 
   FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
-    cprop_operand (stmt, op_p);
+    {
+      tree old_op = USE_FROM_PTR (op_p);
+
+      /* If we have A = B and B = A in the copy propagation tables
+	 (due to an equality comparison), avoid substituting B for A
+	 then A for B in the trivially discovered cases.   This allows
+	 optimization of statements were A and B appear as input
+	 operands.  */
+      if (old_op != last_copy_propagated_op)
+	{
+	  cprop_operand (stmt, op_p);
+
+	  tree new_op = USE_FROM_PTR (op_p);
+	  if (new_op != old_op && TREE_CODE (new_op) == SSA_NAME)
+	    last_copy_propagated_op = new_op;
+	}
+    }
 }
 
 /* Optimize the statement in block BB pointed to by iterator SI

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [tree-optimization/71947] Avoid unwanted propagations
  2016-10-10 21:06 [tree-optimization/71947] Avoid unwanted propagations Jeff Law
@ 2016-10-12 14:37 ` Christophe Lyon
  2016-10-19 16:28   ` Jeff Law
  2016-10-12 15:00 ` Georg-Johann Lay
  1 sibling, 1 reply; 5+ messages in thread
From: Christophe Lyon @ 2016-10-12 14:37 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

Hi Jeff,


On 10 October 2016 at 23:06, Jeff Law <law@redhat.com> wrote:
>
>
> So if we have an equality conditional between A & B, we record into our
> const/copy tables A = B and B = A.
>
> This helps us discover some of the more obscure equivalences. But it also
> creates problems with an expression like
>
> A ^ B
>
> Where we might cprop the first operand generating
>
> B ^ B
>
> Then the second generating
>
> B ^ A
>
> ANd we've lost the folding opportunity.  At first I'd tried folding after
> each propagation step, but that turns into a bit of a nightmare because of
> changes in the underlying structure of the gimple statement and cycles that
> may develop if we re-build the operand cache after folding.
>
> This approach is simpler and should catch all these cases for binary
> operators.  We just track the last copy propagated argument and refuse to
> ping-pong propagations.
>
> It fixes the tests from 71947 and 77647 without regressing (obviously). I've
> included an xfailed test for a more complex situation that we don't
> currently handle (would require backtracking from the equality comparison
> through the logicals that feed the equality comparison).
>
FWIW, this test is XPASS on arm-none-linux-gnueabihf
--with-cpu=cortex-a5 --with-fpu=vfpv3-d16-fp16:

Christophe


> Bootstrapped and regression tested on x86_64.  Applied to the trunk.
>
>
> commit 6223e6e425b6de916f0330b9dbe5698765d4a73c
> Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Mon Oct 10 20:40:59 2016 +0000
>
>             PR tree-optimization/71947
>         * tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B, then
>         B with A within a single statement.
>
>         PR tree-optimization/71947
>         * gcc.dg/tree-ssa/pr71947-1.c: New test.
>         * gcc.dg/tree-ssa/pr71947-2.c: New test.
>         * gcc.dg/tree-ssa/pr71947-3.c: New test.
>         * gcc.dg/tree-ssa/pr71947-4.c: New test.
>         * gcc.dg/tree-ssa/pr71947-5.c: New test.
>         * gcc.dg/tree-ssa/pr71947-6.c: New test.
>
>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240947
> 138bc75d-0d04-0410-961f-82ee72b054a4
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 1738bc7..16e25bf 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2016-10-10  Jeff Law  <law@redhat.com>
> +
> +        PR tree-optimization/71947
> +       * tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B, then
> +       B with A within a single statement.
> +
>  2016-10-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR tree-optimization/77824
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 04966cf..e31bcc6 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,13 @@
> +2016-10-10  Jeff Law  <law@redhat.com>
> +
> +       PR tree-optimization/71947
> +       * gcc.dg/tree-ssa/pr71947-1.c: New test.
> +       * gcc.dg/tree-ssa/pr71947-2.c: New test.
> +       * gcc.dg/tree-ssa/pr71947-3.c: New test.
> +       * gcc.dg/tree-ssa/pr71947-4.c: New test.
> +       * gcc.dg/tree-ssa/pr71947-5.c: New test.
> +       * gcc.dg/tree-ssa/pr71947-6.c: New test.
> +
>  2016-10-10  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>         PR fortran/77915
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
> new file mode 100644
> index 0000000..b033495
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +int f(int x, int y)
> +{
> +   int ret;
> +
> +   if (x == y)
> +     ret = x ^ y;
> +   else
> +     ret = 1;
> +
> +   return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } }
> */
> +
> +
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
> new file mode 100644
> index 0000000..de8f88b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +int f(int x, int y)
> +{
> +  int ret;
> +  if (x == y)
> +    ret = x - y;
> +  else
> +    ret = 1;
> +
> +  return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } }
> */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
> new file mode 100644
> index 0000000..e79847f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +int f(int x, int y)
> +{
> +  int ret = 10;
> +  if (x == y)
> +    ret = x  -  y;
> +  return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } }
> */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
> new file mode 100644
> index 0000000..a881f0d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +
> +static inline long load(long *p)
> +{
> +        long ret;
> +        asm ("movq      %1,%0\n\t" : "=r" (ret) : "m" (*p));
> +        if (ret != *p)
> +                __builtin_unreachable();
> +        return ret;
> +}
> +
> +long foo(long *mem)
> +{
> +        long ret;
> +        ret = load(mem);
> +        return ret + *mem;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: _\[0-9\]+ = _\[0-9\]+ \\* 2"
> "dom2" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c
> new file mode 100644
> index 0000000..fa679f0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +static inline long load(long *p)
> +{
> +        long ret;
> +        asm ("movq      %1,%0\n\t" : "=r" (ret) : "m" (*p));
> +        if (ret != *p)
> +                __builtin_unreachable();
> +        return ret;
> +}
> +
> +long foo(long *mem)
> +{
> +        long ret;
> +        ret = load(mem);
> +        return ret - *mem;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: _\[0-9\]+ = 0;"  "dom2" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c
> new file mode 100644
> index 0000000..9cb89cb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +int f(int x, int y, int a, int b)
> +{
> +  int ret = 10;
> +  if (a == x
> +      && b == y
> +      && a == b)
> +    ret = x - y;
> +
> +  return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" {
> xfail *-*-* } } } */
> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
> index b007388..5376ff9 100644
> --- a/gcc/tree-ssa-dom.c
> +++ b/gcc/tree-ssa-dom.c
> @@ -1731,9 +1731,26 @@ cprop_into_stmt (gimple *stmt)
>  {
>    use_operand_p op_p;
>    ssa_op_iter iter;
> +  tree last_copy_propagated_op = NULL;
>
>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
> -    cprop_operand (stmt, op_p);
> +    {
> +      tree old_op = USE_FROM_PTR (op_p);
> +
> +      /* If we have A = B and B = A in the copy propagation tables
> +        (due to an equality comparison), avoid substituting B for A
> +        then A for B in the trivially discovered cases.   This allows
> +        optimization of statements were A and B appear as input
> +        operands.  */
> +      if (old_op != last_copy_propagated_op)
> +       {
> +         cprop_operand (stmt, op_p);
> +
> +         tree new_op = USE_FROM_PTR (op_p);
> +         if (new_op != old_op && TREE_CODE (new_op) == SSA_NAME)
> +           last_copy_propagated_op = new_op;
> +       }
> +    }
>  }
>
>  /* Optimize the statement in block BB pointed to by iterator SI
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [tree-optimization/71947] Avoid unwanted propagations
  2016-10-10 21:06 [tree-optimization/71947] Avoid unwanted propagations Jeff Law
  2016-10-12 14:37 ` Christophe Lyon
@ 2016-10-12 15:00 ` Georg-Johann Lay
  2016-10-19 16:33   ` Jeff Law
  1 sibling, 1 reply; 5+ messages in thread
From: Georg-Johann Lay @ 2016-10-12 15:00 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On 10.10.2016 23:06, Jeff Law wrote:
>
>
> So if we have an equality conditional between A & B, we record into our
> const/copy tables A = B and B = A.
>
> This helps us discover some of the more obscure equivalences. But it also
> creates problems with an expression like
>
> A ^ B
>
> Where we might cprop the first operand generating
>
> B ^ B
>
> Then the second generating
>
> B ^ A
>
> ANd we've lost the folding opportunity.  At first I'd tried folding after each
> propagation step, but that turns into a bit of a nightmare because of changes
> in the underlying structure of the gimple statement and cycles that may develop
> if we re-build the operand cache after folding.
>
> This approach is simpler and should catch all these cases for binary
> operators.  We just track the last copy propagated argument and refuse to
> ping-pong propagations.
>
> It fixes the tests from 71947 and 77647 without regressing (obviously). I've
> included an xfailed test for a more complex situation that we don't currently
> handle (would require backtracking from the equality comparison through the
> logicals that feed the equality comparison).
>
> Bootstrapped and regression tested on x86_64.  Applied to the trunk.
>
> commit 6223e6e425b6de916f0330b9dbe5698765d4a73c
> Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Mon Oct 10 20:40:59 2016 +0000
>
>             PR tree-optimization/71947
>     	* tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B, then
>     	B with A within a single statement.
>
>     	PR tree-optimization/71947
>     	* gcc.dg/tree-ssa/pr71947-1.c: New test.
>     	* gcc.dg/tree-ssa/pr71947-2.c: New test.
>     	* gcc.dg/tree-ssa/pr71947-3.c: New test.
>     	* gcc.dg/tree-ssa/pr71947-4.c: New test.
>     	* gcc.dg/tree-ssa/pr71947-5.c: New test.
>     	* gcc.dg/tree-ssa/pr71947-6.c: New test.
>
>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240947 138bc75d-0d04-0410-961f-82ee72b054a4
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 1738bc7..16e25bf 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2016-10-10  Jeff Law  <law@redhat.com>
> +
> +        PR tree-optimization/71947
> +	* tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B, then
> +	B with A within a single statement.
> +
>  2016-10-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>  	PR tree-optimization/77824
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 04966cf..e31bcc6 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,13 @@
> +2016-10-10  Jeff Law  <law@redhat.com>
> +
> +	PR tree-optimization/71947
> +	* gcc.dg/tree-ssa/pr71947-1.c: New test.
> +	* gcc.dg/tree-ssa/pr71947-2.c: New test.
> +	* gcc.dg/tree-ssa/pr71947-3.c: New test.
> +	* gcc.dg/tree-ssa/pr71947-4.c: New test.
> +	* gcc.dg/tree-ssa/pr71947-5.c: New test.
> +	* gcc.dg/tree-ssa/pr71947-6.c: New test.
> +
>  2016-10-10  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>  	PR fortran/77915
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
> new file mode 100644
> index 0000000..b033495
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +int f(int x, int y)
> +{
> +   int ret;
> +
> +   if (x == y)
> +     ret = x ^ y;
> +   else
> +     ret = 1;
> +
> +   return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } } */
> +
> +
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
> new file mode 100644
> index 0000000..de8f88b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +int f(int x, int y)
> +{
> +  int ret;
> +  if (x == y)
> +    ret = x - y;
> +  else
> +    ret = 1;
> +
> +  return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
> new file mode 100644
> index 0000000..e79847f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +int f(int x, int y)
> +{
> +  int ret = 10;
> +  if (x == y)
> +    ret = x  -  y;
> +  return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
> new file mode 100644
> index 0000000..a881f0d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +
> +static inline long load(long *p)
> +{
> +        long ret;
> +        asm ("movq      %1,%0\n\t" : "=r" (ret) : "m" (*p));


Hi, shouldn't this test be restricted to target machines that understand MOVQ?

Same issue for gcc.dg/tree-ssa/pr71947-5.c below.

Johann

> +        if (ret != *p)
> +                __builtin_unreachable();
> +        return ret;
> +}
> +
> +long foo(long *mem)
> +{
> +        long ret;
> +        ret = load(mem);
> +        return ret + *mem;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: _\[0-9\]+ = _\[0-9\]+ \\* 2"  "dom2" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c
> new file mode 100644
> index 0000000..fa679f0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-5.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +static inline long load(long *p)
> +{
> +        long ret;
> +        asm ("movq      %1,%0\n\t" : "=r" (ret) : "m" (*p));
> +        if (ret != *p)
> +                __builtin_unreachable();
> +        return ret;
> +}
> +
> +long foo(long *mem)
> +{
> +        long ret;
> +        ret = load(mem);
> +        return ret - *mem;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: _\[0-9\]+ = 0;"  "dom2" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c
> new file mode 100644
> index 0000000..9cb89cb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-6.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
> +
> +
> +int f(int x, int y, int a, int b)
> +{
> +  int ret = 10;
> +  if (a == x
> +      && b == y
> +      && a == b)
> +    ret = x - y;
> +
> +  return ret;
> +}
> +
> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2" { xfail *-*-* } } } */
> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
> index b007388..5376ff9 100644
> --- a/gcc/tree-ssa-dom.c
> +++ b/gcc/tree-ssa-dom.c
> @@ -1731,9 +1731,26 @@ cprop_into_stmt (gimple *stmt)
>  {
>    use_operand_p op_p;
>    ssa_op_iter iter;
> +  tree last_copy_propagated_op = NULL;
>
>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
> -    cprop_operand (stmt, op_p);
> +    {
> +      tree old_op = USE_FROM_PTR (op_p);
> +
> +      /* If we have A = B and B = A in the copy propagation tables
> +	 (due to an equality comparison), avoid substituting B for A
> +	 then A for B in the trivially discovered cases.   This allows
> +	 optimization of statements were A and B appear as input
> +	 operands.  */
> +      if (old_op != last_copy_propagated_op)
> +	{
> +	  cprop_operand (stmt, op_p);
> +
> +	  tree new_op = USE_FROM_PTR (op_p);
> +	  if (new_op != old_op && TREE_CODE (new_op) == SSA_NAME)
> +	    last_copy_propagated_op = new_op;
> +	}
> +    }
>  }
>
>  /* Optimize the statement in block BB pointed to by iterator SI

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [tree-optimization/71947] Avoid unwanted propagations
  2016-10-12 14:37 ` Christophe Lyon
@ 2016-10-19 16:28   ` Jeff Law
  0 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2016-10-19 16:28 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: gcc-patches

On 10/12/2016 08:36 AM, Christophe Lyon wrote:
>>
>> It fixes the tests from 71947 and 77647 without regressing (obviously). I've
>> included an xfailed test for a more complex situation that we don't
>> currently handle (would require backtracking from the equality comparison
>> through the logicals that feed the equality comparison).
>>
> FWIW, this test is XPASS on arm-none-linux-gnueabihf
> --with-cpu=cortex-a5 --with-fpu=vfpv3-d16-fp16:
Most likely branch-costing is the culprit.  Let me see what I can do...

jeff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [tree-optimization/71947] Avoid unwanted propagations
  2016-10-12 15:00 ` Georg-Johann Lay
@ 2016-10-19 16:33   ` Jeff Law
  0 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2016-10-19 16:33 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: gcc-patches

On 10/12/2016 09:00 AM, Georg-Johann Lay wrote:
> On 10.10.2016 23:06, Jeff Law wrote:
>>
>>
>> So if we have an equality conditional between A & B, we record into our
>> const/copy tables A = B and B = A.
>>
>> This helps us discover some of the more obscure equivalences. But it also
>> creates problems with an expression like
>>
>> A ^ B
>>
>> Where we might cprop the first operand generating
>>
>> B ^ B
>>
>> Then the second generating
>>
>> B ^ A
>>
>> ANd we've lost the folding opportunity.  At first I'd tried folding
>> after each
>> propagation step, but that turns into a bit of a nightmare because of
>> changes
>> in the underlying structure of the gimple statement and cycles that
>> may develop
>> if we re-build the operand cache after folding.
>>
>> This approach is simpler and should catch all these cases for binary
>> operators.  We just track the last copy propagated argument and refuse to
>> ping-pong propagations.
>>
>> It fixes the tests from 71947 and 77647 without regressing
>> (obviously). I've
>> included an xfailed test for a more complex situation that we don't
>> currently
>> handle (would require backtracking from the equality comparison
>> through the
>> logicals that feed the equality comparison).
>>
>> Bootstrapped and regression tested on x86_64.  Applied to the trunk.
>>
>> commit 6223e6e425b6de916f0330b9dbe5698765d4a73c
>> Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
>> Date:   Mon Oct 10 20:40:59 2016 +0000
>>
>>             PR tree-optimization/71947
>>         * tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B,
>> then
>>         B with A within a single statement.
>>
>>         PR tree-optimization/71947
>>         * gcc.dg/tree-ssa/pr71947-1.c: New test.
>>         * gcc.dg/tree-ssa/pr71947-2.c: New test.
>>         * gcc.dg/tree-ssa/pr71947-3.c: New test.
>>         * gcc.dg/tree-ssa/pr71947-4.c: New test.
>>         * gcc.dg/tree-ssa/pr71947-5.c: New test.
>>         * gcc.dg/tree-ssa/pr71947-6.c: New test.
>>
>>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240947
>> 138bc75d-0d04-0410-961f-82ee72b054a4
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index 1738bc7..16e25bf 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2016-10-10  Jeff Law  <law@redhat.com>
>> +
>> +        PR tree-optimization/71947
>> +    * tree-ssa-dom.c (cprop_into_stmt): Avoid replacing A with B, then
>> +    B with A within a single statement.
>> +
>>  2016-10-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>>
>>      PR tree-optimization/77824
>> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
>> index 04966cf..e31bcc6 100644
>> --- a/gcc/testsuite/ChangeLog
>> +++ b/gcc/testsuite/ChangeLog
>> @@ -1,3 +1,13 @@
>> +2016-10-10  Jeff Law  <law@redhat.com>
>> +
>> +    PR tree-optimization/71947
>> +    * gcc.dg/tree-ssa/pr71947-1.c: New test.
>> +    * gcc.dg/tree-ssa/pr71947-2.c: New test.
>> +    * gcc.dg/tree-ssa/pr71947-3.c: New test.
>> +    * gcc.dg/tree-ssa/pr71947-4.c: New test.
>> +    * gcc.dg/tree-ssa/pr71947-5.c: New test.
>> +    * gcc.dg/tree-ssa/pr71947-6.c: New test.
>> +
>>  2016-10-10  Thomas Koenig  <tkoenig@gcc.gnu.org>
>>
>>      PR fortran/77915
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
>> new file mode 100644
>> index 0000000..b033495
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-1.c
>> @@ -0,0 +1,19 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
>> +
>> +
>> +int f(int x, int y)
>> +{
>> +   int ret;
>> +
>> +   if (x == y)
>> +     ret = x ^ y;
>> +   else
>> +     ret = 1;
>> +
>> +   return ret;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2"
>> } } */
>> +
>> +
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
>> new file mode 100644
>> index 0000000..de8f88b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-2.c
>> @@ -0,0 +1,16 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
>> +
>> +
>> +int f(int x, int y)
>> +{
>> +  int ret;
>> +  if (x == y)
>> +    ret = x - y;
>> +  else
>> +    ret = 1;
>> +
>> +  return ret;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2"
>> } } */
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
>> new file mode 100644
>> index 0000000..e79847f
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-3.c
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
>> +
>> +int f(int x, int y)
>> +{
>> +  int ret = 10;
>> +  if (x == y)
>> +    ret = x  -  y;
>> +  return ret;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump "Folded to: ret_\[0-9\]+ = 0;"  "dom2"
>> } } */
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
>> new file mode 100644
>> index 0000000..a881f0d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71947-4.c
>> @@ -0,0 +1,22 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-dom-details" } */
>> +
>> +
>> +
>> +static inline long load(long *p)
>> +{
>> +        long ret;
>> +        asm ("movq      %1,%0\n\t" : "=r" (ret) : "m" (*p));
>
>
> Hi, shouldn't this test be restricted to target machines that understand
> MOVQ?
>
> Same issue for gcc.dg/tree-ssa/pr71947-5.c below.
It shouldn't actually matter -- we compile to assembly, but never try to 
assemble the resulting code.  One could replace the movq with anything.

The test actually verifies simplification of the return statement in 
"foo". In one test it should be 2*ret and the other 0.  In neither case 
should it load *mem again.

jeff
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-10-19 16:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-10 21:06 [tree-optimization/71947] Avoid unwanted propagations Jeff Law
2016-10-12 14:37 ` Christophe Lyon
2016-10-19 16:28   ` Jeff Law
2016-10-12 15:00 ` Georg-Johann Lay
2016-10-19 16:33   ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).