[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

--- Comment #11 from Jakub Jelinek  ---
I think we have new clauses that need handling.  Perhaps we should drop the
default: case, so that we get warnings/errors.

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

Martin Liška  changed:

   What|Removed |Added

 CC||marxin at gcc dot gnu.org

--- Comment #10 from Martin Liška  ---
Thomas, Jakub: Can the bug be marked as resolved?

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

--- Comment #9 from Thomas Schwinge  ---
Author: tschwinge
Date: Mon Jun 13 16:37:29 2016
New Revision: 237386

URL: https://gcc.gnu.org/viewcvs?rev=237386=gcc=rev
Log:
[PR middle-end/71373] Document missing OMP_CLAUSE_* in gcc/tree-nested.c

gcc/
PR middle-end/71373
* tree-nested.c (convert_nonlocal_omp_clauses)
(convert_local_omp_clauses): Document missing OMP_CLAUSE_*.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/tree-nested.c

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

--- Comment #8 from Jakub Jelinek  ---
Well, if you want something that works well with wide range of GCCs, then your
case OMP_CLAUSE_TILE:
  /* OpenACC tile clauses are discarded during gimplification, so we
 don't expect to see anything here.  */
  gcc_unreachable ();

case OMP_CLAUSE__CACHE_:
  /* These clauses belong to the OpenACC cache directive, which is
 discarded during gimplification, so we don't expect to see
 anything here.  */
  gcc_unreachable ();

default:
  gcc_unreachable ();
is bad, while:
  /* OpenACC tile clauses are discarded during gimplification, so we
 don't expect to see anything here.  */
case OMP_CLAUSE_TILE:
  /* These clauses belong to the OpenACC cache directive, which is
 discarded during gimplification, so we don't expect to see
 anything here.  */
case OMP_CLAUSE__CACHE_:
default:
  gcc_unreachable ();
is optimized well.  The latter is optimized in GIMPLE_SWITCH during the cfg
pass.  Let me file a PR for the other form (though, in this case, having a
separate gcc_unreachable () in each case is IMHO also much uglier than just
listing it next to default.

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

Thomas Schwinge  changed:

   What|Removed |Added

 Status|NEW |SUSPENDED
   Assignee|unassigned at gcc dot gnu.org  |tschwinge at gcc dot 
gnu.org

--- Comment #7 from Thomas Schwinge  ---
Created attachment 38681
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38681=edit
Document missing OMP_CLAUSE_* in gcc/tree-nested.c

(In reply to Jakub Jelinek from comment #6)
> OMP_CLAUSE_UNIFORM
> OMP_CLAUSE_INBRANCH
> OMP_CLAUSE_NOTINBRANCH
> 
> The above 3 clauses are only allowed in the declare simd directive, so
> nothing tree-nested.c sees.
> 
> OMP_CLAUSE__LOOPTEMP_
> OMP_CLAUSE__SIMDUID_

(Also OMP_CLAUSE__GRIDDIM_.)

> These clauses are only added during omp lowering, tree-nested happens before
> that.
> 
> OMP_CLAUSE_FOR
> OMP_CLAUSE_PARALLEL
> OMP_CLAUSE_SECTIONS
> OMP_CLAUSE_TASKGROUP
> 
> These clauses are only allowed on cancel and cancellation point directives,
> which are at this point already lowered into a function call with numerical
> value.

> There is gcc_unreachable, IMHO no need to list them explicitly

It is a good thing in my opinion: the next curious reader of this code doesn't
have to wonder why some OMP_CLAUSE_* are missing here.

> and doing the
> same thing as the default, I'm afraid gcc is unable to optimize that.

Being curious about that, I'm confirming that using the patch I'm attaching. 
That's surprising to me!  The code size doesn't improve much (0.42 % in my
case), but I would have expected it to stay the same.  Can you tell off-hand
where I would have to look, to teach GCC how to optimize such constructs? 
There should be relatively common, I would think?  (Setting the PR to
"suspended" until that is resolved.)

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

--- Comment #6 from Jakub Jelinek  ---
OMP_CLAUSE_UNIFORM
OMP_CLAUSE_INBRANCH
OMP_CLAUSE_NOTINBRANCH

The above 3 clauses are only allowed in the declare simd directive, so nothing
tree-nested.c sees.

OMP_CLAUSE__LOOPTEMP_
OMP_CLAUSE__SIMDUID_

These clauses are only added during omp lowering, tree-nested happens before
that.

OMP_CLAUSE_FOR
OMP_CLAUSE_PARALLEL
OMP_CLAUSE_SECTIONS
OMP_CLAUSE_TASKGROUP

These clauses are only allowed on cancel and cancellation point directives,
which are at this point already lowered into a function call with numerical
value.

There is gcc_unreachable, IMHO no need to list them explicitly and doing the
same thing as the default, I'm afraid gcc is unable to optimize that.

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

Thomas Schwinge  changed:

   What|Removed |Added

   Keywords|patch   |
 Status|ASSIGNED|NEW
   Assignee|tschwinge at gcc dot gnu.org   |unassigned at gcc dot 
gnu.org

--- Comment #5 from Thomas Schwinge  ---
Fixed OpenACC clauses handling on trunk in r237291, gcc-6-branch in r237296,
and gomp-4_0-branch in r237300.


>From a quick look it appears as if the following OpenMP-specific clauses also
are not being handled:

  * OMP_CLAUSE_UNIFORM
  * OMP_CLAUSE__LOOPTEMP_
  * OMP_CLAUSE_INBRANCH
  * OMP_CLAUSE_NOTINBRANCH
  * OMP_CLAUSE_FOR
  * OMP_CLAUSE_PARALLEL
  * OMP_CLAUSE_SECTIONS
  * OMP_CLAUSE_TASKGROUP
  * OMP_CLAUSE__SIMDUID_
  * OMP_CLAUSE__GRIDDIM_

If these are in fact unreachable in gcc/tree-nested.c, I suggest to document
that with `gcc_unreachable` `case`s, and otherwise specific handling needs to
be implemented -- which I would just have done, but for some clauses, given my
limited OpenMP knowledge, I couldn't easily tell how these need to be handled,
or couldn't easily come up with valid test cases.  (Thus un-assigning myself.)

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373
Bug 71373 depends on bug 71381, which changed state.

Bug 71381 Summary: C/C++ OpenACC cache directive rejects valid syntax
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71381

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

--- Comment #4 from Thomas Schwinge  ---
Author: tschwinge
Date: Fri Jun 10 10:12:36 2016
New Revision: 237300

URL: https://gcc.gnu.org/viewcvs?rev=237300=gcc=rev
Log:
[PR middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

Backport trunk r237291:

gcc/
* gimplify.c (gimplify_adjust_omp_clauses): Discard
OMP_CLAUSE_TILE.
* omp-low.c (scan_sharing_clauses): Don't expect OMP_CLAUSE_TILE.
gcc/testsuite/
* c-c++-common/goacc/combined-directives.c: XFAIL tree scanning
for OpenACC tile clauses.
* gfortran.dg/goacc/combined-directives.f90: Likewise.

gcc/
PR middle-end/71373
* tree-nested.c (convert_nonlocal_omp_clauses)
(convert_local_omp_clauses): Handle OMP_CLAUSE_ASYNC,
OMP_CLAUSE_WAIT, OMP_CLAUSE_INDEPENDENT, OMP_CLAUSE_AUTO,
OMP_CLAUSE__CACHE_, OMP_CLAUSE_TILE.
gcc/testsuite/
PR middle-end/71373
* gcc.dg/goacc/nested-function-1.c: New file.
* gcc.dg/goacc/nested-function-2.c: Likewise.
* gcc.dg/goacc/pr71373.c: Likewise.
* gfortran.dg/goacc/cray-2.f95: Likewise.
* gfortran.dg/goacc/loop-1-2.f95: Likewise.
* gfortran.dg/goacc/loop-3-2.f95: Likewise.
* gfortran.dg/goacc/cray.f95: Update.
* gfortran.dg/goacc/loop-1.f95: Likewise.
* gfortran.dg/goacc/loop-3.f95: Likewise.
* gfortran.dg/goacc/subroutines.f90: Update, and rename to...
* gfortran.dg/goacc/nested-function-1.f90: ... this new file.
libgomp/testsuite/
PR middle-end/71373
* libgomp.oacc-c/nested-function-1.c: New file.
* libgomp.oacc-c/nested-function-2.c: Likewise.
* libgomp.oacc-fortran/nested-function-1.f90: Likewise.
* libgomp.oacc-fortran/nested-function-2.f90: Likewise.
* libgomp.oacc-fortran/nested-function-3.f90: Likewise.

Added:
branches/gomp-4_0-branch/gcc/testsuite/gcc.dg/goacc/nested-function-1.c
branches/gomp-4_0-branch/gcc/testsuite/gcc.dg/goacc/nested-function-2.c
branches/gomp-4_0-branch/gcc/testsuite/gcc.dg/goacc/pr71373.c
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/cray-2.f95
  - copied, changed from r237299,
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/cray.f95
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/loop-1-2.f95
  - copied, changed from r237299,
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/loop-1.f95
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/loop-3-2.f95
  - copied, changed from r237299,
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/loop-3.f95
   
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/nested-function-1.f90
   
branches/gomp-4_0-branch/libgomp/testsuite/libgomp.oacc-c/nested-function-1.c
   
branches/gomp-4_0-branch/libgomp/testsuite/libgomp.oacc-c/nested-function-2.c
   
branches/gomp-4_0-branch/libgomp/testsuite/libgomp.oacc-fortran/nested-function-1.f90
   
branches/gomp-4_0-branch/libgomp/testsuite/libgomp.oacc-fortran/nested-function-2.f90
   
branches/gomp-4_0-branch/libgomp/testsuite/libgomp.oacc-fortran/nested-function-3.f90
Removed:
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/subroutines.f90
Modified:
branches/gomp-4_0-branch/gcc/ChangeLog.gomp
branches/gomp-4_0-branch/gcc/gimplify.c
branches/gomp-4_0-branch/gcc/omp-low.c
branches/gomp-4_0-branch/gcc/testsuite/ChangeLog.gomp
   
branches/gomp-4_0-branch/gcc/testsuite/c-c++-common/goacc/combined-directives.c
   
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/cray.f95
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/loop-1.f95
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/loop-3.f95
branches/gomp-4_0-branch/gcc/tree-nested.c
branches/gomp-4_0-branch/libgomp/ChangeLog.gomp

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

--- Comment #3 from Thomas Schwinge  ---
Author: tschwinge
Date: Fri Jun 10 09:46:18 2016
New Revision: 237296

URL: https://gcc.gnu.org/viewcvs?rev=237296=gcc=rev
Log:
[PR middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

Backport trunk r237291:

gcc/
* gimplify.c (gimplify_adjust_omp_clauses): Discard
OMP_CLAUSE_TILE.
* omp-low.c (scan_sharing_clauses): Don't expect OMP_CLAUSE_TILE.
gcc/testsuite/
* c-c++-common/goacc/combined-directives.c: XFAIL tree scanning
for OpenACC tile clauses.
* gfortran.dg/goacc/combined-directives.f90: Likewise.

gcc/
PR middle-end/71373
* tree-nested.c (convert_nonlocal_omp_clauses)
(convert_local_omp_clauses): Handle OMP_CLAUSE_ASYNC,
OMP_CLAUSE_WAIT, OMP_CLAUSE_INDEPENDENT, OMP_CLAUSE_AUTO,
OMP_CLAUSE__CACHE_, OMP_CLAUSE_TILE.
gcc/testsuite/
PR middle-end/71373
* gcc.dg/goacc/nested-function-1.c: New file.
* gcc.dg/goacc/nested-function-2.c: Likewise.
* gcc.dg/goacc/pr71373.c: Likewise.
* gfortran.dg/goacc/cray-2.f95: Likewise.
* gfortran.dg/goacc/loop-1-2.f95: Likewise.
* gfortran.dg/goacc/loop-3-2.f95: Likewise.
* gfortran.dg/goacc/cray.f95: Update.
* gfortran.dg/goacc/loop-1.f95: Likewise.
* gfortran.dg/goacc/loop-3.f95: Likewise.
* gfortran.dg/goacc/subroutines.f90: Update, and rename to...
* gfortran.dg/goacc/nested-function-1.f90: ... this new file.
libgomp/testsuite/
PR middle-end/71373
* libgomp.oacc-c/nested-function-1.c: New file.
* libgomp.oacc-c/nested-function-2.c: Likewise.
* libgomp.oacc-fortran/nested-function-1.f90: Likewise.
* libgomp.oacc-fortran/nested-function-2.f90: Likewise.
* libgomp.oacc-fortran/nested-function-3.f90: Likewise.

Added:
branches/gcc-6-branch/gcc/testsuite/gcc.dg/goacc/nested-function-1.c
branches/gcc-6-branch/gcc/testsuite/gcc.dg/goacc/nested-function-2.c
branches/gcc-6-branch/gcc/testsuite/gcc.dg/goacc/pr71373.c
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/cray-2.f95
  - copied, changed from r237295,
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/cray.f95
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/loop-1-2.f95
  - copied, changed from r237295,
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/loop-1.f95
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/loop-3-2.f95
  - copied, changed from r237295,
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/loop-3.f95
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/nested-function-1.f90
branches/gcc-6-branch/libgomp/testsuite/libgomp.oacc-c/nested-function-1.c
branches/gcc-6-branch/libgomp/testsuite/libgomp.oacc-c/nested-function-2.c
   
branches/gcc-6-branch/libgomp/testsuite/libgomp.oacc-fortran/nested-function-1.f90
   
branches/gcc-6-branch/libgomp/testsuite/libgomp.oacc-fortran/nested-function-2.f90
   
branches/gcc-6-branch/libgomp/testsuite/libgomp.oacc-fortran/nested-function-3.f90
Removed:
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/subroutines.f90
Modified:
branches/gcc-6-branch/gcc/ChangeLog
branches/gcc-6-branch/gcc/gimplify.c
branches/gcc-6-branch/gcc/omp-low.c
branches/gcc-6-branch/gcc/testsuite/ChangeLog
   
branches/gcc-6-branch/gcc/testsuite/c-c++-common/goacc/combined-directives.c
   
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/cray.f95
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/loop-1.f95
branches/gcc-6-branch/gcc/testsuite/gfortran.dg/goacc/loop-3.f95
branches/gcc-6-branch/gcc/tree-nested.c
branches/gcc-6-branch/libgomp/ChangeLog

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

--- Comment #2 from Thomas Schwinge  ---
Author: tschwinge
Date: Fri Jun 10 09:22:51 2016
New Revision: 237291

URL: https://gcc.gnu.org/viewcvs?rev=237291=gcc=rev
Log:
[PR middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

gcc/
* gimplify.c (gimplify_adjust_omp_clauses): Discard
OMP_CLAUSE_TILE.
* omp-low.c (scan_sharing_clauses): Don't expect OMP_CLAUSE_TILE.
gcc/testsuite/
* c-c++-common/goacc/combined-directives.c: XFAIL tree scanning
for OpenACC tile clauses.
* gfortran.dg/goacc/combined-directives.f90: Likewise.

gcc/
PR middle-end/71373
* tree-nested.c (convert_nonlocal_omp_clauses)
(convert_local_omp_clauses): Handle OMP_CLAUSE_ASYNC,
OMP_CLAUSE_WAIT, OMP_CLAUSE_INDEPENDENT, OMP_CLAUSE_AUTO,
OMP_CLAUSE__CACHE_, OMP_CLAUSE_TILE.
gcc/testsuite/
PR middle-end/71373
* gcc.dg/goacc/nested-function-1.c: New file.
* gcc.dg/goacc/nested-function-2.c: Likewise.
* gcc.dg/goacc/pr71373.c: Likewise.
* gfortran.dg/goacc/cray-2.f95: Likewise.
* gfortran.dg/goacc/loop-1-2.f95: Likewise.
* gfortran.dg/goacc/loop-3-2.f95: Likewise.
* gfortran.dg/goacc/cray.f95: Update.
* gfortran.dg/goacc/loop-1.f95: Likewise.
* gfortran.dg/goacc/loop-3.f95: Likewise.
* gfortran.dg/goacc/subroutines.f90: Update, and rename to...
* gfortran.dg/goacc/nested-function-1.f90: ... this new file.
libgomp/testsuite/
PR middle-end/71373
* libgomp.oacc-c/nested-function-1.c: New file.
* libgomp.oacc-c/nested-function-2.c: Likewise.
* libgomp.oacc-fortran/nested-function-1.f90: Likewise.
* libgomp.oacc-fortran/nested-function-2.f90: Likewise.
* libgomp.oacc-fortran/nested-function-3.f90: Likewise.

Added:
trunk/gcc/testsuite/gcc.dg/goacc/nested-function-1.c
trunk/gcc/testsuite/gcc.dg/goacc/nested-function-2.c
trunk/gcc/testsuite/gcc.dg/goacc/pr71373.c
trunk/gcc/testsuite/gfortran.dg/goacc/cray-2.f95
  - copied, changed from r237290,
trunk/gcc/testsuite/gfortran.dg/goacc/cray.f95
trunk/gcc/testsuite/gfortran.dg/goacc/loop-1-2.f95
  - copied, changed from r237290,
trunk/gcc/testsuite/gfortran.dg/goacc/loop-1.f95
trunk/gcc/testsuite/gfortran.dg/goacc/loop-3-2.f95
  - copied, changed from r237290,
trunk/gcc/testsuite/gfortran.dg/goacc/loop-3.f95
trunk/gcc/testsuite/gfortran.dg/goacc/nested-function-1.f90
trunk/libgomp/testsuite/libgomp.oacc-c/nested-function-1.c
trunk/libgomp/testsuite/libgomp.oacc-c/nested-function-2.c
trunk/libgomp/testsuite/libgomp.oacc-fortran/nested-function-1.f90
trunk/libgomp/testsuite/libgomp.oacc-fortran/nested-function-2.f90
trunk/libgomp/testsuite/libgomp.oacc-fortran/nested-function-3.f90
Removed:
trunk/gcc/testsuite/gfortran.dg/goacc/subroutines.f90
Modified:
trunk/gcc/ChangeLog
trunk/gcc/gimplify.c
trunk/gcc/omp-low.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/c-c++-common/goacc/combined-directives.c
trunk/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90
trunk/gcc/testsuite/gfortran.dg/goacc/cray.f95
trunk/gcc/testsuite/gfortran.dg/goacc/loop-1.f95
trunk/gcc/testsuite/gfortran.dg/goacc/loop-3.f95
trunk/gcc/tree-nested.c
trunk/libgomp/ChangeLog

[Bug middle-end/71373] Handle more OMP_CLAUSE_* in nested function decomposition

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71373

Thomas Schwinge  changed:

   What|Removed |Added

   Keywords||patch
 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2016-06-03
 Depends on||71381
 Ever confirmed|0   |1

--- Comment #1 from Thomas Schwinge  ---
First patch approved,
.
 Waiting for PR71381.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71381
[Bug 71381] C/C++ OpenACC cache directive rejects valid syntax