| Message ID | 8738kwgabi.fsf@schwinge.name |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 09, 2014 at 09:38:25PM +0100, Thomas Schwinge wrote: > In gimplify.c:gimplify_adjust_omp_clauses_1, does the case for > GOVD_MAP_TO_ONLY have a real current use case (I couldn't spot any), or > is it "just for completeness"? It is typically for any artificial vars that are known not to need copying back, such as various artifical vars used for VLAs (say if you do sizeof on vla inside of target region, typesizes etc.). The testsuite coverage is insufficient here, sure. GOVD_MAP_TO_ONLY is kind of GOVD_FIRSTPRIVATE for the target regions, as opposed to GOVD_SHARED. Jakub
Hi! On Thu, 9 Jan 2014 21:43:35 +0100, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Jan 09, 2014 at 09:38:25PM +0100, Thomas Schwinge wrote: > > In gimplify.c:gimplify_adjust_omp_clauses_1, does the case for > > GOVD_MAP_TO_ONLY have a real current use case (I couldn't spot any), or > > is it "just for completeness"? > > It is typically for any artificial vars that are known not to need copying > back, such as various artifical vars used for VLAs (say if you do sizeof > on vla inside of target region, typesizes etc.). The testsuite coverage is > insufficient here, sure. GOVD_MAP_TO_ONLY is kind of GOVD_FIRSTPRIVATE > for the target regions, as opposed to GOVD_SHARED. Thanks, that makes sense (and I put a TODO item up for adding such tests to the testsuite), but I still can't manage to find one that actually triggers the GOVD_MAP_TO_ONLY case in gimplify_adjust_omp_clauses_1. Maybe it's just too late today. Grüße, Thomas
Hi! On Thu, 09 Jan 2014 23:21:26 +0100, I wrote: > On Thu, 9 Jan 2014 21:43:35 +0100, Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Jan 09, 2014 at 09:38:25PM +0100, Thomas Schwinge wrote: > > > In gimplify.c:gimplify_adjust_omp_clauses_1, does the case for > > > GOVD_MAP_TO_ONLY have a real current use case (I couldn't spot any), or > > > is it "just for completeness"? > > > > It is typically for any artificial vars that are known not to need copying > > back, such as various artifical vars used for VLAs (say if you do sizeof > > on vla inside of target region, typesizes etc.). The testsuite coverage is > > insufficient here, sure. GOVD_MAP_TO_ONLY is kind of GOVD_FIRSTPRIVATE > > for the target regions, as opposed to GOVD_SHARED. > > Thanks, that makes sense (and I put a TODO item up for adding such tests > to the testsuite), but I still can't manage to find one that actually > triggers the GOVD_MAP_TO_ONLY case in gimplify_adjust_omp_clauses_1. > Maybe it's just too late today. Haha, and here we go; need something nested to trigger this: int l = 10; float c[l]; #pragma omp target map(c[2:4]) { #pragma omp target { int s = sizeof c; } } Grüße, Thomas
diff --git gcc/gimplify.c gcc/gimplify.c index 3738589..870550c 100644 --- gcc/gimplify.c +++ gcc/gimplify.c @@ -6136,9 +6136,9 @@ gimplify_adjust_omp_clauses_1 (splay_tree_node n, void *data) OMP_CLAUSE_PRIVATE_OUTER_REF (clause) = 1; else if (code == OMP_CLAUSE_MAP) { - OMP_CLAUSE_MAP_KIND (clause) = flags & GOVD_MAP_TO_ONLY - ? OMP_CLAUSE_MAP_TO - : OMP_CLAUSE_MAP_TOFROM; + gcc_assert (!(flags & GOVD_MAP_TO_ONLY)); + OMP_CLAUSE_MAP_KIND (clause) = OMP_CLAUSE_MAP_TOFROM; + if (DECL_SIZE (decl) && TREE_CODE (DECL_SIZE (decl)) != INTEGER_CST) {