Skip to content
Snippets Groups Projects
  • Julian Brown's avatar
    b57abd07
    OpenMP 5.0: Clause ordering for OpenMP 5.0 (topological sorting by base pointer) · b57abd07
    Julian Brown authored
    This patch reimplements the omp_target_reorder_clauses function in
    anticipation of supporting "deeper" struct mappings (that is, with
    several structure dereference operators, or similar).
    
    The idea is that in place of the (possibly quadratic) algorithm in
    omp_target_reorder_clauses that greedily moves clauses containing
    addresses that are subexpressions of other addresses before those other
    addresses, we employ a topological sort algorithm to calculate a proper
    order for map clauses. This should run in linear time, and hopefully
    handles degenerate cases where multiple "levels" of indirect accesses
    are present on a given directive.
    
    The new method also takes care to keep clause groups together, addressing
    the concerns raised in:
    
      https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570501.html
    
    To figure out if some given clause depends on a base pointer in another
    clause, we strip off the outer layers of the address expression, and check
    (via a tree_operand_hash hash table we have built) if the result is a
    "base pointer" as defined in OpenMP 5.0 (1.2.6 Data Terminology). There
    are some subtleties involved, however:
    
     - We must treat MEM_REF with zero offset the same as INDIRECT_REF.
       This should probably be fixed in the front ends instead so we always
       use a canonical form (probably INDIRECT_REF). The following patch
       shows one instance of the problem, but there may be others:
    
       https://gcc.gnu.org/pipermail/gcc-patches/2021-May/571382.html
    
     - Mapping a whole struct implies mapping each of that struct's
       elements, which may be base pointers. Because those base pointers
       aren't necessarily explicitly referenced in the directive in question,
       we treat the whole-struct mapping as a dependency instead.
    
    2022-09-13  Julian Brown  <julian@codesourcery.com>
    
    gcc/
    	* gimplify.cc (is_or_contains_p, omp_target_reorder_clauses): Delete
    	functions.
    	(omp_tsort_mark): Add enum.
    	(omp_mapping_group): Add struct.
    	(debug_mapping_group, omp_get_base_pointer, omp_get_attachment,
    	omp_group_last, omp_gather_mapping_groups, omp_group_base,
    	omp_index_mapping_groups, omp_containing_struct,
    	omp_tsort_mapping_groups_1, omp_tsort_mapping_groups,
    	omp_segregate_mapping_groups, omp_reorder_mapping_groups): New
    	functions.
    	(gimplify_scan_omp_clauses): Call above functions instead of
    	omp_target_reorder_clauses, unless we've seen an error.
    	* omp-low.cc (scan_sharing_clauses): Avoid strict test if we haven't
    	sorted mapping groups.
    
    gcc/testsuite/
    	* g++.dg/gomp/target-lambda-1.C: Adjust expected output.
    	* g++.dg/gomp/target-this-3.C: Likewise.
    	* g++.dg/gomp/target-this-4.C: Likewise.
    b57abd07
    History
    OpenMP 5.0: Clause ordering for OpenMP 5.0 (topological sorting by base pointer)
    Julian Brown authored
    This patch reimplements the omp_target_reorder_clauses function in
    anticipation of supporting "deeper" struct mappings (that is, with
    several structure dereference operators, or similar).
    
    The idea is that in place of the (possibly quadratic) algorithm in
    omp_target_reorder_clauses that greedily moves clauses containing
    addresses that are subexpressions of other addresses before those other
    addresses, we employ a topological sort algorithm to calculate a proper
    order for map clauses. This should run in linear time, and hopefully
    handles degenerate cases where multiple "levels" of indirect accesses
    are present on a given directive.
    
    The new method also takes care to keep clause groups together, addressing
    the concerns raised in:
    
      https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570501.html
    
    To figure out if some given clause depends on a base pointer in another
    clause, we strip off the outer layers of the address expression, and check
    (via a tree_operand_hash hash table we have built) if the result is a
    "base pointer" as defined in OpenMP 5.0 (1.2.6 Data Terminology). There
    are some subtleties involved, however:
    
     - We must treat MEM_REF with zero offset the same as INDIRECT_REF.
       This should probably be fixed in the front ends instead so we always
       use a canonical form (probably INDIRECT_REF). The following patch
       shows one instance of the problem, but there may be others:
    
       https://gcc.gnu.org/pipermail/gcc-patches/2021-May/571382.html
    
     - Mapping a whole struct implies mapping each of that struct's
       elements, which may be base pointers. Because those base pointers
       aren't necessarily explicitly referenced in the directive in question,
       we treat the whole-struct mapping as a dependency instead.
    
    2022-09-13  Julian Brown  <julian@codesourcery.com>
    
    gcc/
    	* gimplify.cc (is_or_contains_p, omp_target_reorder_clauses): Delete
    	functions.
    	(omp_tsort_mark): Add enum.
    	(omp_mapping_group): Add struct.
    	(debug_mapping_group, omp_get_base_pointer, omp_get_attachment,
    	omp_group_last, omp_gather_mapping_groups, omp_group_base,
    	omp_index_mapping_groups, omp_containing_struct,
    	omp_tsort_mapping_groups_1, omp_tsort_mapping_groups,
    	omp_segregate_mapping_groups, omp_reorder_mapping_groups): New
    	functions.
    	(gimplify_scan_omp_clauses): Call above functions instead of
    	omp_target_reorder_clauses, unless we've seen an error.
    	* omp-low.cc (scan_sharing_clauses): Avoid strict test if we haven't
    	sorted mapping groups.
    
    gcc/testsuite/
    	* g++.dg/gomp/target-lambda-1.C: Adjust expected output.
    	* g++.dg/gomp/target-this-3.C: Likewise.
    	* g++.dg/gomp/target-this-4.C: Likewise.