Re: [PATCH] omp-low.c split

[Martin Jambor <mjambor@suse.cz>]

Hi,

On Fri, Dec 09, 2016 at 07:18:54PM +0300, Alexander Monakov wrote:
> On Fri, 9 Dec 2016, Jakub Jelinek wrote:
> > Can you post an incremental patch fixing those issues?
> 
> A few small nits I found while reading the patch.
> 
> First of all, please use 'git diff --patience' (or --histogram) when
> generating such patches, without it the changes in omp-low.c look uglier than
> necessary.

OK, the new patches use --patience, the result is actually quite a bit
smaller.  Thanks for the hint.

> 
> The comment for 'struct oacc_loop' in new file omp-device.c is misplaced: for
> some reason it's above the #include block.

Oh, thanks for noticing, fixed.

> 
> This patch doesn't seem to apply to current trunk due to a conflict in
> cp/parser.c.

Hmm, I don't think so?

  $ svn st
  $ svn up
  Updating '.':
  At revision 243600.
  $ patch --dry-run -p1 < /tmp/misc-next/pat/0001-Split-omp-low-into-multiple-files.patch
  checking file gcc/Makefile.in
  checking file gcc/c-family/c-omp.c
  checking file gcc/c/c-parser.c
  checking file gcc/c/c-typeck.c
  checking file gcc/c/gimple-parser.c
  checking file gcc/config/nvptx/nvptx.c
  checking file gcc/cp/parser.c
  checking file gcc/cp/semantics.c
  checking file gcc/fortran/trans-openmp.c
  checking file gcc/gengtype.c
  checking file gcc/gimple-fold.c
  checking file gcc/gimplify.c
  checking file gcc/lto-cgraph.c
  checking file gcc/omp-device.c
  checking file gcc/omp-device.h
  checking file gcc/omp-expand.c
  checking file gcc/omp-expand.h
  checking file gcc/omp-general.c
  checking file gcc/omp-general.h
  checking file gcc/omp-grid.c
  checking file gcc/omp-grid.h
  checking file gcc/omp-low.c
  checking file gcc/omp-low.h
  checking file gcc/toplev.c
  checking file gcc/tree-cfg.c
  checking file gcc/tree-parloops.c
  checking file gcc/tree-ssa-loop.c
  checking file gcc/tree-vrp.c
  checking file gcc/varpool.c
  $ echo $?
  0

This was using the exact same file I have compressed and sent a while
ago to the mailing list.

> If you could create a git branch (perhaps in your personal
> namespace so you can freely rebase it) with this patchset, I'd appreciate it.
> 

Well... I do not see this as a long-term project so I do not really
see much value in this.  I have used git just because that is how I
conveniently send patches to machines where I bootstrap them.  I hope
this will get committed very early so that we avoid any big conflicts
in omp-low.c.  But if this drags on for a while and if our git mirror
recovers meanwhile, I can.


> When trying to apply the patch, git notes a few remaining whitespace issues:
> 
> $ zcat 0001-Split-omp-low-into-multiple-files.patch.gz | git apply --reject -
> 
> <stdin>:2734: space before tab in indent.
>         # BLOCK 2 (PAR_ENTRY_BB)
> <stdin>:5346: space before tab in indent.
>                                 true, GSI_SAME_STMT);
> <stdin>:8129: space before tab in indent.
>              after a stmt, not before.  */
> <stdin>:9060: space before tab in indent.
>                                   GOMP_atomic_start ();
> <stdin>:9061: space before tab in indent.
>                                   *addr = rhs;
> 

I hope I have addressed this when doing what Jakub suggested, please
let me know if not.

Thanks for looking at the big patch!

Martin