Refactor gcc/tree-vectorize.c:vectorize_loops

Refactor gcc/tree-vectorize.c:vectorize_loops

[Aditya K]

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

Hi,
This is a small patch where I moved a portion of code from the function vectorize_loops outside to improve readability.
Please see the patch attached and give comments for improvements.

Thanks
-Aditya 		 	   		  

[-- Attachment #2: refactor-vect --]
[-- Type: application/octet-stream, Size: 2853 bytes --]

commit 4e19558ab32518b48f4752e22397a7ca26b8915b
Author: hiraditya <hiraditya@hotmail.com>
Date:   Tue Apr 28 20:57:40 2015 -0500

    Refactor vectorize-loops.c
    
    Outlined the code meant to set uids of all the instructions in loop bbs.
            * gcc/vectorize-loops.c

diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index 313e936..ac45774 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -399,6 +399,39 @@ fold_loop_vectorized_call (gimple g, tree value)
     }
 }
 
+static void
+set_uid_loop_bbs(loop_vec_info loop_vinfo, gimple loop_vectorized_call)
+{
+    tree arg = gimple_call_arg (loop_vectorized_call, 1);
+    basic_block *bbs;
+    unsigned int i;
+    struct loop *scalar_loop = get_loop (cfun, tree_to_shwi (arg));
+
+    LOOP_VINFO_SCALAR_LOOP (loop_vinfo) = scalar_loop;
+    gcc_checking_assert (vect_loop_vectorized_call
+                                (LOOP_VINFO_SCALAR_LOOP (loop_vinfo))
+                         == loop_vectorized_call);
+    bbs = get_loop_body (scalar_loop);
+    for (i = 0; i < scalar_loop->num_nodes; i++)
+      {
+        basic_block bb = bbs[i];
+        gimple_stmt_iterator gsi;
+        for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
+             gsi_next (&gsi))
+          {
+            gimple phi = gsi_stmt (gsi);
+            gimple_set_uid (phi, 0);
+          }
+        for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+             gsi_next (&gsi))
+          {
+            gimple stmt = gsi_stmt (gsi);
+            gimple_set_uid (stmt, 0);
+          }
+      }
+    free (bbs);
+}
+
 /* Function vectorize_loops.
 
    Entry point to loop vectorization phase.  */
@@ -461,36 +494,7 @@ vectorize_loops (void)
 
 	gimple loop_vectorized_call = vect_loop_vectorized_call (loop);
 	if (loop_vectorized_call)
-	  {
-	    tree arg = gimple_call_arg (loop_vectorized_call, 1);
-	    basic_block *bbs;
-	    unsigned int i;
-	    struct loop *scalar_loop = get_loop (cfun, tree_to_shwi (arg));
-
-	    LOOP_VINFO_SCALAR_LOOP (loop_vinfo) = scalar_loop;
-	    gcc_checking_assert (vect_loop_vectorized_call
-					(LOOP_VINFO_SCALAR_LOOP (loop_vinfo))
-				 == loop_vectorized_call);
-	    bbs = get_loop_body (scalar_loop);
-	    for (i = 0; i < scalar_loop->num_nodes; i++)
-	      {
-		basic_block bb = bbs[i];
-		gimple_stmt_iterator gsi;
-		for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
-		     gsi_next (&gsi))
-		  {
-		    gimple phi = gsi_stmt (gsi);
-		    gimple_set_uid (phi, 0);
-		  }
-		for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
-		     gsi_next (&gsi))
-		  {
-		    gimple stmt = gsi_stmt (gsi);
-		    gimple_set_uid (stmt, 0);
-		  }
-	      }
-	    free (bbs);
-	  }
+          set_uid_loop_bbs(loop_vinfo, loop_vectorized_call);
 
         if (LOCATION_LOCUS (vect_location) != UNKNOWN_LOCATION
 	    && dump_enabled_p ())

Re: Refactor gcc/tree-vectorize.c:vectorize_loops

[Jeff Law]

On 04/28/2015 08:20 PM, Aditya K wrote:
> Hi,
> This is a small patch where I moved a portion of code from the function vectorize_loops outside to improve readability.
> Please see the patch attached and give comments for improvements.
You need to add a comment for the new function.  These function comments 
should describe what the function does, in terms of its arguments and 
return value (if any).

With a function comment, this refactoring would be fine.

jeff

Re: Refactor gcc/tree-vectorize.c:vectorize_loops

[Jakub Jelinek]

On Tue, Apr 28, 2015 at 09:53:24PM -0600, Jeff Law wrote:
> On 04/28/2015 08:20 PM, Aditya K wrote:
> >Hi,
> >This is a small patch where I moved a portion of code from the function vectorize_loops outside to improve readability.
> >Please see the patch attached and give comments for improvements.
> You need to add a comment for the new function.  These function comments
> should describe what the function does, in terms of its arguments and return
> value (if any).
> 
> With a function comment, this refactoring would be fine.

