Hi!

On Mon, 10 Feb 2014 00:42:54 +0100, Tobias Burnus <bur...@net-b.de> wrote:
> Some general questions to the patch set:

Thanks for helping review!


> * I miss "-fopenacc". Is the support already in the branch? I assume 
> that part is then in c-family/c.opt fortran/lang.opt?
> 
> * Documentation: Do you also need to update fortran/gfortran.texi and/or 
> fortran/invoke.texi? (I assume that -fopenacc is already documented in 
> docs/invoke.texi) [Search for openmp to find possible spots.]
> 
> * Intrinsic module "openacc" and "openacc_lib.h": I assume that those 
> will be created as follow up - or do they already exist? If so, do you 
> need to document something in fortran/intrinsic.texi? Or in libgomp.texi?

Please see
<http://news.gmane.org/find-root.php?message_id=%3C1383766943-8863-3-git-send-email-thomas%40codesourcery.com%3E>
and following for the patch submissions including Fortran bits, and/or
something like the following for reviewing the existing set of changes on
gomp-4_0-branch:

    $ svn log svn://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch | sed -n 
'/^svn merge/ { p; q; }'
    svn merge -r205223:206958 svn+ssh://gcc.gnu.org/svn/gcc/trunk
    $ svn diff svn://gcc.gnu.org/svn/gcc/trunk@206958 
svn://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch > diff

..., or the equivalent Git invocation (which is orders of magnitudes
faster):

    $ git log origin/gomp-4_0-branch | sed -n '/svn merge/ { p; q; }'
        svn merge -r205223:206958 svn+ssh://gcc.gnu.org/svn/gcc/trunk
    $ git svn find-rev r206958 origin/trunk
    765faa80eda3bb75aa044ad015e2e5214bf02c6d
    $ git diff --stat 765faa80eda3bb75aa044ad015e2e5214bf02c6d 
origin/gomp-4_0-branch | grep -v ChangeLog
     gcc/Makefile.in                                    |    3 +-
     gcc/builtins.def                                   |   10 +
     gcc/c-family/c-cppbuiltin.c                        |    3 +
     gcc/c-family/c-omp.c                               |    1 +
     gcc/c-family/c-pragma.c                            |   23 +
     gcc/c-family/c-pragma.h                            |   13 +-
     gcc/c-family/c.opt                                 |    4 +
     gcc/c/c-parser.c                                   |  278 ++++-
     gcc/c/c-tree.h                                     |    1 +
     gcc/c/c-typeck.c                                   |   23 +-
     gcc/cgraph.h                                       |    5 +
     gcc/cgraphbuild.c                                  |   14 +-
     gcc/cgraphunit.c                                   |   15 +-
     gcc/config/arc/arc.h                               |    2 +-
     gcc/config/darwin.h                                |    2 +-
     gcc/config/i386/mingw32.h                          |    2 +-
     gcc/config/ia64/hpux.h                             |    2 +-
     gcc/config/pa/pa-hpux11.h                          |    4 +-
     gcc/config/pa/pa64-hpux.h                          |   24 +-
     gcc/cp/parser.c                                    |    2 +-
     gcc/doc/generic.texi                               |    5 +
     gcc/doc/gimple.texi                                |    8 +
     gcc/doc/invoke.texi                                |   14 +-
     gcc/doc/sourcebuild.texi                           |    3 +
     gcc/fortran/cpp.c                                  |    3 +
     gcc/fortran/f95-lang.c                             |   10 +
     gcc/fortran/gfortran.h                             |    1 +
     gcc/fortran/invoke.texi                            |    7 +-
     gcc/fortran/lang.opt                               |    4 +
     gcc/fortran/options.c                              |    5 +
     gcc/gcc.c                                          |    5 +-
     gcc/gimple-low.c                                   |    1 +
     gcc/gimple-pretty-print.c                          |   58 +
     gcc/gimple-walk.c                                  |   16 +
     gcc/gimple.c                                       |   20 +
     gcc/gimple.def                                     |   10 +-
     gcc/gimple.h                                       |  127 ++-
     gcc/gimplify.c                                     |  144 ++-
     gcc/ipa-inline-analysis.c                          |    2 +-
     gcc/lto-cgraph.c                                   |   14 +
     gcc/lto-streamer.c                                 |    5 +-
     gcc/lto-streamer.h                                 |    6 +
     gcc/lto/lto-partition.c                            |    3 +
     gcc/oacc-builtins.def                              |   31 +
     gcc/omp-low.c                                      | 1081 
