public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
@ 2017-05-06 15:33 Jeff Law
  2017-05-06 18:07 ` Trevor Saunders
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Jeff Law @ 2017-05-06 15:33 UTC (permalink / raw)
  To: gcc-patches

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

This is the 2nd of 3-5 patches to address pr78496.

Jump threading will examine ASSERT_EXPRs as it walks the IL and record 
information from those ASSERT_EXPRs into the available expression and 
const/copies tables where they're later used to simplify conditionals.

We're missing a couple things though.

First an ASSERT_EXPR with an EQ test creates an equality we can enter 
into the const/copy tables.  We were failing to do that.  This is most 
interesting when the RHS of the condition in the ASSERT_EXPR is a 
constant, obviously.  This has a secondary benefit of doing less work to 
get better optimization.

Second, some ASSERT_EXPRs may start off as a relational test such as x 
<= 0, but after range extraction and propagation they could be 
simplified into an equality comparison.  We already do this with 
conditionals and generalizing that code to handle ASSERT_EXPRs is pretty 
easy.  This gives us more chances to extract simple equivalences from 
the condition in ASSERT_EXPRs.

This patch fixes those 2 problems.  I don't think it directly helps 
pr78496 right now as we're unable to pick up the newly exposed jump 
threads until VRP2.   But that's a short term limitation that I've 
already addressed locally :-)

Bootstrapped, regression tested and installed on the trunk.

jeff


ps. An astute observer might note that improving the effectiveness of 
VRP jump threading seems counterproductive since I've stated I want to 
remove VRP jump threading.  These improvements don't significantly 
change how I was planning to do that or the infrastructure we're going 
to rely upon to make that change.  All that changes is where we get the 
information from -- ASSERT_EXPRs vs querying a new API that generates 
the same information as needed.

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

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b0c253b09ae..0f78f2a2ed1 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2017-05-06  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/78496
+	* tree-vrp.c (simplify_assert_expr_using_ranges): New function.
+	(simplify_stmt_using_ranges): Call it.
+	(vrp_dom_walker::before_dom_children): Extract equivalences
+	from an ASSERT_EXPR with an equality comparison against a
+	constant.
+
 2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
 
 	* lra-constraints.c (lra_copy_reg_equiv): New function.
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index fca5b87e798..42782a6d17c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-05-06  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/78496
+	* gcc.dg/tree-ssa/ssa-thread-16.c: New test.
+	* gcc.dg/tree-ssa/ssa-thread-17.c: New test.
+
 2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
 
 	* gcc.target/aarch64/spill_1.c: New test.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
new file mode 100644
index 00000000000..78c349ca14d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
@@ -0,0 +1,38 @@
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
+
+/* We should thread the if (exp == 2) conditional on the
+   the path from inside the if (x) THEN arm.  It is the only
+   jump threading opportunity in this code.  */
+   
+/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
+
+
+extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
+  __attribute__ ((__noreturn__));
+
+int x;
+
+
+int code;
+void
+do_jump (int exp)
+{
+  switch (code)
+    {
+    case 4:
+      if ((exp) == 1)
+	goto normal;
+      if (x)
+	{
+	  if (exp != 0)
+	    abort ();
+	}
+      if ((exp) == 2)
+	goto normal;
+    case 3:
+	abort ();
+    }
+  normal:
+      ;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
new file mode 100644
index 00000000000..692658fbb4b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
@@ -0,0 +1,36 @@
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
+
+/* We should simplify one ASSERT_EXPR from a relational
+   into an equality test.  */
+/* { dg-final { scan-tree-dump-times "Folded into:\[^\r\n\]*ASSERT_EXPR\*\[^\r\n\]* == 1" 1 "vrp1" } } */
+
+/* And simplification of the ASSERT_EXPR leads to a jump threading opportunity.  */
+/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
+
+extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
+  __attribute__ ((__noreturn__));
+
+union gimple_statement_d;
+typedef union gimple_statement_d *gimple;
+
+
+
+union gimple_statement_d
+{
+  unsigned num_ops;
+};
+
+void
+gimple_assign_set_rhs_with_ops_1 (int code, gimple stmt, unsigned new_rhs_ops)
+{
+
+  stmt->num_ops = new_rhs_ops + 1;
+  if (stmt->num_ops <= 1)
+    abort ();
+  if (new_rhs_ops > 1)
+    if (stmt->num_ops <= 2)
+      abort ();
+  if (new_rhs_ops > 2)
+      abort ();
+}
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index cf50e908257..461a48f5c9c 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -9600,6 +9600,43 @@ range_fits_type_p (value_range *vr, unsigned dest_precision, signop dest_sgn)
   return true;
 }
 
+/* Simplify STMT, an ASSERT_EXPR, using ranges.  This is helpful because jump
+   threading looks at the ASSERT_EXPRs.  Collapsing the condition of
+   an ASSERT_EXPR from a relational to an equality test is where most
+   of the benefit occurrs, so that's the only thing we currently do.  */
+
+static bool
+simplify_assert_expr_using_ranges (gimple *stmt)
+{
+  return false;
+  tree cond = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
+  tree_code code = TREE_CODE (cond);
+  tree op0 = TREE_OPERAND (cond, 0);
+
+  /* The condition of the ASSERT_EXPR must be a simple relational
+     between an SSA_NAME (with a range) and a constant.  */
+  if (TREE_CODE (op0) != SSA_NAME
+      || !INTEGRAL_TYPE_P (TREE_TYPE (op0)))
+    return false;
+
+  tree op1 = TREE_OPERAND (cond, 1);
+  if (TREE_CODE (op1) != INTEGER_CST)
+    return false;
+
+  value_range *vr = get_value_range (op0);
+  if (!vr || vr->type != VR_RANGE)
+    return false;
+
+  tree res = test_for_singularity (code, op0, op1, vr);
+  if (res)
+    {
+      TREE_SET_CODE (cond, EQ_EXPR);
+      TREE_OPERAND (cond, 1) = res;
+      return true;
+    }
+  return false;
+}
+
 /* Simplify a conditional using a relational operator to an equality
    test if the range information indicates only one value can satisfy
    the original conditional.  */
@@ -10334,6 +10371,9 @@ simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
 	case MAX_EXPR:
 	  return simplify_min_or_max_using_ranges (gsi, stmt);
 
+	case ASSERT_EXPR:
+	  return simplify_assert_expr_using_ranges (stmt);
+
 	default:
 	  break;
 	}
@@ -10598,6 +10638,18 @@ vrp_dom_walker::before_dom_children (basic_block bb)
 	{
 	  tree rhs1 = gimple_assign_rhs1 (stmt);
 	  tree cond = TREE_OPERAND (rhs1, 1);
+	  tree lhs = gimple_assign_lhs (stmt);
+	  m_const_and_copies->record_const_or_copy (lhs, TREE_OPERAND (rhs1, 0));
+
+	  if (TREE_CODE (cond) == EQ_EXPR)
+	    {
+	      tree cond_op0 = TREE_OPERAND (cond, 0);
+	      tree cond_op1 = TREE_OPERAND (cond, 1);
+	      if (TREE_CODE (cond_op0) == SSA_NAME)
+		m_const_and_copies->record_const_or_copy (cond_op0, cond_op1);
+	      continue;
+	    }
+
 	  tree inverted = invert_truthvalue (cond);
 	  vec<cond_equivalence> p;
 	  p.create (3);
@@ -10605,9 +10657,6 @@ vrp_dom_walker::before_dom_children (basic_block bb)
 	  for (unsigned int i = 0; i < p.length (); i++)
 	    m_avail_exprs_stack->record_cond (&p[i]);
 
-	  tree lhs = gimple_assign_lhs (stmt);
-	  m_const_and_copies->record_const_or_copy (lhs,
-						    TREE_OPERAND (rhs1, 0));
 	  p.release ();
 	  continue;
 	}

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-06 15:33 [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading Jeff Law
@ 2017-05-06 18:07 ` Trevor Saunders
  2017-05-06 20:17   ` Jeff Law
  2017-05-06 23:56 ` Andrew Pinski
  2017-05-08  7:33 ` Richard Biener via gcc-patches
  2 siblings, 1 reply; 11+ messages in thread
From: Trevor Saunders @ 2017-05-06 18:07 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

> +simplify_assert_expr_using_ranges (gimple *stmt)
> +{
> +  return false;

I assume you didn't mean to leave that in?

Trev

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-06 18:07 ` Trevor Saunders
@ 2017-05-06 20:17   ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2017-05-06 20:17 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: gcc-patches