--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -399,6 +399,39 @@ fold_loop_vectorized_call (gimple g, tree value)
     }
 }
 
+static void
+set_uid_loop_bbs(loop_vec_info loop_vinfo, gimple loop_vectorized_call)

The formatting is incorrect too, missing space before ( here.

+{
+    tree arg = gimple_call_arg (loop_vectorized_call, 1);

Lines should be indented by 2 spaces rather than after {

+    basic_block *bbs;
+    unsigned int i;
+    struct loop *scalar_loop = get_loop (cfun, tree_to_shwi (arg));
+
+    LOOP_VINFO_SCALAR_LOOP (loop_vinfo) = scalar_loop;
+    gcc_checking_assert (vect_loop_vectorized_call
+                                (LOOP_VINFO_SCALAR_LOOP (loop_vinfo))
+                         == loop_vectorized_call);
+    bbs = get_loop_body (scalar_loop);
+    for (i = 0; i < scalar_loop->num_nodes; i++)
+      {
+        basic_block bb = bbs[i];
+        gimple_stmt_iterator gsi;
+        for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
+             gsi_next (&gsi))

With the refactoring, this for should fit on one line.

+          {
+            gimple phi = gsi_stmt (gsi);
+            gimple_set_uid (phi, 0);
+          }
+        for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+             gsi_next (&gsi))

Likewise.

 	if (loop_vectorized_call)
-	  {
+          set_uid_loop_bbs(loop_vinfo, loop_vectorized_call);

Again, missing space before (.  And you are using only spaces
when a tab plus two spaces should be used.

	Jakub

RE: Refactor gcc/tree-vectorize.c:vectorize_loops

[Aditya K]

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


Thanks for the feedback. I have added comment and properly indented the code.

-Aditya

----------------------------------------
> Date: Wed, 29 Apr 2015 09:31:46 +0200
> From: jakub@redhat.com
> To: law@redhat.com
> CC: hiraditya@msn.com; gcc-patches@gcc.gnu.org
> Subject: Re: Refactor gcc/tree-vectorize.c:vectorize_loops
>
> On Tue, Apr 28, 2015 at 09:53:24PM -0600, Jeff Law wrote:
>> On 04/28/2015 08:20 PM, Aditya K wrote:
>>>Hi,
>>>This is a small patch where I moved a portion of code from the function vectorize_loops outside to improve readability.
>>>Please see the patch attached and give comments for improvements.
>> You need to add a comment for the new function. These function comments
>> should describe what the function does, in terms of its arguments and return
>> value (if any).
>>
>> With a function comment, this refactoring would be fine.
>
> --- a/gcc/tree-vectorizer.c
> +++ b/gcc/tree-vectorizer.c
> @@ -399,6 +399,39 @@ fold_loop_vectorized_call (gimple g, tree value)
> }
> }
>
> +static void
> +set_uid_loop_bbs(loop_vec_info loop_vinfo, gimple loop_vectorized_call)
>
> The formatting is incorrect too, missing space before ( here.
>
> +{
> + tree arg = gimple_call_arg (loop_vectorized_call, 1);
>
> Lines should be indented by 2 spaces rather than after {
>
> + basic_block *bbs;
> + unsigned int i;
> + struct loop *scalar_loop = get_loop (cfun, tree_to_shwi (arg));
> +
> + LOOP_VINFO_SCALAR_LOOP (loop_vinfo) = scalar_loop;
> + gcc_checking_assert (vect_loop_vectorized_call
> + (LOOP_VINFO_SCALAR_LOOP (loop_vinfo))
> + == loop_vectorized_call);
> + bbs = get_loop_body (scalar_loop);
> + for (i = 0; i < scalar_loop->num_nodes; i++)
> + {
> + basic_block bb = bbs[i];
> + gimple_stmt_iterator gsi;
> + for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
> + gsi_next (&gsi))
>
> With the refactoring, this for should fit on one line.
>
> + {
> + gimple phi = gsi_stmt (gsi);
> + gimple_set_uid (phi, 0);
> + }
> + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
> + gsi_next (&gsi))
>
> Likewise.
>
> if (loop_vectorized_call)
> - {
> + set_uid_loop_bbs(loop_vinfo, loop_vectorized_call);
>
> Again, missing space before (. And you are using only spaces
> when a tab plus two spaces should be used.
>
> Jakub
 		 	   		  

[-- Attachment #2: refactor-vectorize-loops.patch --]
[-- Type: application/octet-stream, Size: 2989 bytes --]

commit 07e787aa12ad774f939d4b6568f1abdb68b0f490
Author: Aditya Kumar <hiraditya@hotmail.com>
Date:   Wed Apr 29 09:30:25 2015 -0500

    Refactor vectorize-loops.c
    
        Outlined the code meant to set uids of all the instructions in loop bbs.
                * gcc/vectorize-loops.c

diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index 313e936..5691556 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -398,6 +398,39 @@ fold_loop_vectorized_call (gimple g, tree value)
       update_stmt (use_stmt);
     }
 }
+/* Set the uids of all the statements in basic blocks inside loop
+   represented by LOOP_VINFO. LOOP_VECTORIZED_CALL is the internal
+   call guarding the loop which has been if converted.  */
+static void
+set_uid_loop_bbs (loop_vec_info loop_vinfo, gimple loop_vectorized_call)
+{
+  tree arg = gimple_call_arg (loop_vectorized_call, 1);
+  basic_block *bbs;
+  unsigned int i;
+  struct loop *scalar_loop = get_loop (cfun, tree_to_shwi (arg));
+
+  LOOP_VINFO_SCALAR_LOOP (loop_vinfo) = scalar_loop;
+  gcc_checking_assert (vect_loop_vectorized_call
+			      (LOOP_VINFO_SCALAR_LOOP (loop_vinfo))
+		       == loop_vectorized_call);
+  bbs = get_loop_body (scalar_loop);
+  for (i = 0; i < scalar_loop->num_nodes; i++)
+    {
+      basic_block bb = bbs[i];
+      gimple_stmt_iterator gsi;
+      for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+        {
+          gimple phi = gsi_stmt (gsi);
+          gimple_set_uid (phi, 0);
+        }
+      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+        {
+          gimple stmt = gsi_stmt (gsi);
+          gimple_set_uid (stmt, 0);
+        }
+    }
+  free (bbs);
+}
 
 /* Function vectorize_loops.
 
@@ -461,37 +494,7 @@ vectorize_loops (void)
 
 	gimple loop_vectorized_call = vect_loop_vectorized_call (loop);
 	if (loop_vectorized_call)
-	  {
-	    tree arg = gimple_call_arg (loop_vectorized_call, 1);
-	    basic_block *bbs;
-	    unsigned int i;
-	    struct loop *scalar_loop = get_loop (cfun, tree_to_shwi (arg));
-
-	    LOOP_VINFO_SCALAR_LOOP (loop_vinfo) = scalar_loop;
-	    gcc_checking_assert (vect_loop_vectorized_call
-					(LOOP_VINFO_SCALAR_LOOP (loop_vinfo))
-				 == loop_vectorized_call);
-	    bbs = get_loop_body (scalar_loop);
-	    for (i = 0; i < scalar_loop->num_nodes; i++)
-	      {
-		basic_block bb = bbs[i];
-		gimple_stmt_iterator gsi;
-		for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
-		     gsi_next (&gsi))
-		  {
-		    gimple phi = gsi_stmt (gsi);
-		    gimple_set_uid (phi, 0);
-		  }
-		for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
-		     gsi_next (&gsi))
-		  {
-		    gimple stmt = gsi_stmt (gsi);
-		    gimple_set_uid (stmt, 0);
-		  }
-	      }
-	    free (bbs);
-	  }
-
+	  set_uid_loop_bbs (loop_vinfo, loop_vectorized_call);
         if (LOCATION_LOCUS (vect_location) != UNKNOWN_LOCATION
 	    && dump_enabled_p ())
           dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,

Re: Refactor gcc/tree-vectorize.c:vectorize_loops

[Jeff Law]

On 04/29/2015 08:37 AM, Aditya K wrote:
>
> Thanks for the feedback. I have added comment and properly indented the code.
I made a couple more formatting fixes (spaces -> tab & line wrapping), 
improved the ChangeLog, did a bootstrap & regression test on 
x86_64-linux-gnu and installed the final patch on the trunk.

Thanks,
Jeff

RE: Refactor gcc/tree-vectorize.c:vectorize_loops

[Aditya K]

Thank you very much Jeff.

-Aditya

----------------------------------------
> Date: Wed, 29 Apr 2015 23:44:20 -0600
> From: law@redhat.com
> To: hiraditya@msn.com; jakub@redhat.com
> CC: gcc-patches@gcc.gnu.org
> Subject: Re: Refactor gcc/tree-vectorize.c:vectorize_loops
>
> On 04/29/2015 08:37 AM, Aditya K wrote:
>>
>> Thanks for the feedback. I have added comment and properly indented the code.
> I made a couple more formatting fixes (spaces -> tab & line wrapping),
> improved the ChangeLog, did a bootstrap & regression test on
> x86_64-linux-gnu and installed the final patch on the trunk.
>
> Thanks,
> Jeff
>