++++++++++++++----
     gcc/passes.c                                       |    6 +-
     gcc/testsuite/c-c++-common/cpp/openacc-define-1.c  |    6 +
     gcc/testsuite/c-c++-common/cpp/openacc-define-2.c  |    7 +
     gcc/testsuite/c-c++-common/cpp/openacc-define-3.c  |   11 +
     .../c-c++-common/goacc-gomp/nesting-fail-1.c       |  121 ++
     .../c-c++-common/goacc/data-clause-duplicate-1.c   |   13 +
     gcc/testsuite/c-c++-common/goacc/deviceptr-1.c     |   64 ++
     gcc/testsuite/c-c++-common/goacc/nesting-fail-1.c  |   11 +
     gcc/testsuite/c-c++-common/goacc/parallel-1.c      |    6 +
     gcc/testsuite/c-c++-common/goacc/parallel-fail-1.c |    6 +
     gcc/testsuite/gcc.dg/goacc-gomp/goacc-gomp.exp     |   38 +
     gcc/testsuite/gcc.dg/goacc/goacc.exp               |   37 +
     gcc/testsuite/gfortran.dg/openacc-define-1.f90     |    7 +
     gcc/testsuite/gfortran.dg/openacc-define-2.f90     |    7 +
     gcc/testsuite/gfortran.dg/openacc-define-3.f90     |   11 +
     gcc/testsuite/lib/target-supports.exp              |    9 +
     gcc/tree-core.h                                    |   41 +-
     gcc/tree-inline.c                                  |    4 +
     gcc/tree-nested.c                                  |   12 +
     gcc/tree-pass.h                                    |    2 +-
     gcc/tree-pretty-print.c                            |   26 +
     gcc/tree.def                                       |   11 +-
     gcc/tree.h                                         |    9 +-
     libgomp/Makefile.am                                |   11 +-
     libgomp/Makefile.in                                |   15 +-
     libgomp/config.h.in                                |    6 +
     libgomp/configure                                  |   63 ++
     libgomp/configure.ac                               |    9 +
     libgomp/libgomp.map                                |    8 +
     libgomp/libgomp_g.h                                |    5 +
     libgomp/oacc-parallel.c                            |   53 +
     libgomp/openacc.f90                                |   39 +
     libgomp/openacc.h                                  |   45 +
     libgomp/openacc_lib.h                              |   29 +
     libgomp/splay-tree.h                               |  232 ++++
     libgomp/target.c                                   |  634 ++++++++++-
     libgomp/testsuite/libgomp.oacc-c++/c++.exp         |   66 ++
     libgomp/testsuite/libgomp.oacc-c/c.exp             |   36 +
     libgomp/testsuite/libgomp.oacc-c/goacc_parallel.c  |   25 +
     libgomp/testsuite/libgomp.oacc-c/lib-1.c           |    7 +
     libgomp/testsuite/libgomp.oacc-c/parallel-1.c      |  156 +++
     libgomp/testsuite/libgomp.oacc-fortran/fortran.exp |   69 ++
     libgomp/testsuite/libgomp.oacc-fortran/lib-1.f90   |    3 +
     libgomp/testsuite/libgomp.oacc-fortran/lib-2.f     |    3 +
     libgomp/testsuite/libgomp.oacc-fortran/lib-3.f     |    3 +
     .../libgomp.oacc-fortran/openacc_version-1.f       |    9 +
     .../libgomp.oacc-fortran/openacc_version-2.f90     |    9 +
     102 files changed, 6964 insertions(+), 297 deletions(-)


> Ilmir Usmanov wrote:
> >     OpenACC 1.0 fortran FE support -- tests.
> >
> >     gcc/testsuite/gfortran.dg/goacc/
> >     * goacc.exp: New test directory.
> > +++ b/gcc/testsuite/gfortran.dg/goacc/branch.f95
> > @@ -0,0 +1,55 @@
> > +! { dg-do compile }
> > +! { dg-options "-fopenacc" }
> 
> Is there a reason that you don't automatically add that flag via goacc.exp?

Right, see below.

> > --- /dev/null
> > +++ b/gcc/testsuite/gfortran.dg/goacc/goacc.exp
> > @@ -0,0 +1,36 @@
> > +# Load support procs.
> > +load_lib gfortran-dg.exp
> > +
> > +if ![check_effective_target_fopenmp] {
> > +  return
> > +}
> 
> I assume that this should be indeed "fopenmp" here and not "fopenacc" as 
> both share libgomp?

It should be fopenacc, because it tests whether the compiler accepts
-fopenacc.

> > +# Main loop.
> > +gfortran-dg-runtest [lsort \
> > +       [find $srcdir/$subdir *.\[fF\]{,90,95,03,08} ] ] " -fopenacc 
> > -fdump-parse-tree"
> 
> As you use -fopenacc here, you probably can get rid of it in dg-options. 

Right.

> Can't you? I am not sure whether -fdump-parse-tree is needed; on the 
> other hand, it just clutters the *log files.

Yes, I think that should just be in the test case files that actually
need it?

> As -fopenmp seemingly can be mixed with -fopenacc, I think it would be 
> nice to have some test cases where !$omp and !$acc are both placed - in 
> either order - before the same Fortran statement.


Grüße,
 Thomas

Attachment: pgpNVV1XyQD_Z.pgp
Description: PGP signature

Reply via email to