diff mbox

[EVRP] Fold stmts with vrp_fold_stmt

Message ID 1ceb9248-9146-ff56-d914-5abe6deea0fa@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah Oct. 5, 2016, 5:58 a.m. UTC
Hi Richard,
Thanks for the review.

On 04/10/16 19:56, Richard Biener wrote:
> On Tue, 4 Oct 2016, kugan wrote:
>
>> Hi,
>>
>> This patch improves Early VRP by folding stmts using vrp_fold_stmt as it is
>> done in ssa_propagate for VRP.
>
> Why?

I thought it would be good for early vrp to simplify stmts using ranges. 
If we simplify obvious cases early, wouldn't it be better for IPA/LTO?

   I'd like us to move away from the fold_stmt callback of
> substitute-and-fold (I have actually started some work towards that).

I must have missed it. But what is the general issue with 
substitute-and-fold.

>
>> I have also changed EVRP to handle POINTER_TYPE_P. I will send follow up
>> patches to use this in IPA-VRP.
>
> For pointers all VRP does is track non-NULLness.  Can you split out this
> part?
Attached patch does that.

> I'm really worried about all the testsuite changes -- it means we are
> losing test coverage for VRP :/

As you said earlier, unit testing with gimple FE should help. I am also 
wondering if we should organize these testcases such that it is run once 
without evrp and once with evrp to test both?

Thanks,
Kugan

gcc/ChangeLog:

2016-10-05  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
	  POINTER_TYPE_P.


gcc/testsuite/ChangeLog:

2016-10-05  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* gcc.dg/tree-ssa/evrp4.c: New test.

>
> Richard.
>
>> Bootstrapped and regression testd on x86_64-linux-gnu with no new regressions.
>> Is this OK for trunk?
>>
>> Thanks,
>> Kugan
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-03  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>> 	* gcc.dg/pr68217.c: Adjust testcase as more cases are now handled in
>> 	  evrp.
>> 	* gcc.dg/predict-1.c: Likewise.
>> 	* gcc.dg/predict-9.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr20318.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr21001.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr21090.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr21294.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr21559.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr21563.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr23744.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr25382.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr61839_1.c: Likewise.
>> 	* gcc.dg/tree-ssa/pr68431.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp03.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp07.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp09.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp17.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp18.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp19.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp20.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp23.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp24.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp58.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp92.c: Likewise.
>> 	* gcc.dg/tree-ssa/vrp98.c: Likewise.
>> 	* gcc.dg/vrp-min-max-1.c: Likewise.
>>
>> gcc/ChangeLog:
>>
>> 2016-10-03  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>> 	* tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
>> 	  POINTER_TYPE_P. Also fold stmts with vrp_fold_stmt.
>>
>

Comments

Richard Biener Oct. 5, 2016, 8:50 a.m. UTC | #1
On Wed, 5 Oct 2016, kugan wrote:

> Hi Richard,
> Thanks for the review.
> 
> On 04/10/16 19:56, Richard Biener wrote:
> > On Tue, 4 Oct 2016, kugan wrote:
> > 
> > > Hi,
> > > 
> > > This patch improves Early VRP by folding stmts using vrp_fold_stmt as it
> > > is
> > > done in ssa_propagate for VRP.
> > 
> > Why?
> 
> I thought it would be good for early vrp to simplify stmts using ranges. If we
> simplify obvious cases early, wouldn't it be better for IPA/LTO?
> 
>   I'd like us to move away from the fold_stmt callback of
> > substitute-and-fold (I have actually started some work towards that).
> 
> I must have missed it. But what is the general issue with 
> substitute-and-fold.

The pass specific stmt folding hook is a distraction and causes extra
work.  Most of the things the CCP variant does are no longer necessary
for example.  First and foremost I'm trying to enable "dce" in
substitute-and-fold for VRP and handle ASSERT_EXPR "removal" by
means of propagation -- this should fix quite a few missed pattern
matchings with fold_stmt which when running into ASSERT_EXPRs just
give up.

> > 
> > > I have also changed EVRP to handle POINTER_TYPE_P. I will send follow up
> > > patches to use this in IPA-VRP.
> > 
> > For pointers all VRP does is track non-NULLness.  Can you split out this
> > part?
> Attached patch does that.

