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

-Aditya

----------------------------------------
> Date: Wed, 29 Apr 2015 09:31:46 +0200
> From: ja...@redhat.com
> To: l...@redhat.com
> CC: hiradi...@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: refactor-vectorize-loops.patch
Description: Binary data

Reply via email to