On 05/06/2017 09:38 AM, Trevor Saunders wrote:
>> +simplify_assert_expr_using_ranges (gimple *stmt)
>> +{
>> +  return false;
> 
> I assume you didn't mean to leave that in?
No I didn't.  I used to to extract the testcases.  Ugh.
jeff

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-06 15:33 [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading Jeff Law
  2017-05-06 18:07 ` Trevor Saunders
@ 2017-05-06 23:56 ` Andrew Pinski
  2017-05-07  9:10   ` Andrew Pinski
  2017-05-08  7:33 ` Richard Biener via gcc-patches
  2 siblings, 1 reply; 11+ messages in thread
From: Andrew Pinski @ 2017-05-06 23:56 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Sat, May 6, 2017 at 8:03 AM, Jeff Law <law@redhat.com> wrote:
> This is the 2nd of 3-5 patches to address pr78496.
>
> Jump threading will examine ASSERT_EXPRs as it walks the IL and record
> information from those ASSERT_EXPRs into the available expression and
> const/copies tables where they're later used to simplify conditionals.
>
> We're missing a couple things though.
>
> First an ASSERT_EXPR with an EQ test creates an equality we can enter into
> the const/copy tables.  We were failing to do that.  This is most
> interesting when the RHS of the condition in the ASSERT_EXPR is a constant,
> obviously.  This has a secondary benefit of doing less work to get better
> optimization.
>
> Second, some ASSERT_EXPRs may start off as a relational test such as x <= 0,
> but after range extraction and propagation they could be simplified into an
> equality comparison.  We already do this with conditionals and generalizing
> that code to handle ASSERT_EXPRs is pretty easy.  This gives us more chances
> to extract simple equivalences from the condition in ASSERT_EXPRs.
>
> This patch fixes those 2 problems.  I don't think it directly helps pr78496
> right now as we're unable to pick up the newly exposed jump threads until
> VRP2.   But that's a short term limitation that I've already addressed
> locally :-)
>
> Bootstrapped, regression tested and installed on the trunk.


After this patch on aarch64-linux-gnu I get a bootstrap failure while
linking lto1/cc1/cc1plus/go1 in stage2:
godump.o: In function `go_define(unsigned int, char const*)':
godump.c:(.text+0x36c): relocation truncated to fit:
R_AARCH64_ADR_PREL_LO21 against `.rodata'
godump.c:(.text+0x4b4): relocation truncated to fit:
R_AARCH64_ADR_PREL_LO21 against `.rodata'
collect2: error: ld returned 1 exit status
../../gcc/gcc/lto/Make-lang.in:81: recipe for target 'lto1' failed
make[3]: *** [lto1] Error 1

Thanks,
Andrew


>
> jeff
>
>
> ps. An astute observer might note that improving the effectiveness of VRP
> jump threading seems counterproductive since I've stated I want to remove
> VRP jump threading.  These improvements don't significantly change how I was
> planning to do that or the infrastructure we're going to rely upon to make
> that change.  All that changes is where we get the information from --
> ASSERT_EXPRs vs querying a new API that generates the same information as
> needed.
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index b0c253b09ae..0f78f2a2ed1 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2017-05-06  Jeff Law  <law@redhat.com>
> +
> +       PR tree-optimization/78496
> +       * tree-vrp.c (simplify_assert_expr_using_ranges): New function.
> +       (simplify_stmt_using_ranges): Call it.
> +       (vrp_dom_walker::before_dom_children): Extract equivalences
> +       from an ASSERT_EXPR with an equality comparison against a
> +       constant.
> +
>  2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
>
>         * lra-constraints.c (lra_copy_reg_equiv): New function.
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index fca5b87e798..42782a6d17c 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-05-06  Jeff Law  <law@redhat.com>
> +
> +       PR tree-optimization/78496
> +       * gcc.dg/tree-ssa/ssa-thread-16.c: New test.
> +       * gcc.dg/tree-ssa/ssa-thread-17.c: New test.
> +
>  2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
>
>         * gcc.target/aarch64/spill_1.c: New test.
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
> new file mode 100644
> index 00000000000..78c349ca14d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
> @@ -0,0 +1,38 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
> +
> +/* We should thread the if (exp == 2) conditional on the
> +   the path from inside the if (x) THEN arm.  It is the only
> +   jump threading opportunity in this code.  */
> +
> +/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
> +
> +
> +extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
> +  __attribute__ ((__noreturn__));
> +
> +int x;
> +
> +
> +int code;
> +void
> +do_jump (int exp)
> +{
> +  switch (code)
> +    {
> +    case 4:
> +      if ((exp) == 1)
> +       goto normal;
> +      if (x)
> +       {
> +         if (exp != 0)
> +           abort ();
> +       }
> +      if ((exp) == 2)
> +       goto normal;
> +    case 3:
> +       abort ();
> +    }
> +  normal:
> +      ;
> +}
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
> new file mode 100644
> index 00000000000..692658fbb4b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
> +
> +/* We should simplify one ASSERT_EXPR from a relational
> +   into an equality test.  */
> +/* { dg-final { scan-tree-dump-times "Folded
> into:\[^\r\n\]*ASSERT_EXPR\*\[^\r\n\]* == 1" 1 "vrp1" } } */
> +
> +/* And simplification of the ASSERT_EXPR leads to a jump threading
> opportunity.  */
> +/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
> +
> +extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
> +  __attribute__ ((__noreturn__));
> +
> +union gimple_statement_d;
> +typedef union gimple_statement_d *gimple;
> +
> +
> +
> +union gimple_statement_d
> +{
> +  unsigned num_ops;
> +};
> +
> +void
> +gimple_assign_set_rhs_with_ops_1 (int code, gimple stmt, unsigned
> new_rhs_ops)
> +{
> +
> +  stmt->num_ops = new_rhs_ops + 1;
> +  if (stmt->num_ops <= 1)
> +    abort ();
> +  if (new_rhs_ops > 1)
> +    if (stmt->num_ops <= 2)
> +      abort ();
> +  if (new_rhs_ops > 2)
> +      abort ();
> +}
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index cf50e908257..461a48f5c9c 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -9600,6 +9600,43 @@ range_fits_type_p (value_range *vr, unsigned
> dest_precision, signop dest_sgn)
>    return true;
>  }
>
> +/* Simplify STMT, an ASSERT_EXPR, using ranges.  This is helpful because
> jump
> +   threading looks at the ASSERT_EXPRs.  Collapsing the condition of
> +   an ASSERT_EXPR from a relational to an equality test is where most
> +   of the benefit occurrs, so that's the only thing we currently do.  */
> +
> +static bool
> +simplify_assert_expr_using_ranges (gimple *stmt)
> +{
> +  return false;
> +  tree cond = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
> +  tree_code code = TREE_CODE (cond);
> +  tree op0 = TREE_OPERAND (cond, 0);
> +
> +  /* The condition of the ASSERT_EXPR must be a simple relational
> +     between an SSA_NAME (with a range) and a constant.  */
> +  if (TREE_CODE (op0) != SSA_NAME
> +      || !INTEGRAL_TYPE_P (TREE_TYPE (op0)))
> +    return false;
> +
> +  tree op1 = TREE_OPERAND (cond, 1);
> +  if (TREE_CODE (op1) != INTEGER_CST)
> +    return false;
> +
> +  value_range *vr = get_value_range (op0);
> +  if (!vr || vr->type != VR_RANGE)
> +    return false;
> +
> +  tree res = test_for_singularity (code, op0, op1, vr);
> +  if (res)
> +    {
> +      TREE_SET_CODE (cond, EQ_EXPR);
> +      TREE_OPERAND (cond, 1) = res;
> +      return true;
> +    }
> +  return false;
> +}
> +
>  /* Simplify a conditional using a relational operator to an equality
>     test if the range information indicates only one value can satisfy
>     the original conditional.  */
> @@ -10334,6 +10371,9 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
> *gsi)
>         case MAX_EXPR:
>           return simplify_min_or_max_using_ranges (gsi, stmt);
>
> +       case ASSERT_EXPR:
> +         return simplify_assert_expr_using_ranges (stmt);
> +
>         default:
>           break;
>         }
> @@ -10598,6 +10638,18 @@ vrp_dom_walker::before_dom_children (basic_block
> bb)
>         {
>           tree rhs1 = gimple_assign_rhs1 (stmt);
>           tree cond = TREE_OPERAND (rhs1, 1);
> +         tree lhs = gimple_assign_lhs (stmt);
> +         m_const_and_copies->record_const_or_copy (lhs, TREE_OPERAND (rhs1,
> 0));
> +
> +         if (TREE_CODE (cond) == EQ_EXPR)
> +           {
> +             tree cond_op0 = TREE_OPERAND (cond, 0);
> +             tree cond_op1 = TREE_OPERAND (cond, 1);
> +             if (TREE_CODE (cond_op0) == SSA_NAME)
> +               m_const_and_copies->record_const_or_copy (cond_op0,
> cond_op1);
> +             continue;
> +           }
> +
>           tree inverted = invert_truthvalue (cond);
>           vec<cond_equivalence> p;
>           p.create (3);
> @@ -10605,9 +10657,6 @@ vrp_dom_walker::before_dom_children (basic_block bb)
>           for (unsigned int i = 0; i < p.length (); i++)
>             m_avail_exprs_stack->record_cond (&p[i]);
>
> -         tree lhs = gimple_assign_lhs (stmt);
> -         m_const_and_copies->record_const_or_copy (lhs,
> -                                                   TREE_OPERAND (rhs1, 0));
>           p.release ();
>           continue;
>         }
>

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-06 23:56 ` Andrew Pinski
@ 2017-05-07  9:10   ` Andrew Pinski
  2017-05-07 17:32     ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Pinski @ 2017-05-07  9:10 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Sat, May 6, 2017 at 4:55 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Sat, May 6, 2017 at 8:03 AM, Jeff Law <law@redhat.com> wrote:
>> This is the 2nd of 3-5 patches to address pr78496.
>>
>> Jump threading will examine ASSERT_EXPRs as it walks the IL and record
>> information from those ASSERT_EXPRs into the available expression and
>> const/copies tables where they're later used to simplify conditionals.
>>
>> We're missing a couple things though.
>>
>> First an ASSERT_EXPR with an EQ test creates an equality we can enter into
>> the const/copy tables.  We were failing to do that.  This is most
>> interesting when the RHS of the condition in the ASSERT_EXPR is a constant,
>> obviously.  This has a secondary benefit of doing less work to get better
>> optimization.
>>
>> Second, some ASSERT_EXPRs may start off as a relational test such as x <= 0,
>> but after range extraction and propagation they could be simplified into an
>> equality comparison.  We already do this with conditionals and generalizing
>> that code to handle ASSERT_EXPRs is pretty easy.  This gives us more chances
>> to extract simple equivalences from the condition in ASSERT_EXPRs.
>>
>> This patch fixes those 2 problems.  I don't think it directly helps pr78496
>> right now as we're unable to pick up the newly exposed jump threads until
>> VRP2.   But that's a short term limitation that I've already addressed
>> locally :-)
>>
>> Bootstrapped, regression tested and installed on the trunk.
>
>
> After this patch on aarch64-linux-gnu I get a bootstrap failure while
> linking lto1/cc1/cc1plus/go1 in stage2:
> godump.o: In function `go_define(unsigned int, char const*)':
> godump.c:(.text+0x36c): relocation truncated to fit:
> R_AARCH64_ADR_PREL_LO21 against `.rodata'
> godump.c:(.text+0x4b4): relocation truncated to fit:
> R_AARCH64_ADR_PREL_LO21 against `.rodata'
> collect2: error: ld returned 1 exit status
> ../../gcc/gcc/lto/Make-lang.in:81: recipe for target 'lto1' failed
> make[3]: *** [lto1] Error 1

I should mention this is even after the patch to
simplify_assert_expr_using_ranges .

Thanks,
Andrew

>
> Thanks,
> Andrew
>
>
>>
>> jeff
>>
>>
>> ps. An astute observer might note that improving the effectiveness of VRP
>> jump threading seems counterproductive since I've stated I want to remove
>> VRP jump threading.  These improvements don't significantly change how I was
>> planning to do that or the infrastructure we're going to rely upon to make
>> that change.  All that changes is where we get the information from --
>> ASSERT_EXPRs vs querying a new API that generates the same information as
>> needed.
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index b0c253b09ae..0f78f2a2ed1 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,12 @@
>> +2017-05-06  Jeff Law  <law@redhat.com>
>> +
>> +       PR tree-optimization/78496
>> +       * tree-vrp.c (simplify_assert_expr_using_ranges): New function.
>> +       (simplify_stmt_using_ranges): Call it.
>> +       (vrp_dom_walker::before_dom_children): Extract equivalences
>> +       from an ASSERT_EXPR with an equality comparison against a
>> +       constant.
>> +
>>  2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
>>
>>         * lra-constraints.c (lra_copy_reg_equiv): New function.
>> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
>> index fca5b87e798..42782a6d17c 100644
>> --- a/gcc/testsuite/ChangeLog
>> +++ b/gcc/testsuite/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2017-05-06  Jeff Law  <law@redhat.com>
>> +
>> +       PR tree-optimization/78496
>> +       * gcc.dg/tree-ssa/ssa-thread-16.c: New test.
>> +       * gcc.dg/tree-ssa/ssa-thread-17.c: New test.
>> +
>>  2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
>>
>>         * gcc.target/aarch64/spill_1.c: New test.
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
>> new file mode 100644
>> index 00000000000..78c349ca14d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
>> @@ -0,0 +1,38 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
>> +
>> +/* We should thread the if (exp == 2) conditional on the
>> +   the path from inside the if (x) THEN arm.  It is the only
>> +   jump threading opportunity in this code.  */
>> +
>> +/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
>> +
>> +
>> +extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
>> +  __attribute__ ((__noreturn__));
>> +
>> +int x;
>> +
>> +
>> +int code;
>> +void
>> +do_jump (int exp)
>> +{
>> +  switch (code)
>> +    {
>> +    case 4:
>> +      if ((exp) == 1)
>> +       goto normal;
>> +      if (x)
>> +       {
>> +         if (exp != 0)
>> +           abort ();
>> +       }
>> +      if ((exp) == 2)
>> +       goto normal;
>> +    case 3:
>> +       abort ();
>> +    }
>> +  normal:
>> +      ;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
>> new file mode 100644
>> index 00000000000..692658fbb4b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
>> @@ -0,0 +1,36 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
>> +
>> +/* We should simplify one ASSERT_EXPR from a relational
>> +   into an equality test.  */
>> +/* { dg-final { scan-tree-dump-times "Folded
>> into:\[^\r\n\]*ASSERT_EXPR\*\[^\r\n\]* == 1" 1 "vrp1" } } */
>> +
>> +/* And simplification of the ASSERT_EXPR leads to a jump threading
>> opportunity.  */
>> +/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
>> +
>> +extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
>> +  __attribute__ ((__noreturn__));
>> +
>> +union gimple_statement_d;
>> +typedef union gimple_statement_d *gimple;
>> +
>> +
>> +
>> +union gimple_statement_d
>> +{
>> +  unsigned num_ops;
>> +};
>> +
>> +void
>> +gimple_assign_set_rhs_with_ops_1 (int code, gimple stmt, unsigned
>> new_rhs_ops)
>> +{
>> +
>> +  stmt->num_ops = new_rhs_ops + 1;
>> +  if (stmt->num_ops <= 1)
>> +    abort ();
>> +  if (new_rhs_ops > 1)
>> +    if (stmt->num_ops <= 2)
>> +      abort ();
>> +  if (new_rhs_ops > 2)
>> +      abort ();
>> +}
>> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
>> index cf50e908257..461a48f5c9c 100644
>> --- a/gcc/tree-vrp.c
>> +++ b/gcc/tree-vrp.c
>> @@ -9600,6 +9600,43 @@ range_fits_type_p (value_range *vr, unsigned
>> dest_precision, signop dest_sgn)
>>    return true;
>>  }
>>
>> +/* Simplify STMT, an ASSERT_EXPR, using ranges.  This is helpful because
>> jump
>> +   threading looks at the ASSERT_EXPRs.  Collapsing the condition of
>> +   an ASSERT_EXPR from a relational to an equality test is where most
>> +   of the benefit occurrs, so that's the only thing we currently do.  */
>> +
>> +static bool
>> +simplify_assert_expr_using_ranges (gimple *stmt)
>> +{
>> +  return false;
>> +  tree cond = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
>> +  tree_code code = TREE_CODE (cond);
>> +  tree op0 = TREE_OPERAND (cond, 0);
>> +
>> +  /* The condition of the ASSERT_EXPR must be a simple relational
>> +     between an SSA_NAME (with a range) and a constant.  */
>> +  if (TREE_CODE (op0) != SSA_NAME
>> +      || !INTEGRAL_TYPE_P (TREE_TYPE (op0)))
>> +    return false;
>> +
>> +  tree op1 = TREE_OPERAND (cond, 1);
>> +  if (TREE_CODE (op1) != INTEGER_CST)
>> +    return false;
>> +
>> +  value_range *vr = get_value_range (op0);
>> +  if (!vr || vr->type != VR_RANGE)
>> +    return false;
>> +
>> +  tree res = test_for_singularity (code, op0, op1, vr);
>> +  if (res)
>> +    {
>> +      TREE_SET_CODE (cond, EQ_EXPR);
>> +      TREE_OPERAND (cond, 1) = res;
>> +      return true;
>> +    }
>> +  return false;
>> +}
>> +
>>  /* Simplify a conditional using a relational operator to an equality
>>     test if the range information indicates only one value can satisfy
>>     the original conditional.  */
>> @@ -10334,6 +10371,9 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
>> *gsi)
>>         case MAX_EXPR:
>>           return simplify_min_or_max_using_ranges (gsi, stmt);
>>
>> +       case ASSERT_EXPR:
>> +         return simplify_assert_expr_using_ranges (stmt);
>> +
>>         default:
>>           break;
>>         }
>> @@ -10598,6 +10638,18 @@ vrp_dom_walker::before_dom_children (basic_block
>> bb)
>>         {
>>           tree rhs1 = gimple_assign_rhs1 (stmt);
>>           tree cond = TREE_OPERAND (rhs1, 1);
>> +         tree lhs = gimple_assign_lhs (stmt);
>> +         m_const_and_copies->record_const_or_copy (lhs, TREE_OPERAND (rhs1,
>> 0));
>> +
>> +         if (TREE_CODE (cond) == EQ_EXPR)
>> +           {
>> +             tree cond_op0 = TREE_OPERAND (cond, 0);
>> +             tree cond_op1 = TREE_OPERAND (cond, 1);
>> +             if (TREE_CODE (cond_op0) == SSA_NAME)
>> +               m_const_and_copies->record_const_or_copy (cond_op0,
>> cond_op1);
>> +             continue;
>> +           }
>> +
>>           tree inverted = invert_truthvalue (cond);
>>           vec<cond_equivalence> p;
>>           p.create (3);
>> @@ -10605,9 +10657,6 @@ vrp_dom_walker::before_dom_children (basic_block bb)
>>           for (unsigned int i = 0; i < p.length (); i++)
>>             m_avail_exprs_stack->record_cond (&p[i]);
>>
>> -         tree lhs = gimple_assign_lhs (stmt);
>> -         m_const_and_copies->record_const_or_copy (lhs,
>> -                                                   TREE_OPERAND (rhs1, 0));
>>           p.release ();
>>           continue;
>>         }
>>

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-07  9:10   ` Andrew Pinski
@ 2017-05-07 17:32     ` Jeff Law
  2017-05-08  3:20       ` Andrew Pinski
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2017-05-07 17:32 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On 05/06/2017 05:56 PM, Andrew Pinski wrote:
> On Sat, May 6, 2017 at 4:55 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Sat, May 6, 2017 at 8:03 AM, Jeff Law <law@redhat.com> wrote:
>>> This is the 2nd of 3-5 patches to address pr78496.
>>>
>>> Jump threading will examine ASSERT_EXPRs as it walks the IL and record
>>> information from those ASSERT_EXPRs into the available expression and
>>> const/copies tables where they're later used to simplify conditionals.
>>>
>>> We're missing a couple things though.
>>>
>>> First an ASSERT_EXPR with an EQ test creates an equality we can enter into
>>> the const/copy tables.  We were failing to do that.  This is most
>>> interesting when the RHS of the condition in the ASSERT_EXPR is a constant,
>>> obviously.  This has a secondary benefit of doing less work to get better
>>> optimization.
>>>
>>> Second, some ASSERT_EXPRs may start off as a relational test such as x <= 0,
>>> but after range extraction and propagation they could be simplified into an
>>> equality comparison.  We already do this with conditionals and generalizing
>>> that code to handle ASSERT_EXPRs is pretty easy.  This gives us more chances
>>> to extract simple equivalences from the condition in ASSERT_EXPRs.
>>>
>>> This patch fixes those 2 problems.  I don't think it directly helps pr78496
>>> right now as we're unable to pick up the newly exposed jump threads until
>>> VRP2.   But that's a short term limitation that I've already addressed
>>> locally :-)
>>>
>>> Bootstrapped, regression tested and installed on the trunk.
>>
>>
>> After this patch on aarch64-linux-gnu I get a bootstrap failure while
>> linking lto1/cc1/cc1plus/go1 in stage2:
>> godump.o: In function `go_define(unsigned int, char const*)':
>> godump.c:(.text+0x36c): relocation truncated to fit:
>> R_AARCH64_ADR_PREL_LO21 against `.rodata'
>> godump.c:(.text+0x4b4): relocation truncated to fit:
>> R_AARCH64_ADR_PREL_LO21 against `.rodata'
>> collect2: error: ld returned 1 exit status
>> ../../gcc/gcc/lto/Make-lang.in:81: recipe for target 'lto1' failed
>> make[3]: *** [lto1] Error 1
> 
> I should mention this is even after the patch to
> simplify_assert_expr_using_ranges .
Just spun up an aarch64 machine for testing and it's failing for me too. 
   I'll revert the patch and dig in.

