[Bug c++/80029] [6/7 Regression] valgrind error in new_omp_context(omp_region_type) (gimplify.c:400)

[cesar at gcc dot gnu.org]

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

cesar at gcc dot gnu.org changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #5 from cesar at gcc dot gnu.org ---
Fixed in trunk.

[Bug c++/80029] [6/7 Regression] valgrind error in new_omp_context(omp_region_type) (gimplify.c:400)

[cesar at gcc dot gnu.org]

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

--- Comment #4 from cesar at gcc dot gnu.org ---
Author: cesar
Date: Wed Mar 22 13:52:10 2017
New Revision: 246381

URL: https://gcc.gnu.org/viewcvs?rev=246381=gcc=rev
Log:
PR c++/80029

gcc/
* gimplify.c (is_oacc_declared): New function.
(oacc_default_clause): Use it to set default flags for acc declared
variables inside parallel regions.
(gimplify_scan_omp_clauses): Strip firstprivate pointers for acc
declared variables.
(gimplify_oacc_declare): Gimplify the declare clauses.  Add the
declare attribute to any decl as necessary.

libgomp/
* testsuite/libgomp.oacc-c-c++-common/declare-vla.c: New test.

Added:
trunk/libgomp/testsuite/libgomp.oacc-c-c++-common/declare-vla.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/gimplify.c
trunk/libgomp/ChangeLog

[Bug c++/80029] [6/7 Regression] valgrind error in new_omp_context(omp_region_type) (gimplify.c:400)

[cesar at gcc dot gnu.org]

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

--- Comment #3 from cesar at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #2)
> That doesn't work, it is too early.
> With:
> --- gimplify.c.jj 2017-03-08 18:19:24.0 +0100
> +++ gimplify.c2017-03-20 07:22:05.587913531 +0100
> @@ -9261,6 +9261,8 @@ gimplify_oacc_declare (tree *expr_p, gim
>omp_add_variable (gimplify_omp_ctxp, decl, GOVD_SEEN);
>  }
>  
> +  gimplify_adjust_omp_clauses (pre_p, NULL, , OACC_DECLARE);
> +
>stmt = gimple_build_omp_target (NULL, GF_OMP_TARGET_KIND_OACC_DECLARE,
> clauses);
>  
> we don't ICE on it, but still no idea what is the right thing to do.
> From what I understand, #pragma acc declare is active for all code from the
> directive till end of function (unless the variables mentioned in its
> clauses go out of scope earlier), so in theory perhaps the right thing is to
> move the gimplify omp context structure up in the gimplify context tree so
> that they are just children of the function (what happens if you put #pragma
> acc declare inside
> the body of some other acc region?).

You are correctly describing the behavior of declare in OpenACC. Declared
variables live throughout the scope specified by the user. However, I believe
the scope must be global or function, i.e. I don't think you can declare
variables inside nested blocks in C/C++.

> Though on the other side, e.g. for #pragma omp target enter data we adjust
> the omp clauses right away, but that doesn't really affect the handling of
> variables later on.  So, the important question is do you need the variables
> mentioned in acc declare clauses to be in the hash tables as GOMP_MAP after
> gimplify_oacc_declare returns or not?  If not, then the above patch might
> DTRT, otherwise we need to figure out something different.

I don't think this memory leak is present in gomp-4_0-branch. The patch that
resolves this issue was posted back in September 2016 here
. I'll rebuild trunk
with that patch to see if it resolves the memory leak there.

[Bug c++/80029] [6/7 Regression] valgrind error in new_omp_context(omp_region_type) (gimplify.c:400)

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

 CC||cesar at gcc dot gnu.org,
   ||tschwinge at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek  ---
That doesn't work, it is too early.
With:
--- gimplify.c.jj   2017-03-08 18:19:24.0 +0100
+++ gimplify.c  2017-03-20 07:22:05.587913531 +0100
@@ -9261,6 +9261,8 @@ gimplify_oacc_declare (tree *expr_p, gim
   omp_add_variable (gimplify_omp_ctxp, decl, GOVD_SEEN);
 }

+  gimplify_adjust_omp_clauses (pre_p, NULL, , OACC_DECLARE);
+
   stmt = gimple_build_omp_target (NULL, GF_OMP_TARGET_KIND_OACC_DECLARE,
  clauses);

we don't ICE on it, but still no idea what is the right thing to do.
>From what I understand, #pragma acc declare is active for all code from the
directive till end of function (unless the variables mentioned in its clauses
go out of scope earlier), so in theory perhaps the right thing is to move the
gimplify omp context structure up in the gimplify context tree so that they are
just children of the function (what happens if you put #pragma acc declare
inside
the body of some other acc region?).
Though on the other side, e.g. for #pragma omp target enter data we adjust the
omp clauses right away, but that doesn't really affect the handling of
variables later on.  So, the important question is do you need the variables
mentioned in acc declare clauses to be in the hash tables as GOMP_MAP after
gimplify_oacc_declare returns or not?  If not, then the above patch might DTRT,
otherwise we need to figure out something different.

[Bug c++/80029] [6/7 Regression] valgrind error in new_omp_context(omp_region_type) (gimplify.c:400)

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-03-14
 CC||jakub at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #1 from Jakub Jelinek  ---
I think following change could cure it:

--- gcc/gimplify.c.jj   2017-02-16 08:36:56.0 +0100
+++ gcc/gimplify.c  2017-03-14 17:30:02.040865055 +0100
@@ -9236,6 +9236,7 @@ gimplify_oacc_declare (tree *expr_p, gim
   clauses = OACC_DECLARE_CLAUSES (expr);

   gimplify_scan_omp_clauses (, pre_p, ORT_TARGET_DATA, OACC_DECLARE);
+  gimplify_adjust_omp_clauses (pre_p, NULL, , OACC_DECLARE);

   for (t = clauses; t; t = OMP_CLAUSE_CHAIN (t))
 {

though can't test that right now.

[Bug c++/80029] [6/7 Regression] valgrind error in new_omp_context(omp_region_type) (gimplify.c:400)

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Target Milestone|--- |6.4