That patch is ok.

> > I'm really worried about all the testsuite changes -- it means we are
> > losing test coverage for VRP :/
> 
> As you said earlier, unit testing with gimple FE should help. I am also
> wondering if we should organize these testcases such that it is run once
> without evrp and once with evrp to test both?

Yeah, that would be nice.  I always wondered about adding the ability
to add a "local torture" to individual testcases, say

{ dg-do compile }
{ dg-torture-options { "-O2 -fdisable-tree-evrp" "-O2" } }

(and make it be "additional" torture options when in dg-torture.exp)

Requires TCL hacking which isn't exactly my favourite...

Alternatively (seen elsewhere) put the testcase in a header and
have two testcases include it, with different dg-options ...

Richard.

> Thanks,
> Kugan
> 
> gcc/ChangeLog:
> 
> 2016-10-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
> 	  POINTER_TYPE_P.
> 
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-10-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* gcc.dg/tree-ssa/evrp4.c: New test.
> 
> > 
> > Richard.
> > 
> > > Bootstrapped and regression testd on x86_64-linux-gnu with no new
> > > regressions.
> > > Is this OK for trunk?
> > > 
> > > Thanks,
> > > Kugan
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 2016-10-03  Kugan Vivekanandarajah  <kuganv@linaro.org>
> > > 
> > > 	* gcc.dg/pr68217.c: Adjust testcase as more cases are now handled in
> > > 	  evrp.
> > > 	* gcc.dg/predict-1.c: Likewise.
> > > 	* gcc.dg/predict-9.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr20318.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr21001.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr21090.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr21294.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr21559.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr21563.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr23744.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr25382.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr61839_1.c: Likewise.
> > > 	* gcc.dg/tree-ssa/pr68431.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp03.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp07.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp09.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp17.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp18.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp19.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp20.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp23.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp24.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp58.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp92.c: Likewise.
> > > 	* gcc.dg/tree-ssa/vrp98.c: Likewise.
> > > 	* gcc.dg/vrp-min-max-1.c: Likewise.
> > > 
> > > gcc/ChangeLog:
> > > 
> > > 2016-10-03  Kugan Vivekanandarajah  <kuganv@linaro.org>
> > > 
> > > 	* tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
> > > 	  POINTER_TYPE_P. Also fold stmts with vrp_fold_stmt.
> > > 
> > 
>
diff mbox

Patch

From c9badd0cee1433af67ba5e1a45a90b4b659a244f Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Mon, 3 Oct 2016 06:12:05 +1100
Subject: [PATCH 1/5] Handle pointer type in evrp

---
 gcc/testsuite/gcc.dg/tree-ssa/evrp4.c | 20 ++++++++++++++++++++
 gcc/tree-vrp.c                        |  3 ++-
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/evrp4.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/evrp4.c b/gcc/testsuite/gcc.dg/tree-ssa/evrp4.c
new file mode 100644
index 0000000..ebb87ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/evrp4.c
@@ -0,0 +1,20 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp" } */
+
+int foo (int *p);
+
+struct st
+{
+  int a;
+  int b;
+};
+
+int bar (struct st *s)
+{
+
+  if (!s)
+    return 0;
+  foo (&s->a);
+}
+
+/* { dg-final { scan-tree-dump "\~\\\[0B, 0B\\\]" "evrp" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 7a08be7..46bbd82 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10666,7 +10666,8 @@  evrp_dom_walker::before_dom_children (basic_block bb)
 	  && gimple_code (stmt) == GIMPLE_COND
 	  && (op0 = gimple_cond_lhs (stmt))
 	  && TREE_CODE (op0) == SSA_NAME
-	  && INTEGRAL_TYPE_P (TREE_TYPE (gimple_cond_lhs (stmt))))
+	  && (INTEGRAL_TYPE_P (TREE_TYPE (gimple_cond_lhs (stmt)))
+	      || POINTER_TYPE_P (TREE_TYPE (gimple_cond_lhs (stmt)))))
 	{
 	  /* Entering a new scope.  Try to see if we can find a VR
 	     here.  */
-- 
2.7.4