jeff

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-07 17:32     ` Jeff Law
@ 2017-05-08  3:20       ` Andrew Pinski
  2017-05-08  4:30         ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Pinski @ 2017-05-08  3:20 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Sun, May 7, 2017 at 8:06 AM, Jeff Law <law@redhat.com> wrote:
> On 05/06/2017 05:56 PM, Andrew Pinski wrote:
>>
>> On Sat, May 6, 2017 at 4:55 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>
>>> On Sat, May 6, 2017 at 8:03 AM, Jeff Law <law@redhat.com> wrote:
>>>>
>>>> This is the 2nd of 3-5 patches to address pr78496.
>>>>
>>>> Jump threading will examine ASSERT_EXPRs as it walks the IL and record
>>>> information from those ASSERT_EXPRs into the available expression and
>>>> const/copies tables where they're later used to simplify conditionals.
>>>>
>>>> We're missing a couple things though.
>>>>
>>>> First an ASSERT_EXPR with an EQ test creates an equality we can enter
>>>> into
>>>> the const/copy tables.  We were failing to do that.  This is most
>>>> interesting when the RHS of the condition in the ASSERT_EXPR is a
>>>> constant,
>>>> obviously.  This has a secondary benefit of doing less work to get
>>>> better
>>>> optimization.
>>>>
>>>> Second, some ASSERT_EXPRs may start off as a relational test such as x
>>>> <= 0,
>>>> but after range extraction and propagation they could be simplified into
>>>> an
>>>> equality comparison.  We already do this with conditionals and
>>>> generalizing
>>>> that code to handle ASSERT_EXPRs is pretty easy.  This gives us more
>>>> chances
>>>> to extract simple equivalences from the condition in ASSERT_EXPRs.
>>>>
>>>> This patch fixes those 2 problems.  I don't think it directly helps
>>>> pr78496
>>>> right now as we're unable to pick up the newly exposed jump threads
>>>> until
>>>> VRP2.   But that's a short term limitation that I've already addressed
>>>> locally :-)
>>>>
>>>> Bootstrapped, regression tested and installed on the trunk.
>>>
>>>
>>>
>>> After this patch on aarch64-linux-gnu I get a bootstrap failure while
>>> linking lto1/cc1/cc1plus/go1 in stage2:
>>> godump.o: In function `go_define(unsigned int, char const*)':
>>> godump.c:(.text+0x36c): relocation truncated to fit:
>>> R_AARCH64_ADR_PREL_LO21 against `.rodata'
>>> godump.c:(.text+0x4b4): relocation truncated to fit:
>>> R_AARCH64_ADR_PREL_LO21 against `.rodata'
>>> collect2: error: ld returned 1 exit status
>>> ../../gcc/gcc/lto/Make-lang.in:81: recipe for target 'lto1' failed
>>> make[3]: *** [lto1] Error 1
>>
>>
>> I should mention this is even after the patch to
>> simplify_assert_expr_using_ranges .
>
> Just spun up an aarch64 machine for testing and it's failing for me too.
> I'll revert the patch and dig in.

Looks like I misread my regression hunter output.  This patch was not
the cause.  I will write to the person who actually caused it.

Thanks,
Andrew


>
> jeff

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-08  3:20       ` Andrew Pinski
@ 2017-05-08  4:30         ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2017-05-08  4:30 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On 05/07/2017 09:18 PM, Andrew Pinski wrote:
> On Sun, May 7, 2017 at 8:06 AM, Jeff Law <law@redhat.com> wrote:
>> On 05/06/2017 05:56 PM, Andrew Pinski wrote:
>>>
>>> On Sat, May 6, 2017 at 4:55 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>>
>>>> On Sat, May 6, 2017 at 8:03 AM, Jeff Law <law@redhat.com> wrote:
>>>>>
>>>>> This is the 2nd of 3-5 patches to address pr78496.
>>>>>
>>>>> Jump threading will examine ASSERT_EXPRs as it walks the IL and record
>>>>> information from those ASSERT_EXPRs into the available expression and
>>>>> const/copies tables where they're later used to simplify conditionals.
>>>>>
>>>>> We're missing a couple things though.
>>>>>
>>>>> First an ASSERT_EXPR with an EQ test creates an equality we can enter
>>>>> into
>>>>> the const/copy tables.  We were failing to do that.  This is most
>>>>> interesting when the RHS of the condition in the ASSERT_EXPR is a
>>>>> constant,
>>>>> obviously.  This has a secondary benefit of doing less work to get
>>>>> better
>>>>> optimization.
>>>>>
>>>>> Second, some ASSERT_EXPRs may start off as a relational test such as x
>>>>> <= 0,
>>>>> but after range extraction and propagation they could be simplified into
>>>>> an
>>>>> equality comparison.  We already do this with conditionals and
>>>>> generalizing
>>>>> that code to handle ASSERT_EXPRs is pretty easy.  This gives us more
>>>>> chances
>>>>> to extract simple equivalences from the condition in ASSERT_EXPRs.
>>>>>
>>>>> This patch fixes those 2 problems.  I don't think it directly helps
>>>>> pr78496
>>>>> right now as we're unable to pick up the newly exposed jump threads
>>>>> until
>>>>> VRP2.   But that's a short term limitation that I've already addressed
>>>>> locally :-)
>>>>>
>>>>> Bootstrapped, regression tested and installed on the trunk.
>>>>
>>>>
>>>>
>>>> After this patch on aarch64-linux-gnu I get a bootstrap failure while
>>>> linking lto1/cc1/cc1plus/go1 in stage2:
>>>> godump.o: In function `go_define(unsigned int, char const*)':
>>>> godump.c:(.text+0x36c): relocation truncated to fit:
>>>> R_AARCH64_ADR_PREL_LO21 against `.rodata'
>>>> godump.c:(.text+0x4b4): relocation truncated to fit:
>>>> R_AARCH64_ADR_PREL_LO21 against `.rodata'
>>>> collect2: error: ld returned 1 exit status
>>>> ../../gcc/gcc/lto/Make-lang.in:81: recipe for target 'lto1' failed
>>>> make[3]: *** [lto1] Error 1
>>>
>>>
>>> I should mention this is even after the patch to
>>> simplify_assert_expr_using_ranges .
>>
>> Just spun up an aarch64 machine for testing and it's failing for me too.
>> I'll revert the patch and dig in.
> 
> Looks like I misread my regression hunter output.  This patch was not
> the cause.  I will write to the person who actually caused it.
> 
> Thanks,
Funny, I literally just came to the conclusion it was Richard S's LRA 
patch :-)

jeff

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-06 15:33 [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading Jeff Law
  2017-05-06 18:07 ` Trevor Saunders
  2017-05-06 23:56 ` Andrew Pinski
@ 2017-05-08  7:33 ` Richard Biener via gcc-patches
  2017-05-08 15:56   ` Jeff Law
  2 siblings, 1 reply; 11+ messages in thread
From: Richard Biener via gcc-patches @ 2017-05-08  7:33 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Sat, May 6, 2017 at 5:03 PM, Jeff Law <law@redhat.com> wrote:
> This is the 2nd of 3-5 patches to address pr78496.
>
> Jump threading will examine ASSERT_EXPRs as it walks the IL and record
> information from those ASSERT_EXPRs into the available expression and
> const/copies tables where they're later used to simplify conditionals.
>
> We're missing a couple things though.
>
> First an ASSERT_EXPR with an EQ test creates an equality we can enter into
> the const/copy tables.  We were failing to do that.  This is most
> interesting when the RHS of the condition in the ASSERT_EXPR is a constant,
> obviously.  This has a secondary benefit of doing less work to get better
> optimization.
>
> Second, some ASSERT_EXPRs may start off as a relational test such as x <= 0,
> but after range extraction and propagation they could be simplified into an
> equality comparison.  We already do this with conditionals and generalizing
> that code to handle ASSERT_EXPRs is pretty easy.  This gives us more chances
> to extract simple equivalences from the condition in ASSERT_EXPRs.
>
> This patch fixes those 2 problems.  I don't think it directly helps pr78496
> right now as we're unable to pick up the newly exposed jump threads until
> VRP2.   But that's a short term limitation that I've already addressed
> locally :-)
>
> Bootstrapped, regression tested and installed on the trunk.
>
> jeff
>
>
> ps. An astute observer might note that improving the effectiveness of VRP
> jump threading seems counterproductive since I've stated I want to remove
> VRP jump threading.  These improvements don't significantly change how I was
> planning to do that or the infrastructure we're going to rely upon to make
> that change.  All that changes is where we get the information from --
> ASSERT_EXPRs vs querying a new API that generates the same information as
> needed.

That API is already there.  It's called register_edge_assert_for (it might need
a wrapper to be most useful and also needs to be exported / moved from VRP
to somewhere else).  Both VRP and EVRP use it to "compute" ASSERT_EXPRs.

So I'm not sure if changing VRP with your patches is a good thing when you
could have used the new API in the first place ...

Richard.

>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index b0c253b09ae..0f78f2a2ed1 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2017-05-06  Jeff Law  <law@redhat.com>
> +
> +       PR tree-optimization/78496
> +       * tree-vrp.c (simplify_assert_expr_using_ranges): New function.
> +       (simplify_stmt_using_ranges): Call it.
> +       (vrp_dom_walker::before_dom_children): Extract equivalences
> +       from an ASSERT_EXPR with an equality comparison against a
> +       constant.
> +
>  2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
>
>         * lra-constraints.c (lra_copy_reg_equiv): New function.
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index fca5b87e798..42782a6d17c 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-05-06  Jeff Law  <law@redhat.com>
> +
> +       PR tree-optimization/78496
> +       * gcc.dg/tree-ssa/ssa-thread-16.c: New test.
> +       * gcc.dg/tree-ssa/ssa-thread-17.c: New test.
> +
>  2017-05-06  Richard Sandiford  <richard.sandiford@linaro.org>
>
>         * gcc.target/aarch64/spill_1.c: New test.
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
> new file mode 100644
> index 00000000000..78c349ca14d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-16.c
> @@ -0,0 +1,38 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
> +
> +/* We should thread the if (exp == 2) conditional on the
> +   the path from inside the if (x) THEN arm.  It is the only
> +   jump threading opportunity in this code.  */
> +
> +/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
> +
> +
> +extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
> +  __attribute__ ((__noreturn__));
> +
> +int x;
> +
> +
> +int code;
> +void
> +do_jump (int exp)
> +{
> +  switch (code)
> +    {
> +    case 4:
> +      if ((exp) == 1)
> +       goto normal;
> +      if (x)
> +       {
> +         if (exp != 0)
> +           abort ();
> +       }
> +      if ((exp) == 2)
> +       goto normal;
> +    case 3:
> +       abort ();
> +    }
> +  normal:
> +      ;
> +}
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
> new file mode 100644
> index 00000000000..692658fbb4b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-17.c
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
> +
> +/* We should simplify one ASSERT_EXPR from a relational
> +   into an equality test.  */
> +/* { dg-final { scan-tree-dump-times "Folded
> into:\[^\r\n\]*ASSERT_EXPR\*\[^\r\n\]* == 1" 1 "vrp1" } } */
> +
> +/* And simplification of the ASSERT_EXPR leads to a jump threading
> opportunity.  */
> +/* { dg-final { scan-tree-dump-times "Threaded" 1 "vrp1" } } */
> +
> +extern void abort (void) __attribute__ ((__nothrow__, __leaf__))
> +  __attribute__ ((__noreturn__));
> +
> +union gimple_statement_d;
> +typedef union gimple_statement_d *gimple;
> +
> +
> +
> +union gimple_statement_d
> +{
> +  unsigned num_ops;
> +};
> +
> +void
> +gimple_assign_set_rhs_with_ops_1 (int code, gimple stmt, unsigned
> new_rhs_ops)
> +{
> +
> +  stmt->num_ops = new_rhs_ops + 1;
> +  if (stmt->num_ops <= 1)
> +    abort ();
> +  if (new_rhs_ops > 1)
> +    if (stmt->num_ops <= 2)
> +      abort ();
> +  if (new_rhs_ops > 2)
> +      abort ();
> +}
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index cf50e908257..461a48f5c9c 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -9600,6 +9600,43 @@ range_fits_type_p (value_range *vr, unsigned
> dest_precision, signop dest_sgn)
>    return true;
>  }
>
> +/* Simplify STMT, an ASSERT_EXPR, using ranges.  This is helpful because
> jump
> +   threading looks at the ASSERT_EXPRs.  Collapsing the condition of
> +   an ASSERT_EXPR from a relational to an equality test is where most
> +   of the benefit occurrs, so that's the only thing we currently do.  */
> +
> +static bool
> +simplify_assert_expr_using_ranges (gimple *stmt)
> +{
> +  return false;
> +  tree cond = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
> +  tree_code code = TREE_CODE (cond);
> +  tree op0 = TREE_OPERAND (cond, 0);
> +
> +  /* The condition of the ASSERT_EXPR must be a simple relational
> +     between an SSA_NAME (with a range) and a constant.  */
> +  if (TREE_CODE (op0) != SSA_NAME
> +      || !INTEGRAL_TYPE_P (TREE_TYPE (op0)))
> +    return false;
> +
> +  tree op1 = TREE_OPERAND (cond, 1);
> +  if (TREE_CODE (op1) != INTEGER_CST)
> +    return false;
> +
> +  value_range *vr = get_value_range (op0);
> +  if (!vr || vr->type != VR_RANGE)
> +    return false;
> +
> +  tree res = test_for_singularity (code, op0, op1, vr);
> +  if (res)
> +    {
> +      TREE_SET_CODE (cond, EQ_EXPR);
> +      TREE_OPERAND (cond, 1) = res;
> +      return true;
> +    }
> +  return false;
> +}
> +
>  /* Simplify a conditional using a relational operator to an equality
>     test if the range information indicates only one value can satisfy
>     the original conditional.  */
> @@ -10334,6 +10371,9 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
> *gsi)
>         case MAX_EXPR:
>           return simplify_min_or_max_using_ranges (gsi, stmt);
>
> +       case ASSERT_EXPR:
> +         return simplify_assert_expr_using_ranges (stmt);
> +
>         default:
>           break;
>         }
> @@ -10598,6 +10638,18 @@ vrp_dom_walker::before_dom_children (basic_block
> bb)
>         {
>           tree rhs1 = gimple_assign_rhs1 (stmt);
>           tree cond = TREE_OPERAND (rhs1, 1);
> +         tree lhs = gimple_assign_lhs (stmt);
> +         m_const_and_copies->record_const_or_copy (lhs, TREE_OPERAND (rhs1,
> 0));
> +
> +         if (TREE_CODE (cond) == EQ_EXPR)
> +           {
> +             tree cond_op0 = TREE_OPERAND (cond, 0);
> +             tree cond_op1 = TREE_OPERAND (cond, 1);
> +             if (TREE_CODE (cond_op0) == SSA_NAME)
> +               m_const_and_copies->record_const_or_copy (cond_op0,
> cond_op1);
> +             continue;
> +           }
> +
>           tree inverted = invert_truthvalue (cond);
>           vec<cond_equivalence> p;
>           p.create (3);
> @@ -10605,9 +10657,6 @@ vrp_dom_walker::before_dom_children (basic_block bb)
>           for (unsigned int i = 0; i < p.length (); i++)
>             m_avail_exprs_stack->record_cond (&p[i]);
>
> -         tree lhs = gimple_assign_lhs (stmt);
> -         m_const_and_copies->record_const_or_copy (lhs,
> -                                                   TREE_OPERAND (rhs1, 0));
>           p.release ();
>           continue;
>         }
>

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-08  7:33 ` Richard Biener via gcc-patches
@ 2017-05-08 15:56   ` Jeff Law
  2017-05-08 20:31     ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2017-05-08 15:56 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On 05/08/2017 01:25 AM, Richard Biener via gcc-patches wrote:
>>
>>
>> ps. An astute observer might note that improving the effectiveness of VRP
>> jump threading seems counterproductive since I've stated I want to remove
>> VRP jump threading.  These improvements don't significantly change how I was
>> planning to do that or the infrastructure we're going to rely upon to make
>> that change.  All that changes is where we get the information from --
>> ASSERT_EXPRs vs querying a new API that generates the same information as
>> needed.
> 
> That API is already there.  It's called register_edge_assert_for (it might need
> a wrapper to be most useful and also needs to be exported / moved from VRP
> to somewhere else).  Both VRP and EVRP use it to "compute" ASSERT_EXPRs
As I mentioned in an earlier message, this has a lot of similarity with 
what Andrew has been doing.  Furthermore, parts of this mimick code I've 
written in DOM and code we need for the backward threader to make it 
strong enough to eliminate the VRP threading code.  I'd really like to 
be working on unifying all that work within the next few days (as 
originally hoping for today, but slightly side-tracked last week).



> So I'm not sure if changing VRP with your patches is a good thing when you
> could have used the new API in the first place ...
I don't see that the changes to date around 78496 change things 
significantly in regards to the immediate plans to remove ASSERT_EXPR. 
The work around 78496 raises the bar in terms of what information we 
need to be able to extract, but that IMHO, is a fine thing to do ;-)


However, the existence of register_edge_assert_for does change how I'm 
looking at the next issue for 78496 as well as how to tackle a host of 
related issues.  It may end up being the case that we stop 78496 work 
after patch #2, work on ASSERT_EXPR removal, then re-eval 78496.

Jeff

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

* Re: [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading
  2017-05-08 15:56   ` Jeff Law
