On Mon, Dec 17, 2012 at 10:44:59PM -0800, Teresa Johnson wrote:
>Index: tree-ssa-loop-ivcanon.c
>===================================================================
>--- tree-ssa-loop-ivcanon.c    (revision 194516)
>+++ tree-ssa-loop-ivcanon.c    (working copy)
>@@ -639,22 +639,24 @@ unloop_loops (bitmap loop_closed_ssa_invalidated,
> 
> /* Tries to unroll LOOP completely, i.e. NITER times.
>    UL determines which loops we are allowed to unroll.
>-   EXIT is the exit of the loop that should be eliminated.  
>+   EXIT is the exit of the loop that should be eliminated.
>    MAXITER specfy bound on number of iterations, -1 if it is
>-   not known or too large for HOST_WIDE_INT.  */
>+   not known or too large for HOST_WIDE_INT. The location
>+   LOCUS corresponding to the loop is used when emitting
>+   a summary of the unroll to the dump file.  */
> 
> static bool
> try_unroll_loop_completely (struct loop *loop,
>                           edge exit, tree niter,
>                           enum unroll_level ul,
>-                          HOST_WIDE_INT maxiter)
>+                          HOST_WIDE_INT maxiter,
>+                            location_t locus)

whitespace damage?

>Index: loop-unroll.c
>===================================================================
>--- loop-unroll.c      (revision 194516)
>+++ loop-unroll.c      (working copy)
>@@ -148,6 +148,61 @@ static void combine_var_copies_in_loop_exit (struc
>                                            basic_block);
> static rtx get_expansion (struct var_to_expand *);
> 
>+/* Emit a message summarizing the unroll or peel that will be
>+   performed for LOOP, along with the loop's location LOCUS, if
>+   appropriate given the dump or -fopt-info settings.  */
>+
>+static void
>+report_unroll_peel(struct loop *loop, location_t locus)

missing space before (

contrib/check_GNU_style.sh generally says:
Dot, space, space, new sentence.
loop-dump.01.patch:223:+   not known or too large for HOST_WIDE_INT. The 
location
loop-dump.01.patch:514:+   * of the for or while statement, if possible. To do 
this, look

Dot, space, space, end of comment.
loop-dump.01.patch:504:+/* Return location corresponding to the loop control 
condition if possible. */
loop-dump.01.patch:541:+  /* Next check the latch, to see if it is non-empty. *
loop-dump.01.patch:555:+  /* If all else fails, simply return the current 
function location. */

There should be exactly one space between function name and parentheses.
loop-dump.01.patch:329:+report_unroll_peel(struct loop *loop, location_t locus)
loop-dump.01.patch:386:+      location_t locus = get_loop_location(loop);
loop-dump.01.patch:404:+          report_unroll_peel(loop, locus);
loop-dump.01.patch:412:+      location_t locus = get_loop_location(loop);
loop-dump.01.patch:429:+      report_unroll_peel(loop, locus);
loop-dump.01.patch:533:+  if ((exit = single_exit(loop)))

>@@ -248,6 +305,7 @@ peel_loops_completely (int flags)
> 
>       if (loop->lpt_decision.decision == LPT_PEEL_COMPLETELY)
>       {
>+          report_unroll_peel(loop, locus);
>         peel_loop_completely (loop);

whitespace damage? You seem to have this kind of whitespace error
throughout the patch. I take it you are aware of
http://gcc.gnu.org/wiki/FormattingCodeForGCC
and just forgot to have it on the machine you edited?

I seemingly have
$ cat ~/.vim/gcc_style.vim 
" put this plugin into ~/.vim/gcc_style.vim and source it into your ~/.vimrc via
" source ~/.vim/gcc_style.vim
if exists("g:loaded_gcc_style") || &cp
  finish
endif
let g:loaded_gcc_style = 1

augroup gcc_style
  autocmd BufReadPost,FileReadPost * call s:maybe_gcc_style()
augroup END
if exists("*s:maybe_gcc_style")
  finish
endif
let s:cpo_save = &cpo
set cpo&vim

function! s:maybe_gcc_style()
  let s:i = 1 + 0
  while s:i <= line("$") && s:i <= 25
    let s:line = getline(s:i)
    if s:line =~ '^\s*This\sfile\sis\spart\sof\sGCC.*'
      " gcc-mode
      set cino=:s,{s,n-s,>2s,^-s
      set sw=2
      set sts=2
      set cindent
      set smartindent
      set autoindent
      break
    else
      let s:i = s:i + 1
    endif
  endwhile
endfunction

"command! NoGCCstyle unlet! g:loaded_gcc_style | au! gcc_style
"command! DoGCCstyle runtime gcc_style.vim
let &cpo = s:cpo_save

>Index: cfgloop.c
>===================================================================
>--- cfgloop.c  (revision 194516)
>+++ cfgloop.c  (working copy)
>@@ -1666,3 +1666,59 @@ loop_exits_from_bb_p (struct loop *loop, basic_blo
> 
>   return false;
> }
>+
>+/* Return location corresponding to the loop control condition if possible. */
>+
>+location_t
>+get_loop_location (struct loop *loop)
>+{
>+  rtx insn = NULL;
>+  struct niter_desc *desc = NULL;
>+  edge exit;
>+
>+  /* For a for or while loop, we would like to return the location
>+   * of the for or while statement, if possible. To do this, look
>+   * for the branch guarding the loop back-edge.
>+   */

IIRC there is not supposed to be a * in comments.

Other than these nits i like it (but cannot approve it).

thanks,

Reply via email to