Hi Jakub,
here is v3 of this patch set.

On 2020/10/29 7:44 PM, Jakub Jelinek wrote:
>> +extern void c_omp_adjust_clauses (tree, bool);
> So, can you please rename the function to either
> c_omp_adjust_target_clauses or c_omp_adjust_mapping_clauses or
> c_omp_adjust_map_clauses?

I've renamed it to 'c_omp_adjust_map_clauses'.

>> --- a/gcc/c-family/c-omp.c
>> +++ b/gcc/c-family/c-omp.c
>> @@ -2579,3 +2579,50 @@ c_omp_map_clause_name (tree clause, bool oacc)
>>       }
>>     return omp_clause_code_name[OMP_CLAUSE_CODE (clause)];
>>   }
>> +
>> +/* Adjust map clauses after normal clause parsing, mainly to turn specific
>> +   base-pointer map cases into attach/detach and mark them addressable.  */
>> +void
>> +c_omp_adjust_clauses (tree clauses, bool is_target)
>> +{
>> +  for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
>> +    if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
>> +	&& OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
> If this is only meant to handle decls, perhaps there should be
> && DECL_P (OMP_CLAUSE_DECL (c))
> ?
> 
>> +	&& TREE_CODE (TREE_TYPE (OMP_CLAUSE_DECL (c))) != ARRAY_TYPE)
>> +      {
>> +	tree ptr = OMP_CLAUSE_DECL (c);
>> +	bool ptr_mapped = false;
>> +	if (is_target)
>> +	  {
>> +	    for (tree m = clauses; m; m = OMP_CLAUSE_CHAIN (m))
>> +	      if (OMP_CLAUSE_CODE (m) == OMP_CLAUSE_MAP
>> +		  && OMP_CLAUSE_DECL (m) == ptr
>> +		  && (OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALLOC
>> +		      || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TO
>> +		      || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_FROM
>> +		      || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TOFROM
>> +		      || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_TO
>> +		      || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_FROM
>> +		      || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_TOFROM))
>> +		{
>> +		  ptr_mapped = true;
>> +		  break;
>> +		}
> What you could e.g. do is have this loop at the start of function, with
> && DECL_P (OMP_CLAUSE_DECL (m))
> instead of the == ptr check, and perhaps && POINTER_TYPE_P (TREE_TYPE
> (OMP_CLAUSE_DECL (m))) check and set a bit in a bitmap for each such decl,
> then in the GOMP_MAP_FIRSTPRIVATE_POINTER loop just check the bitmap.
> Or, keep it in the loop like it is above, but populate the bitmap
> lazily (upon seeing the first GOMP_MAP_FIRSTPRIVATE_POINTER) and for further
> ones just use it.

I re-wrote c_omp_adjust_map_clauses to address the complexity issues you mentioned,
now it should be limited by a linear pass to collect and merge the firstprivate base
pointer + existence of a mapping of it, using a hash_map.

Patch set has been re-tested with no regressions for gcc, g++, gfortran, and libgomp.

Thanks,
Chung-Lin

	gcc/c-family/
	* c-common.h (c_omp_adjust_map_clauses): New declaration.
	* c-omp.c (c_omp_adjust_map_clauses): New function.

	gcc/c/
	* c-parser.c (c_parser_omp_target_data): Add use of
	new c_omp_adjust_map_clauses function. Add GOMP_MAP_ATTACH_DETACH as
	handled map clause kind.
	(c_parser_omp_target_enter_data): Likewise.
	(c_parser_omp_target_exit_data): Likewise.
	(c_parser_omp_target): Likewise.
	* c-typeck.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
	use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type.
	(c_finish_omp_clauses): Adjust bitmap checks to allow struct decl and
	same struct field access to co-exist on OpenMP construct.

	gcc/cp/
	* parser.c (cp_parser_omp_target_data): Add use of
	new c_omp_adjust_map_clauses function. Add GOMP_MAP_ATTACH_DETACH as
	handled map clause kind.
	(cp_parser_omp_target_enter_data): Likewise.
	(cp_parser_omp_target_exit_data): Likewise.
	(cp_parser_omp_target): Likewise.
	* semantics.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
	use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. Fix
	interaction between reference case and attach/detach.
	(finish_omp_clauses): Adjust bitmap checks to allow struct decl and
	same struct field access to co-exist on OpenMP construct.