@ 2017-05-08 20:31     ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2017-05-08 20:31 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On 05/08/2017 09:54 AM, Jeff Law wrote:
> So I'm not sure if changing VRP with your patches is a good thing when 
>> you
>> could have used the new API in the first place ...
> I don't see that the changes to date around 78496 change things 
> significantly in regards to the immediate plans to remove ASSERT_EXPR. 
> The work around 78496 raises the bar in terms of what information we 
> need to be able to extract, but that IMHO, is a fine thing to do ;-)
As expected, it's pretty easy to change to the newer way of doing 
things.  ie, extracting from the GIMPLE_COND rather than the 
ASSERT_EXPR.   It really isn't a big deal.


> 
> 
> However, the existence of register_edge_assert_for does change how I'm 
> looking at the next issue for 78496 as well as how to tackle a host of 
> related issues.  It may end up being the case that we stop 78496 work 
> after patch #2, work on ASSERT_EXPR removal, then re-eval 78496.
And as expected the unwindable VRs do help significantly in the 3rd hunk 
of the work for 78496.  Probably the biggest issue here is how to 
compose the bits to avoid code duplication between EVRP and the VRP jump 
threading.

Jeff

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

end of thread, other threads:[~2017-05-08 20:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-06 15:33 [PATCH 2/n] [PR tree-optimization/78496] Simplify ASSERT_EXPRs to facilitate jump threading Jeff Law
2017-05-06 18:07 ` Trevor Saunders
2017-05-06 20:17   ` Jeff Law
2017-05-06 23:56 ` Andrew Pinski
2017-05-07  9:10   ` Andrew Pinski
2017-05-07 17:32     ` Jeff Law
2017-05-08  3:20       ` Andrew Pinski
2017-05-08  4:30         ` Jeff Law
2017-05-08  7:33 ` Richard Biener via gcc-patches
2017-05-08 15:56   ` Jeff Law
2017-05-08 20:31     ` 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).