[PATCH 2/3][POPCOUNT] Check if zero check is done before entering the loop

[PATCH 2/3][POPCOUNT] Check if zero check is done before entering the loop

[Kugan Vivekanandarajah]

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

gcc/ChangeLog:

2018-06-22  Kugan Vivekanandarajah  <kuganv@linaro.org>

    * tree-ssa-loop-niter.c (number_of_iterations_popcount): If popcount
    argument is checked for zero before entering loop, avoid checking again.

[-- Attachment #2: 0002-in-niter-dont-check-for-zero-when-it-is-alrealy-chec.patch --]
[-- Type: text/x-patch, Size: 2174 bytes --]

From 4f2a6ad5a49eec0a1cae15e033329f889f9137b9 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 22 Jun 2018 14:11:28 +1000
Subject: [PATCH 2/3] in niter dont check for zero when it is alrealy checked

Change-Id: I98982537bca14cb99a85d0da70d33a6c044385fd
---
 gcc/tree-ssa-loop-niter.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index 9365915..2299aca 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -2503,6 +2503,7 @@ number_of_iterations_popcount (loop_p loop, edge exit,
   HOST_WIDE_INT max;
   adjust = true;
   tree fn = NULL_TREE;
+  bool check_zero = true;
 
   /* Check loop terminating branch is like
      if (b != 0).  */
@@ -2590,7 +2591,37 @@ number_of_iterations_popcount (loop_p loop, edge exit,
 
   niter->niter = iter;
   niter->assumptions = boolean_true_node;
-  if (adjust)
+  if (adjust
+      && EDGE_COUNT (loop->header->preds) == 2)
+    {
+      /* Sometimes, src of the popcount is checked for
+	 zero before entering the loop.  In this case we
+	 dont need to check for zero again.  */
+      edge pred_edge = EDGE_PRED (loop->header, 0);
+      gimple *stmt = last_stmt (pred_edge->src);
+
+      /* If there is an empty pre-header, go one block
+	 above.  */
+      if (!stmt
+	  && EDGE_COUNT (pred_edge->src->preds) == 1)
+	{
+	  pred_edge = EDGE_PRED (pred_edge->src, 0);
+	  stmt = last_stmt (pred_edge->src);
+	}
+
+      /* If we have the src != 0 check and if we are entering
+	 the loop when the condition is true, we can skip zero
+	 check.  */
+      if (stmt
+	  && gimple_code (stmt) == GIMPLE_COND
+	  && gimple_cond_code (stmt) == NE_EXPR
+	  && pred_edge->flags & EDGE_TRUE_VALUE
+	  && TREE_CODE (gimple_cond_lhs (stmt)) == SSA_NAME
+	  && (gimple_phi_arg_def (phi, loop_preheader_edge (loop)->dest_idx)
+	      == gimple_cond_lhs (stmt)))
+	check_zero = false;
+    }
+  if (adjust && check_zero)
     niter->may_be_zero = fold_build2 (EQ_EXPR, boolean_type_node, src,
 				      build_zero_cst
 				      (TREE_TYPE (src)));
-- 
2.7.4

Re: [PATCH 2/3][POPCOUNT] Check if zero check is done before entering the loop

[Richard Biener]

On Fri, Jun 22, 2018 at 11:14 AM Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
>
> gcc/ChangeLog:

The canonical way is calling simplify_using_initial_conditions on the
may_be_zero condition.

Richard.

> 2018-06-22  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>     * tree-ssa-loop-niter.c (number_of_iterations_popcount): If popcount
>     argument is checked for zero before entering loop, avoid checking again.

Re: [PATCH 2/3][POPCOUNT] Check if zero check is done before entering the loop

[Kugan Vivekanandarajah]

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

Hi Richard,

Thanks for the review.

On 25 June 2018 at 20:02, Richard Biener <richard.guenther@gmail.com> wrote:
> On Fri, Jun 22, 2018 at 11:14 AM Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>>
>> gcc/ChangeLog:
>
> The canonical way is calling simplify_using_initial_conditions on the
> may_be_zero condition.
>
> Richard.
>
>> 2018-06-22  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>     * tree-ssa-loop-niter.c (number_of_iterations_popcount): If popcount
>>     argument is checked for zero before entering loop, avoid checking again.
Do you like the attached patch which does this.

Thanks,
Kugan

[-- Attachment #2: 0002-in-niter-dont-check-for-zero-when-it-is-alrealy-chec.patch --]
[-- Type: text/x-patch, Size: 1090 bytes --]

From 78cb0ea3d058f1d1db73f259825b8bb07eb1ca30 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 22 Jun 2018 14:11:28 +1000
Subject: [PATCH 2/3] in niter dont check for zero when it is alrealy checked

Change-Id: Ie94a35a1a3c2d8bdffd3dc54a94684c032efc7e0
---
 gcc/tree-ssa-loop-niter.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index f5ffc0f..be0cff5 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -2596,10 +2596,15 @@ number_of_iterations_popcount (loop_p loop, edge exit,
 
   niter->niter = iter;
   niter->assumptions = boolean_true_node;
+
   if (adjust)
-    niter->may_be_zero = fold_build2 (EQ_EXPR, boolean_type_node, src,
+    {
+      tree may_be_zero = fold_build2 (EQ_EXPR, boolean_type_node, src,
 				      build_zero_cst
 				      (TREE_TYPE (src)));
+      niter->may_be_zero =
+	simplify_using_initial_conditions (loop, may_be_zero);
+    }
   else
     niter->may_be_zero = boolean_false_node;
 
-- 
2.7.4

Re: [PATCH 2/3][POPCOUNT] Check if zero check is done before entering the loop

[Richard Biener]

On Wed, Jun 27, 2018 at 7:01 AM Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
>
> Hi Richard,
>
> Thanks for the review.
>
> On 25 June 2018 at 20:02, Richard Biener <richard.guenther@gmail.com> wrote:
> > On Fri, Jun 22, 2018 at 11:14 AM Kugan Vivekanandarajah
> > <kugan.vivekanandarajah@linaro.org> wrote:
> >>
> >> gcc/ChangeLog:
> >
> > The canonical way is calling simplify_using_initial_conditions on the
> > may_be_zero condition.
> >
> > Richard.
> >
> >> 2018-06-22  Kugan Vivekanandarajah  <kuganv@linaro.org>
> >>
> >>     * tree-ssa-loop-niter.c (number_of_iterations_popcount): If popcount
> >>     argument is checked for zero before entering loop, avoid checking again.
> Do you like the attached patch which does this.

Yes.

OK if bootstrapped/tested.

Richard.

> Thanks,
> Kugan