This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, 4/16] Implement -foffload-alias
- From: Tom de Vries <Tom_deVries at mentor dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Richard Biener <rguenther at suse dot de>, "gcc-patches at gnu dot org" <gcc-patches at gnu dot org>
- Date: Mon, 14 Mar 2016 14:16:15 +0100
- Subject: Re: [PATCH, 4/16] Implement -foffload-alias
- Authentication-results: sourceware.org; auth=none
- References: <20151111110034 dot GF5675 at tucnak dot redhat dot com> <5644B84D dot 6050504 at mentor dot com> <alpine dot LSU dot 2 dot 11 dot 1511130944270 dot 4884 at t29 dot fhfr dot qr> <5645C33B dot 9080802 at mentor dot com> <alpine dot LSU dot 2 dot 11 dot 1511131228450 dot 4884 at t29 dot fhfr dot qr> <20151113113938 dot GM5675 at tucnak dot redhat dot com> <565058F0 dot 8040509 at mentor dot com> <alpine dot LSU dot 2 dot 11 dot 1511231241230 dot 4884 at t29 dot fhfr dot qr> <56584191 dot 60704 at mentor dot com> <565846A8 dot 6000509 at mentor dot com> <20151202095849 dot GE5675 at tucnak dot redhat dot com>
On 02/12/15 10:58, Jakub Jelinek wrote:
On Fri, Nov 27, 2015 at 01:03:52PM +0100, Tom de Vries wrote:
Handle non-declared variables in kernels alias analysis
2015-11-27 Tom de Vries <tom@codesourcery.com>
* gimplify.c (gimplify_scan_omp_clauses): Initialize
OMP_CLAUSE_ORIG_DECL.
* omp-low.c (install_var_field_1): Handle base_pointers_restrict for
pointers.
(map_ptr_clause_points_to_clause_p)
(nr_map_ptr_clauses_pointing_to_clause): New function.
(omp_target_base_pointers_restrict_p): Handle GOMP_MAP_POINTER.
* tree-pretty-print.c (dump_omp_clause): Print OMP_CLAUSE_ORIG_DECL.
* tree.c (omp_clause_num_ops): Set num_ops for OMP_CLAUSE_MAP to 3.
* tree.h (OMP_CLAUSE_ORIG_DECL): New macro.
* c-c++-common/goacc/kernels-alias-10.c: New test.
* c-c++-common/goacc/kernels-alias-9.c: New test.
I don't like this (mainly the addition of OMP_CLAUSE_ORIG_DECL),
but it also sounds wrong to me.
The primary question is how do you handle GOMP_MAP_POINTER
(which is something we don't use for C/C++ OpenMP anymore,
and Fortran OpenMP will stop using it in GCC 7 or 6.2?) on the OpenACC
libgomp side, does it work like GOMP_MAP_ALLOC or GOMP_MAP_FORCE_ALLOC?
When a GOMP_MAP_POINTER mapping is encountered, first we check if it has
been mapped before:
- if it hasn't been mapped before, we check if the area the pointer
points to has been mapped, and if not, error out. Else we map the
pointer to a device pointer, and write the device pointer value
to the device pointer variable.
- if the pointer has been mapped before, we reuse the mapping and write
the device pointer value to the device pointer variable.
Similarly GOMP_MAP_TO_PSET.
If it works like GOMP_MAP_ALLOC (it does
on the OpenMP side in target.c, so if something is already mapped, no
further pointer assignment happens), then your change looks wrong.
If it works like GOMP_MAP_FORCE_ALLOC, then you just should treat
GOMP_MAP_POINTER on all OpenACC constructs as opcode that allows the
restrict operation.
I guess it works mostly like GOMP_MAP_ALLOC, but I don't understand the
relevance of the comparison for the patch. What is interesting for the
restrict optimization is whether what GOMP_MAP_POINTER points to has
been mapped with or without the force flag during the same mapping sequence.
If it should behave differently depending on
if the corresponding array section has been mapped with GOMP_MAP_FORCE_*
or without it,
The mapping itself shouldn't behave differently.
then supposedly you should use a different code for
those two.
I could add f.i. an unsigned int aux_flags to struct tree_omp_clause,
set a new POINTS_TO_FORCE_VAR flag when translating the acc clause into
mapping clauses, and use that flag later on when dealing with the
GOMP_MAP_POINTER clause. Is that an acceptable approach?
[ Instead I could define a new gcc-internal-only
GOMP_MAP_POINTER_POINTS_TO_FORCE kind, but I'd rather avoid this, given
that it would be handled the same as GOMP_MAP_POINTER everywhere, except
for a single point in the source code. ]
Thanks,
- Tom