[PATCH] omp-low.c split

[PATCH] omp-low.c split

[Martin Jambor]

[-- Attachment #1: Type: text/plain, Size: 15336 bytes --]

Hello,

this is the promised attempt at splitting omp-low.c that turns:

  gcc$ wc -l omp-*.[ch]
   21129 omp-low.c
      44 omp-low.h
    1726 omp-simd-clone.c

into:

  gcc$ wc -l omp-*.[ch]
    1718 omp-device.c
      30 omp-device.h
    8195 omp-expand.c
      32 omp-expand.h
     650 omp-general.c
      91 omp-general.h
    1407 omp-grid.c
      27 omp-grid.h
    9240 omp-low.c
      31 omp-low.h
    1726 omp-simd-clone.c

Basically, I tried to:

  - move everything that seemed to part of expand or somehow
    expand-related (usually because it uses omp_region data
    structure) to a new file omp-expand.c,

  - move everything that is part of pass_oacc_device_lower,
    pass_omp_device_lower and pass_omp_target_link to a new file
    omp-device.h,

  - move all pre-lowering gridification stuff to a new file
    omp-grid.c.  I have moved expansion of HSA giridified kernels to
    omp-expand.c because moving them elsewhere would mean exposing
    omp_region somehow.  We can always do that as a follow-up.

  - I moved stuff that was used from all over the place to a new file
    omp-general.c (unless it would mean exposing omp_region or
    omp_context types).

  - I have added a header file associated with each new .c file for
    symbols exported from them.  I have renamed all exported functions
    so that all of them start with either omp_ or oacc_

  - I have removed includes in omp-low.c that no were no longer
    necessary.

  - I have removed static inline function get_base_type that was
    apparently no longer used.

  - The new files should not have any trailing whitespace and I
    corrected a (very) few instances of bad horizontal whitespace but
    I tried to keep the actual changes to a minimum.

I am opened to suggestions what to do differently, names of the file
are for example of course subject to discussion, and I absolutely
welcome any review and checking, for one I am not going to pretend I
understand the stuff I put into omp-device.c.  If however there is
consensus that we should do something like this, I would like to ask
the community to freeze omp-low.c file until this gets committed, I
hope you understand that I am afraid of any conflicts.

I have bootstrapped and tested a very similar patch (before #include
pruning) on x86_64-linux and on aarch64-linux, re-runs of those are
currently underway.

Please find the changelog below and the compressed patch (1.1 MB
before gzipping) attached.  After all feedback is resolved, I would
like to commit this to trunk.

Thanks,

Martin



2016-12-09  Martin Jambor  <mjambor@suse.cz>

	* omp-general.h: New file.
	* omp-general.c: New file.
	* omp-expand.h: Likewise.
	* omp-expand.c: Likewise.
	* omp-device.h: Likewise.
	* omp-device.c: Likewise.
	* omp-grid.c: Likewise.
	* omp-grid.c: Likewise.
	* omp-low.h: Include omp-general.h and omp-grid.h.  Removed includes
	of params.h, symbol-summary.h, lto-section-names.h, cilk.h, tree-eh.h,
	ipa-prop.h, tree-cfgcleanup.h, cfgloop.h, except.h, expr.h, stmt.h,
	varasm.h, calls.h, explow.h, dojump.h, flags.h, tree-into-ssa.h,
	tree-cfg.h, cfganal.h, alias.h, emit-rtl.h, optabs.h, expmed.h,
	alloc-pool.h, cfghooks.h, rtl.h and memmodel.h.
	(omp_find_combined_for): Declare.
	(find_omp_clause): Renamed to omp_find_clause and moved to
	omp-general.h.
	(free_omp_regions): Renamed to omp_free_regions and moved to
	omp-expand.h.
	(replace_oacc_fn_attrib): Renamed to oacc_replace_fn_attrib and moved
	to omp-general.h.
	(set_oacc_fn_attrib): Renamed to oacc_set_fn_attrib and moved to
	omp-general.h.
	(build_oacc_routine_dims): Renamed to oacc_build_routine_dims and
	moved to omp-general.h.
	(get_oacc_fn_attrib): Renamed to oacc_get_fn_attrib and moved to
	omp-general.h.
	(oacc_fn_attrib_kernels_p): Moved to omp-general.h.
	(get_oacc_fn_dim_size): Renamed to oacc_get_fn_dim_size and moved to
	omp-general.c.
	(omp_expand_local): Moved to omp-expand.h.
	(make_gimple_omp_edges): Renamed to omp_make_gimple_edges and moved to
	omp-expand.h.
	(omp_finish_file): Moved to omp-device.h.
	(default_goacc_validate_dims): Renamed to
	oacc_default_goacc_validate_dims and moved to omp-device.h.
	(offload_funcs, offload_vars): Moved to omp-device.h.
	* omp-low.c: Include omp-general.h, omp-device.h and omp-grid.h.
	(omp_region): Moved to omp-expand.c.
	(omp_for_data_loop): Moved to omp-general.h.
	(omp_for_data): Likewise.
	(oacc_loop): Moved to omp-device.c.
	(oacc_loop_flags): Moved to omp-general.h.
	(offload_funcs, offload_vars): Moved to omp-device.c.
	(root_omp_region): Moved to omp-expand.c.
	(omp_any_child_fn_dumped): Likewise.
	(find_omp_clause): Renamed to omp_find_clause and moved to
	omp-general.c.
	(is_combined_parallel): kMoved to omp-expand.c.
	(is_reference): Renamed to omp_is_reference and and moved to
	omp-general.c.
	(adjust_for_condition): Renamed to omp_adjust_for_condition and moved
	to omp-general.c.
	(get_omp_for_step_from_incr): Renamed to omp_get_for_step_from_incr
	and moved to omp-general.c.
	(extract_omp_for_data): Renamed to omp_extract_for_data and moved to
	omp-general.c.
	(workshare_safe_to_combine_p): Moved to omp-expand.c.
	(omp_adjust_chunk_size): Likewise.
	(get_ws_args_for): Likewise.
	(get_base_type): Removed.
	(dump_omp_region): Moved to omp-expand.c.
	(debug_omp_region): Likewise.
	(debug_all_omp_regions): Likewise.
	(new_omp_region): Likewise.
	(free_omp_region_1): Likewise.
	(free_omp_regions): Renamed to omp_free_regions and moved to
	omp-expand.c.
	(find_combined_for): Renamed to omp_find_combined_for, made global.
	(build_omp_barrier): Renamed to omp_build_barrier and moved to
	omp-general.c.
	(omp_max_vf): Moved to omp-general.c.
	(omp_max_simt_vf): Likewise.
	(gimple_build_cond_empty): Moved to omp-expand.c.
	(parallel_needs_hsa_kernel_p): Likewise.
	(expand_omp_build_assign): Moved declaration to omp-expand.c.
	(expand_parallel_call): Moved to omp-expand.c.
	(expand_cilk_for_call): Likewise.
	(expand_task_call): Likewise.
	(vec2chain): Likewise.
	(remove_exit_barrier): Likewise.
	(remove_exit_barriers): Likewise.
	(optimize_omp_library_calls): Likewise.
	(expand_omp_regimplify_p): Likewise.
	(expand_omp_build_assign): Likewise.
	(expand_omp_taskreg): Likewise.
	(oacc_collapse): Likewise.
	(expand_oacc_collapse_init): Likewise.
	(expand_oacc_collapse_vars): Likewise.
	(expand_omp_for_init_counts): Likewise.
	(expand_omp_for_init_vars): Likewise.
	(extract_omp_for_update_vars): Likewise.
	(expand_omp_ordered_source): Likewise.
	(expand_omp_ordered_sink): Likewise.
	(expand_omp_ordered_source_sink): Likewise.
	(expand_omp_for_ordered_loops): Likewise.
	(expand_omp_for_generic): Likewise.
	(expand_omp_for_static_nochunk): Likewise.
	(find_phi_with_arg_on_edge): Likewise.
	(expand_omp_for_static_chunk): Likewise.
	(expand_cilk_for): Likewise.
	(expand_omp_simd): Likewise.
	(expand_omp_taskloop_for_outer): Likewise.
	(expand_omp_taskloop_for_inner): Likewise.
	(expand_oacc_for): Likewise.
	(expand_omp_for): Likewise.
	(expand_omp_sections): Likewise.
	(expand_omp_single): Likewise.
	(expand_omp_synch): Likewise.
	(expand_omp_atomic_load): Likewise.
	(expand_omp_atomic_store): Likewise.
	(expand_omp_atomic_fetch_op): Likewise.
	(expand_omp_atomic_pipeline): Likewise.
	(expand_omp_atomic_mutex): Likewise.
	(expand_omp_atomic): Likewise.
	(oacc_launch_pack): and moved to omp-general.c, made public.
	(OACC_FN_ATTRIB): Likewise.
	(replace_oacc_fn_attrib): Renamed to oacc_replace_fn_attrib and moved
	to omp-general.c.
	(set_oacc_fn_attrib): Renamed to oacc_set_fn_attrib and moved to
	omp-general.c.
	(build_oacc_routine_dims): Renamed to oacc_build_routine_dims and
	moved to omp-general.c.
	(get_oacc_fn_attrib): Renamed to oacc_get_fn_attrib and moved to
	omp-general.c.
	(oacc_fn_attrib_kernels_p): Moved to omp-general.c.
	(oacc_fn_attrib_level): Moved to omp-device.c.
	(get_oacc_fn_dim_size): Renamed to oacc_get_fn_dim_size and moved to
	omp-general.c.
	(get_oacc_ifn_dim_arg): Renamed to oacc_get_ifn_dim_arg and moved to
	omp-general.c.
	(mark_loops_in_oacc_kernels_region): Moved to omp-expand.c.
	(grid_launch_attributes_trees): Likewise.
	(grid_attr_trees): Likewise.
	(grid_create_kernel_launch_attr_types): Likewise.
	(grid_insert_store_range_dim): Likewise.
	(grid_get_kernel_launch_attributes): Likewise.
	(get_target_argument_identifier_1): Likewise.
	(get_target_argument_identifier): Likewise.
	(get_target_argument_value): Likewise.
	(push_target_argument_according_to_value): Likewise.
	(get_target_arguments): Likewise.
	(expand_omp_target): Likewise.
	(grid_expand_omp_for_loop): Moved to omp-grid.c.
	(grid_arg_decl_map): Likewise.
	(grid_remap_kernel_arg_accesses): Likewise.
	(grid_expand_target_grid_body): Likewise.
	(expand_omp): Renamed to omp_expand and moved to omp-expand.c.
	(build_omp_regions_1): Moved to omp-expand.c.
	(build_omp_regions_root): Likewise.
	(omp_expand_local): Likewise.
	(build_omp_regions): Likewise.
	(execute_expand_omp): Likewise.
	(pass_data_expand_omp): Likewise.
	(pass_expand_omp): Likewise.
	(make_pass_expand_omp): Likewise.
	(pass_data_expand_omp_ssa): Likewise.
	(pass_expand_omp_ssa): Likewise.
	(make_pass_expand_omp_ssa): Likewise.
	(grid_lastprivate_predicate): Renamed to
	omp_grid_lastprivate_predicate and moved to omp-grid.c, made public.
	(grid_prop): Moved to omp-grid.c.
	(GRID_MISSED_MSG_PREFIX): Likewise.
	(grid_safe_assignment_p): Likewise.
	(grid_seq_only_contains_local_assignments): Likewise.
	(grid_find_single_omp_among_assignments_1): Likewise.
	(grid_find_single_omp_among_assignments): Likewise.
	(grid_find_ungridifiable_statement): Likewise.
	(grid_parallel_clauses_gridifiable): Likewise.
	(grid_inner_loop_gridifiable_p): Likewise.
	(grid_dist_follows_simple_pattern): Likewise.
	(grid_gfor_follows_tiling_pattern): Likewise.
	(grid_call_permissible_in_distribute_p): Likewise.
	(grid_handle_call_in_distribute): Likewise.
	(grid_dist_follows_tiling_pattern): Likewise.
	(grid_target_follows_gridifiable_pattern): Likewise.
	(grid_remap_prebody_decls): Likewise.
	(grid_var_segment): Likewise.
	(grid_mark_variable_segment): Likewise.
	(grid_copy_leading_local_assignments): Likewise.
	(grid_process_grid_body): Likewise.
	(grid_eliminate_combined_simd_part): Likewise.
	(grid_mark_tiling_loops): Likewise.
	(grid_mark_tiling_parallels_and_loops): Likewise.
	(grid_process_kernel_body_copy): Likewise.
	(grid_attempt_target_gridification): Likewise.
	(grid_gridify_all_targets_stmt): Likewise.
	(grid_gridify_all_targets): Renamed to omp_grid_gridify_all_targets
	and moved to omp-grid.c, made public.
	(make_gimple_omp_edges): Renamed to omp_make_gimple_edges and moved to
	omp-expand.c.
	(add_decls_addresses_to_decl_constructor): Moved to omp-device.c.
	(omp_finish_file): Likewise.
	(oacc_thread_numbers): Likewise.
	(oacc_xform_loop): Likewise.
	(oacc_default_dims, oacc_min_dims): Likewise.
	(oacc_parse_default_dims): Likewise.
	(oacc_validate_dims): Likewise.
	(new_oacc_loop_raw): Likewise.
	(new_oacc_loop_outer): Likewise.
	(new_oacc_loop): Likewise.
	(new_oacc_loop_routine): Likewise.
	(finish_oacc_loop): Likewise.
	(free_oacc_loop): Likewise.
	(dump_oacc_loop_part): Likewise.
	(dump_oacc_loop): Likewise.
	(debug_oacc_loop): Likewise.
	(oacc_loop_discover_walk): Likewise.
	(oacc_loop_sibling_nreverse): Likewise.
	(oacc_loop_discovery): Likewise.
	(oacc_loop_xform_head_tail): Likewise.
	(oacc_loop_xform_loop): Likewise.
	(oacc_loop_process): Likewise.
	(oacc_loop_fixed_partitions): Likewise.
	(oacc_loop_auto_partitions): Likewise.
	(oacc_loop_partition): Likewise.
	(default_goacc_fork_join): Likewise.
	(default_goacc_reduction): Likewise.
	(execute_oacc_device_lower): Likewise.
	(default_goacc_validate_dims): Likewise.
	(default_goacc_dim_limit): Likewise.
	(pass_data_oacc_device_lower): Likewise.
	(pass_oacc_device_lower): Likewise.
	(make_pass_oacc_device_lower): Likewise.
	(execute_omp_device_lower): Likewise.
	(pass_data_omp_device_lower): Likewise.
	(pass_omp_device_lower): Likewise.
	(make_pass_omp_device_lower): Likewise.
	(pass_data_omp_target_link): Likewise.
	(pass_omp_target_link): Likewise.
	(find_link_var_op): Likewise.
	(pass_omp_target_link::execute): Likewise.
	(make_pass_omp_target_link): Likewise.

	* Makefile.in (OBJS): Added omp-device.o, omp-expand.o, omp-general.o
	and omp-grid.o.
	(GTFILES): Added omp-device.h, omp-device.c and omp-expand.c, removed
	omp-low.h.
	* gimple-fold.c: Include omp-general.h instead of omp-low.h.
	(fold_internal_goacc_dim): Adjusted calls to
	get_oacc_ifn_dim_arg and get_oacc_fn_dim_size to use their new names.
	* gimplify.c: Include omp-low.h.
	(omp_notice_variable): Adjust the call to get_oacc_fn_attrib to use
	its new name.
	(gimplify_omp_task): Adjusted calls to find_omp_clause to use its new
	name.
	(gimplify_omp_for): Likewise.
	* lto-cgraph.c: Include omp-device.h instead of omp-low.h.
	* toplev.c: Include omp-device.h instead of omp-low.h.
	* tree-cfg.c: Include omp-general.h instead of omp-low.h.  Also
	include omp-expand.h.
	(make_edges_bb): Adjusted the call to make_gimple_omp_edges to use its
	new name.
	(make_edges): Adjust the call to free_omp_regions to use its new name.
	* tree-parloops.c: Include omp-general.h.
	(create_parallel_loop): Adjusted the call to set_oacc_fn_attrib to use
	its new name.
	(parallelize_loops): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	* tree-ssa-loop.c: Include omp-general.h instead of omp-low.h.
	(gate_oacc_kernels): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	* tree-vrp.c: Include omp-general.h instead of omp-low.h.
	(extract_range_basic): Adjusted calls to get_oacc_ifn_dim_arg and
	get_oacc_fn_dim_size to use their new names.
	* varpool.c: Include omp-device.h instead of omp-low.h.
	* gengtype.c (open_base_files): Replace omp-low.h with omp-device.h in
	ifiles.

gcc/c-family:
	* c-omp.c: Include omp-general.h instead of omp-low.h.
	(c_finish_oacc_wait): Adjusted call to find_omp_clause to use its new
	name.

gcc/c/
	* c-parser.c: Include omp-general.h and omp-device.h instead of
	omp-low.h.
	(c_finish_oacc_routine): Adjusted call to
	get_oacc_fn_attrib, build_oacc_routine_dims and replace_oacc_fn_attrib
	to use their new names.
	(c_parser_oacc_enter_exit_data): Adjusted call to find_omp_clause to
	use its new name.
	(c_parser_oacc_update): Likewise.
	(c_parser_omp_simd): Likewise.
	(c_parser_omp_target_update): Likewise.
	* c-typeck.c: Include omp-general.h instead of omp-low.h.
	(c_finish_omp_cancel): Adjusted call to find_omp_clause to use its new
	name.
	(c_finish_omp_cancellation_point): Likewise.
	* gimple-parser.c: Do not include omp-low.h

gcc/cp/
	* parser.c: Include omp-general.h and omp-device.h instead of
	omp-low.h.
	(cp_parser_omp_simd): Adjusted calls to find_omp_clause to use its new
	name.
	(cp_parser_omp_target_update): Likewise.
	(cp_parser_oacc_declare): Likewise.
	(cp_parser_oacc_enter_exit_data): Likewise.
	(cp_parser_oacc_update): Likewise.
	(cp_finalize_oacc_routine): Adjusted call to get_oacc_fn_attrib,
	build_oacc_routine_dims and replace_oacc_fn_attrib to use their new
	names.
	* semantics.c: Include omp-general insteda of omp-low.h.
	(finish_omp_for): Adjusted calls to find_omp_clause to use its new
	name.
	(finish_omp_cancel): Likewise.
	(finish_omp_cancellation_point): Likewise.

fortran/
	* trans-openmp.c: Include omp-general.h.

[-- Attachment #2: 0001-Split-omp-low-into-multiple-files.patch.gz --]
[-- Type: application/x-gzip, Size: 232750 bytes --]

Re: [PATCH] omp-low.c split

[Alexander Monakov]

Hi Martin,

Just one quick question -- do you know if config/nvptx/nvptx.c needs changes
with this patch?  I see it has an '#include "omp-low.h"', and it seems your
patch is renaming some functions -- is the intention that no interfaces used in
target-specific files are changed during the split?

Thanks.
Alexander

Re: [PATCH] omp-low.c split

[Martin Jambor]

Hi,

On Fri, Dec 09, 2016 at 04:25:10PM +0300, Alexander Monakov wrote:
> Hi Martin,
> 
> Just one quick question -- do you know if config/nvptx/nvptx.c needs changes
> with this patch?  I see it has an '#include "omp-low.h"', and it seems your
> patch is renaming some functions -- is the intention that no interfaces used in
> target-specific files are changed during the split?
> 

Unfortunately no, that file also needs to be changed, even if very
slightly.  Specifically, omp-general.h also needs to be included and
calls to get_oacc_fn_attrib need to be changed to call
oacc_get_fn_attrib.  omp-low.h has to stay included for
omp_reduction_init_op and omp_reduction_init which did not change.

Sorry about that, it was the only file in the back-ends and I forgot
about it.  I have added the following to my patch but it would be
great if you verified it still compiles and works as expected for you.

Thanks,

Martin


	* config/nvptx/nvptx.c: Include omp-generic.c.
	(nvptx_expand_call): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	(nvptx_reorg): Likewise.
	(nvptx_record_offload_symbol): Likewise.


diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 405a91b..17fe551 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -55,6 +55,7 @@
 #include "gimple.h"
 #include "stor-layout.h"
 #include "builtins.h"
+#include "omp-general.h"
 #include "omp-low.h"
 #include "gomp-constants.h"
 #include "dumpfile.h"
@@ -1389,7 +1390,7 @@ nvptx_expand_call (rtx retval, rtx address)
 	  if (DECL_STATIC_CHAIN (decl))
 	    cfun->machine->has_chain = true;
 
-	  tree attr = get_oacc_fn_attrib (decl);
+	  tree attr = oacc_get_fn_attrib (decl);
 	  if (attr)
 	    {
 	      tree dims = TREE_VALUE (attr);
@@ -4090,7 +4091,7 @@ nvptx_reorg (void)
   /* Determine launch dimensions of the function.  If it is not an
      offloaded function  (i.e. this is a regular compiler), the
      function has no neutering.  */
-  tree attr = get_oacc_fn_attrib (current_function_decl);
+  tree attr = oacc_get_fn_attrib (current_function_decl);
   if (attr)
     {
       /* If we determined this mask before RTL expansion, we could
@@ -4243,7 +4244,7 @@ nvptx_record_offload_symbol (tree decl)
 
     case FUNCTION_DECL:
       {
-	tree attr = get_oacc_fn_attrib (decl);
+	tree attr = oacc_get_fn_attrib (decl);
 	/* OpenMP offloading does not set this attribute.  */
 	tree dims = attr ? TREE_VALUE (attr) : NULL_TREE;
 

Re: [PATCH] omp-low.c split

[Jakub Jelinek]

On Fri, Dec 09, 2016 at 02:53:41PM +0100, Martin Jambor wrote:
> Unfortunately no, that file also needs to be changed, even if very
> slightly.  Specifically, omp-general.h also needs to be included and
> calls to get_oacc_fn_attrib need to be changed to call
> oacc_get_fn_attrib.  omp-low.h has to stay included for
> omp_reduction_init_op and omp_reduction_init which did not change.
> 
> Sorry about that, it was the only file in the back-ends and I forgot
> about it.  I have added the following to my patch but it would be
> great if you verified it still compiles and works as expected for you.

The patch including this looks mostly good to me, but can we take it
also as an opportunity to clean up formatting where it went wrong over the
years?
contrib/check_GNU_style.sh 0001-Split-omp-low-into-multiple-files.patch
reports lots of issues, as always, the script is not perfect and one needs
to use reasonable judgement.
There are e.g. 9 lines with over 80 chars, 19 lines with blocks of 8 spaces
instead of tabs, some trailing whitespaces, "Dot, space, space, new
sentence", "Dot, space, space, end of comment", some of the
"There should be exactly one space between function name and parenthesis"
- for omp directives in comments it should stay as is, but
oacc_loop_sibling_nreverse  (loop->child);
if (__builtin_expect(zero, false)) goto zero_iter_bb;
tgt = &OMP_CLAUSE_CHAIN(c);
should be fixed.
Space before dot are all false positives, etc.
Trailing operator seems to also have 2 real bugs.

Can you post an incremental patch fixing those issues?

Thanks.

	Jakub

Re: [PATCH] omp-low.c split

[Alexander Monakov]

On Fri, 9 Dec 2016, Jakub Jelinek wrote:
> Can you post an incremental patch fixing those issues?

A few small nits I found while reading the patch.

First of all, please use 'git diff --patience' (or --histogram) when
generating such patches, without it the changes in omp-low.c look uglier than
necessary.

The comment for 'struct oacc_loop' in new file omp-device.c is misplaced: for
some reason it's above the #include block.

This patch doesn't seem to apply to current trunk due to a conflict in
cp/parser.c.  If you could create a git branch (perhaps in your personal
namespace so you can freely rebase it) with this patchset, I'd appreciate it.

When trying to apply the patch, git notes a few remaining whitespace issues:

$ zcat 0001-Split-omp-low-into-multiple-files.patch.gz | git apply --reject -

<stdin>:2734: space before tab in indent.
        # BLOCK 2 (PAR_ENTRY_BB)
<stdin>:5346: space before tab in indent.
                                true, GSI_SAME_STMT);
<stdin>:8129: space before tab in indent.
             after a stmt, not before.  */
<stdin>:9060: space before tab in indent.
                                  GOMP_atomic_start ();
<stdin>:9061: space before tab in indent.
                                  *addr = rhs;


Hope this helps.
Alexander

Re: [PATCH] omp-low.c split

[Martin Jambor]

[-- Attachment #1: Type: text/plain, Size: 16693 bytes --]

Hi,

On Fri, Dec 09, 2016 at 03:22:04PM +0100, Jakub Jelinek wrote:
> On Fri, Dec 09, 2016 at 02:53:41PM +0100, Martin Jambor wrote:
> > Unfortunately no, that file also needs to be changed, even if very
> > slightly.  Specifically, omp-general.h also needs to be included and
> > calls to get_oacc_fn_attrib need to be changed to call
> > oacc_get_fn_attrib.  omp-low.h has to stay included for
> > omp_reduction_init_op and omp_reduction_init which did not change.
> > 
> > Sorry about that, it was the only file in the back-ends and I forgot
> > about it.  I have added the following to my patch but it would be
> > great if you verified it still compiles and works as expected for you.
> 
> The patch including this looks mostly good to me, but can we take it
> also as an opportunity to clean up formatting where it went wrong over the
> years?

Sure.  I have found out that I myself had erroneously my editor set to
warn only after I exceeded 81 characters on a line, so a few of those
violations were actually my fault.  (I have actually fixed this
without noticing a few weeks ago but I'm afraid I have added quite a
few lines like that in the meantime.)

> contrib/check_GNU_style.sh 0001-Split-omp-low-into-multiple-files.patch
> reports lots of issues, as always, the script is not perfect and one needs
> to use reasonable judgement.
> There are e.g. 9 lines with over 80 chars, 19 lines with blocks of 8 spaces
> instead of tabs, some trailing whitespaces, "Dot, space, space, new
> sentence", "Dot, space, space, end of comment", some of the
> "There should be exactly one space between function name and parenthesis"
> - for omp directives in comments it should stay as is, but
> oacc_loop_sibling_nreverse  (loop->child);
> if (__builtin_expect(zero, false)) goto zero_iter_bb;
> tgt = &OMP_CLAUSE_CHAIN(c);
> should be fixed.
> Space before dot are all false positives, etc.
> Trailing operator seems to also have 2 real bugs.
> 
> Can you post an incremental patch fixing those issues?

I'm attaching a refreshed patch (on top of revision 243504, git mirror
commit-id 2f757f2) and another one which fixes many of the violations
reported by contrib/check_GNU_style.sh.  Both patches were generated
from git with --patience parameter, as Alex suggested, which actually
resulted in smaller files, both in terms of the actual size and line
numbers.  I have also moved the comment above includes in omp-device.c
which Alex found to the appropriate place.

When I run the combined patch through check_GNU_style.sh, I see:

  - "Space before dot" errors which are in comments and thus false
    positives.

  - "Sentences should end with a dot.  Dot, space, space, end of the
    comment" errors, but all of them are short comments which do not
    form a sentence, and reasonably so, and thus do not end with a
    dot.  (I have fixed a two or three which were sentences and did
    not end with a dot).

  - "There should be exactly one space between function name and
    parenthesis" errors but all of them are in comments or strings.

  - "There should be no space before a left square bracket" errors
    with the same reason why they are false positives.

  - "Braces should be on a separate line" errors, some of which are in
    comments and others are one-line functions in our pass classes.
    The latter seem to be the norm elsewhere and so I left them as
    they were.


I have bootstrapped the two patches on aarch64-linux and bootstrapped
and tested them on x86_64-linux.  What do you think?

Thanks,

Martin


2016-12-08  Martin Jambor  <mjambor@suse.cz>

	* omp-general.h: New file.
	* omp-expand.h: Likewise.
	* omp-expand.c: Likewise.
	* omp-device.c: Likewise.
	* omp-low.h: Include omp-general.h and omp-grid.h.  Removed includes
	of params.h, symbol-summary.h, lto-section-names.h, cilk.h, tree-eh.h,
	ipa-prop.h, tree-cfgcleanup.h, cfgloop.h, except.h, expr.h, stmt.h,
	varasm.h, calls.h, explow.h, dojump.h, flags.h, tree-into-ssa.h,
	tree-cfg.h, cfganal.h, alias.h, emit-rtl.h, optabs.h, expmed.h,
	alloc-pool.h, cfghooks.h, rtl.h and memmodel.h.
	(omp_find_combined_for): Declare.
	(find_omp_clause): Renamed to omp_find_clause and moved to
	omp-general.h.
	(free_omp_regions): Renamed to omp_free_regions and moved to
	omp-expand.h.
	(replace_oacc_fn_attrib): Renamed to oacc_replace_fn_attrib and moved
	to omp-general.h.
	(set_oacc_fn_attrib): Renamed to oacc_set_fn_attrib and moved to
	omp-general.h.
	(build_oacc_routine_dims): Renamed to oacc_build_routine_dims and
	moved to omp-general.h.
	(get_oacc_fn_attrib): Renamed to oacc_get_fn_attrib and moved to
	omp-general.h.
	(oacc_fn_attrib_kernels_p): Moved to omp-general.h.
	(get_oacc_fn_dim_size): Renamed to oacc_get_fn_dim_size and moved to
	omp-general.c.
	(omp_expand_local): Moved to omp-expand.h.
	(make_gimple_omp_edges): Renamed to omp_make_gimple_edges and moved to
	omp-expand.h.
	(omp_finish_file): Moved to omp-device.h.
	(default_goacc_validate_dims): Renamed to
	oacc_default_goacc_validate_dims and moved to omp-device.h.
	(offload_funcs, offload_vars): Moved to omp-device.h.
	* omp-low.c: Include omp-general.h, omp-device.h and omp-grid.h.
	(omp_region): Moved to omp-expand.c.
	(omp_for_data_loop): Moved to omp-general.h.
	(omp_for_data): Likewise.
	(oacc_loop): Moved to omp-device.c.
	(oacc_loop_flags): Moved to omp-general.h.
	(offload_funcs, offload_vars): Moved to omp-device.c.
	(root_omp_region): Moved to omp-expand.c.
	(omp_any_child_fn_dumped): Likewise.
	(find_omp_clause): Renamed to omp_find_clause and moved to
	omp-general.c.
	(is_combined_parallel): kMoved to omp-expand.c.
	(is_reference): Renamed to omp_is_reference and and moved to
	omp-general.c.
	(adjust_for_condition): Renamed to omp_adjust_for_condition and moved
	to omp-general.c.
	(get_omp_for_step_from_incr): Renamed to omp_get_for_step_from_incr
	and moved to omp-general.c.
	(extract_omp_for_data): Renamed to omp_extract_for_data and moved to
	omp-general.c.
	(workshare_safe_to_combine_p): Moved to omp-expand.c.
	(omp_adjust_chunk_size): Likewise.
	(get_ws_args_for): Likewise.
	(get_base_type): Removed.
	(dump_omp_region): Moved to omp-expand.c.
	(debug_omp_region): Likewise.
	(debug_all_omp_regions): Likewise.
	(new_omp_region): Likewise.
	(free_omp_region_1): Likewise.
	(free_omp_regions): Renamed to omp_free_regions and moved to
	omp-expand.c.
	(find_combined_for): Renamed to omp_find_combined_for, made global.
	(build_omp_barrier): Renamed to omp_build_barrier and moved to
	omp-general.c.
	(omp_max_vf): Moved to omp-general.c.
	(omp_max_simt_vf): Likewise.
	(gimple_build_cond_empty): Moved to omp-expand.c.
	(parallel_needs_hsa_kernel_p): Likewise.
	(expand_omp_build_assign): Moved declaration to omp-expand.c.
	(expand_parallel_call): Moved to omp-expand.c.
	(expand_cilk_for_call): Likewise.
	(expand_task_call): Likewise.
	(vec2chain): Likewise.
	(remove_exit_barrier): Likewise.
	(remove_exit_barriers): Likewise.
	(optimize_omp_library_calls): Likewise.
	(expand_omp_regimplify_p): Likewise.
	(expand_omp_build_assign): Likewise.
	(expand_omp_taskreg): Likewise.
	(oacc_collapse): Likewise.
	(expand_oacc_collapse_init): Likewise.
	(expand_oacc_collapse_vars): Likewise.
	(expand_omp_for_init_counts): Likewise.
	(expand_omp_for_init_vars): Likewise.
	(extract_omp_for_update_vars): Likewise.
	(expand_omp_ordered_source): Likewise.
	(expand_omp_ordered_sink): Likewise.
	(expand_omp_ordered_source_sink): Likewise.
	(expand_omp_for_ordered_loops): Likewise.
	(expand_omp_for_generic): Likewise.
	(expand_omp_for_static_nochunk): Likewise.
	(find_phi_with_arg_on_edge): Likewise.
	(expand_omp_for_static_chunk): Likewise.
	(expand_cilk_for): Likewise.
	(expand_omp_simd): Likewise.
	(expand_omp_taskloop_for_outer): Likewise.
	(expand_omp_taskloop_for_inner): Likewise.
	(expand_oacc_for): Likewise.
	(expand_omp_for): Likewise.
	(expand_omp_sections): Likewise.
	(expand_omp_single): Likewise.
	(expand_omp_synch): Likewise.
	(expand_omp_atomic_load): Likewise.
	(expand_omp_atomic_store): Likewise.
	(expand_omp_atomic_fetch_op): Likewise.
	(expand_omp_atomic_pipeline): Likewise.
	(expand_omp_atomic_mutex): Likewise.
	(expand_omp_atomic): Likewise.
	(oacc_launch_pack): and moved to omp-general.c, made public.
	(OACC_FN_ATTRIB): Likewise.
	(replace_oacc_fn_attrib): Renamed to oacc_replace_fn_attrib and moved
	to omp-general.c.
	(set_oacc_fn_attrib): Renamed to oacc_set_fn_attrib and moved to
	omp-general.c.
	(build_oacc_routine_dims): Renamed to oacc_build_routine_dims and
	moved to omp-general.c.
	(get_oacc_fn_attrib): Renamed to oacc_get_fn_attrib and moved to
	omp-general.c.
	(oacc_fn_attrib_kernels_p): Moved to omp-general.c.
	(oacc_fn_attrib_level): Moved to omp-device.c.
	(get_oacc_fn_dim_size): Renamed to oacc_get_fn_dim_size and moved to
	omp-general.c.
	(get_oacc_ifn_dim_arg): Renamed to oacc_get_ifn_dim_arg and moved to
	omp-general.c.
	(mark_loops_in_oacc_kernels_region): Moved to omp-expand.c.
	(grid_launch_attributes_trees): Likewise.
	(grid_attr_trees): Likewise.
	(grid_create_kernel_launch_attr_types): Likewise.
	(grid_insert_store_range_dim): Likewise.
	(grid_get_kernel_launch_attributes): Likewise.
	(get_target_argument_identifier_1): Likewise.
	(get_target_argument_identifier): Likewise.
	(get_target_argument_value): Likewise.
	(push_target_argument_according_to_value): Likewise.
	(get_target_arguments): Likewise.
	(expand_omp_target): Likewise.
	(grid_expand_omp_for_loop): Moved to omp-grid.c.
	(grid_arg_decl_map): Likewise.
	(grid_remap_kernel_arg_accesses): Likewise.
	(grid_expand_target_grid_body): Likewise.
	(expand_omp): Renamed to omp_expand and moved to omp-expand.c.
	(build_omp_regions_1): Moved to omp-expand.c.
	(build_omp_regions_root): Likewise.
	(omp_expand_local): Likewise.
	(build_omp_regions): Likewise.
	(execute_expand_omp): Likewise.
	(pass_data_expand_omp): Likewise.
	(pass_expand_omp): Likewise.
	(make_pass_expand_omp): Likewise.
	(pass_data_expand_omp_ssa): Likewise.
	(pass_expand_omp_ssa): Likewise.
	(make_pass_expand_omp_ssa): Likewise.
	(grid_lastprivate_predicate): Renamed to
	omp_grid_lastprivate_predicate and moved to omp-grid.c, made public.
	(grid_prop): Moved to omp-grid.c.
	(GRID_MISSED_MSG_PREFIX): Likewise.
	(grid_safe_assignment_p): Likewise.
	(grid_seq_only_contains_local_assignments): Likewise.
	(grid_find_single_omp_among_assignments_1): Likewise.
	(grid_find_single_omp_among_assignments): Likewise.
	(grid_find_ungridifiable_statement): Likewise.
	(grid_parallel_clauses_gridifiable): Likewise.
	(grid_inner_loop_gridifiable_p): Likewise.
	(grid_dist_follows_simple_pattern): Likewise.
	(grid_gfor_follows_tiling_pattern): Likewise.
	(grid_call_permissible_in_distribute_p): Likewise.
	(grid_handle_call_in_distribute): Likewise.
	(grid_dist_follows_tiling_pattern): Likewise.
	(grid_target_follows_gridifiable_pattern): Likewise.
	(grid_remap_prebody_decls): Likewise.
	(grid_var_segment): Likewise.
	(grid_mark_variable_segment): Likewise.
	(grid_copy_leading_local_assignments): Likewise.
	(grid_process_grid_body): Likewise.
	(grid_eliminate_combined_simd_part): Likewise.
	(grid_mark_tiling_loops): Likewise.
	(grid_mark_tiling_parallels_and_loops): Likewise.
	(grid_process_kernel_body_copy): Likewise.
	(grid_attempt_target_gridification): Likewise.
	(grid_gridify_all_targets_stmt): Likewise.
	(grid_gridify_all_targets): Renamed to omp_grid_gridify_all_targets
	and moved to omp-grid.c, made public.
	(make_gimple_omp_edges): Renamed to omp_make_gimple_edges and moved to
	omp-expand.c.
	(add_decls_addresses_to_decl_constructor): Moved to omp-device.c.
	(omp_finish_file): Likewise.
	(oacc_thread_numbers): Likewise.
	(oacc_xform_loop): Likewise.
	(oacc_default_dims, oacc_min_dims): Likewise.
	(oacc_parse_default_dims): Likewise.
	(oacc_validate_dims): Likewise.
	(new_oacc_loop_raw): Likewise.
	(new_oacc_loop_outer): Likewise.
	(new_oacc_loop): Likewise.
	(new_oacc_loop_routine): Likewise.
	(finish_oacc_loop): Likewise.
	(free_oacc_loop): Likewise.
	(dump_oacc_loop_part): Likewise.
	(dump_oacc_loop): Likewise.
	(debug_oacc_loop): Likewise.
	(oacc_loop_discover_walk): Likewise.
	(oacc_loop_sibling_nreverse): Likewise.
	(oacc_loop_discovery): Likewise.
	(oacc_loop_xform_head_tail): Likewise.
	(oacc_loop_xform_loop): Likewise.
	(oacc_loop_process): Likewise.
	(oacc_loop_fixed_partitions): Likewise.
	(oacc_loop_auto_partitions): Likewise.
	(oacc_loop_partition): Likewise.
	(default_goacc_fork_join): Likewise.
	(default_goacc_reduction): Likewise.
	(execute_oacc_device_lower): Likewise.
	(default_goacc_validate_dims): Likewise.
	(default_goacc_dim_limit): Likewise.
	(pass_data_oacc_device_lower): Likewise.
	(pass_oacc_device_lower): Likewise.
	(make_pass_oacc_device_lower): Likewise.
	(execute_omp_device_lower): Likewise.
	(pass_data_omp_device_lower): Likewise.
	(pass_omp_device_lower): Likewise.
	(make_pass_omp_device_lower): Likewise.
	(pass_data_omp_target_link): Likewise.
	(pass_omp_target_link): Likewise.
	(find_link_var_op): Likewise.
	(pass_omp_target_link::execute): Likewise.
	(make_pass_omp_target_link): Likewise.

	* Makefile.in (OBJS): Added omp-device.o, omp-expand.o, omp-general.o
	and omp-grid.o.
	(GTFILES): Added omp-device.h, omp-device.c and omp-expand.c, removed
	omp-low.h.
	* gimple-fold.c: Include omp-general.h instead of omp-low.h.
	(fold_internal_goacc_dim): Adjusted calls to
	get_oacc_ifn_dim_arg and get_oacc_fn_dim_size to use their new names.
	* gimplify.c: Include omp-low.h.
	(omp_notice_variable): Adjust the call to get_oacc_fn_attrib to use
	its new name.
	(gimplify_omp_task): Adjusted calls to find_omp_clause to use its new
	name.
	(gimplify_omp_for): Likewise.
	* lto-cgraph.c: Include omp-device.h instead of omp-low.h.
	* toplev.c: Include omp-device.h instead of omp-low.h.
	* tree-cfg.c: Include omp-general.h instead of omp-low.h.  Also
	include omp-expand.h.
	(make_edges_bb): Adjusted the call to make_gimple_omp_edges to use its
	new name.
	(make_edges): Adjust the call to free_omp_regions to use its new name.
	* tree-parloops.c: Include omp-general.h.
	(create_parallel_loop): Adjusted the call to set_oacc_fn_attrib to use
	its new name.
	(parallelize_loops): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	* tree-ssa-loop.c: Include omp-general.h instead of omp-low.h.
	(gate_oacc_kernels): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	* tree-vrp.c: Include omp-general.h instead of omp-low.h.
	(extract_range_basic): Adjusted calls to get_oacc_ifn_dim_arg and
	get_oacc_fn_dim_size to use their new names.
	* varpool.c: Include omp-device.h instead of omp-low.h.
	* gengtype.c (open_base_files): Replace omp-low.h with omp-device.h in
	ifiles.
	* config/nvptx/nvptx.c: Include omp-generic.c.
	(nvptx_expand_call): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	(nvptx_reorg): Likewise.
	(nvptx_record_offload_symbol): Likewise.

gcc/c-family:
	* c-omp.c: Include omp-general.h instead of omp-low.h.
	(c_finish_oacc_wait): Adjusted call to find_omp_clause to use its new
	name.

gcc/c/
	* c-parser.c: Include omp-general.h and omp-device.h instead of
	omp-low.h.
	(c_finish_oacc_routine): Adjusted call to
	get_oacc_fn_attrib, build_oacc_routine_dims and replace_oacc_fn_attrib
	to use their new names.
	(c_parser_oacc_enter_exit_data): Adjusted call to find_omp_clause to
	use its new name.
	(c_parser_oacc_update): Likewise.
	(c_parser_omp_simd): Likewise.
	(c_parser_omp_target_update): Likewise.
	* c-typeck.c: Include omp-general.h instead of omp-low.h.
	(c_finish_omp_cancel): Adjusted call to find_omp_clause to use its new
	name.
	(c_finish_omp_cancellation_point): Likewise.
	* gimple-parser.c: Do not include omp-low.h

gcc/cp/
	* parser.c: Include omp-general.h and omp-device.h instead of
	omp-low.h.
	(cp_parser_omp_simd): Adjusted calls to find_omp_clause to use its new
	name.
	(cp_parser_omp_target_update): Likewise.
	(cp_parser_oacc_declare): Likewise.
	(cp_parser_oacc_enter_exit_data): Likewise.
	(cp_parser_oacc_update): Likewise.
	(cp_finalize_oacc_routine): Adjusted call to get_oacc_fn_attrib,
	build_oacc_routine_dims and replace_oacc_fn_attrib to use their new
	names.
	* semantics.c: Include omp-general insteda of omp-low.h.
	(finish_omp_for): Adjusted calls to find_omp_clause to use its new
	name.
	(finish_omp_cancel): Likewise.
	(finish_omp_cancellation_point): Likewise.

fortran/
	* trans-openmp.c: Include omp-general.h.


2016-12-08  Martin Jambor  <mjambor@suse.cz>

	    * omp-device.c: Fix coding style.
	    * omp-expand.c: Likewise.
	    * omp-general.c: Likewise.
	    * omp-grid.c: Likewise.
	    * omp-low.c: Fix coding style of parts touched by the
	      previous splitting patch.


[-- Attachment #2: 0001-Split-omp-low-into-multiple-files.patch.gz --]
[-- Type: application/x-gzip, Size: 187077 bytes --]

[-- Attachment #3: 0002-Coding-style-fixes.patch.gz --]
[-- Type: application/x-gzip, Size: 9024 bytes --]

Re: [PATCH] omp-low.c split

[Martin Jambor]

Hi,

On Fri, Dec 09, 2016 at 07:18:54PM +0300, Alexander Monakov wrote:
> On Fri, 9 Dec 2016, Jakub Jelinek wrote:
> > Can you post an incremental patch fixing those issues?
> 
> A few small nits I found while reading the patch.
> 
> First of all, please use 'git diff --patience' (or --histogram) when
> generating such patches, without it the changes in omp-low.c look uglier than
> necessary.

OK, the new patches use --patience, the result is actually quite a bit
smaller.  Thanks for the hint.

> 
> The comment for 'struct oacc_loop' in new file omp-device.c is misplaced: for
> some reason it's above the #include block.

Oh, thanks for noticing, fixed.

> 
> This patch doesn't seem to apply to current trunk due to a conflict in
> cp/parser.c.

Hmm, I don't think so?

  $ svn st
  $ svn up
  Updating '.':
  At revision 243600.
  $ patch --dry-run -p1 < /tmp/misc-next/pat/0001-Split-omp-low-into-multiple-files.patch
  checking file gcc/Makefile.in
  checking file gcc/c-family/c-omp.c
  checking file gcc/c/c-parser.c
  checking file gcc/c/c-typeck.c
  checking file gcc/c/gimple-parser.c
  checking file gcc/config/nvptx/nvptx.c
  checking file gcc/cp/parser.c
  checking file gcc/cp/semantics.c
  checking file gcc/fortran/trans-openmp.c
  checking file gcc/gengtype.c
  checking file gcc/gimple-fold.c
  checking file gcc/gimplify.c
  checking file gcc/lto-cgraph.c
  checking file gcc/omp-device.c
  checking file gcc/omp-device.h
  checking file gcc/omp-expand.c
  checking file gcc/omp-expand.h
  checking file gcc/omp-general.c
  checking file gcc/omp-general.h
  checking file gcc/omp-grid.c
  checking file gcc/omp-grid.h
  checking file gcc/omp-low.c
  checking file gcc/omp-low.h
  checking file gcc/toplev.c
  checking file gcc/tree-cfg.c
  checking file gcc/tree-parloops.c
  checking file gcc/tree-ssa-loop.c
  checking file gcc/tree-vrp.c
  checking file gcc/varpool.c
  $ echo $?
  0

This was using the exact same file I have compressed and sent a while
ago to the mailing list.

> If you could create a git branch (perhaps in your personal
> namespace so you can freely rebase it) with this patchset, I'd appreciate it.
> 

Well... I do not see this as a long-term project so I do not really
see much value in this.  I have used git just because that is how I
conveniently send patches to machines where I bootstrap them.  I hope
this will get committed very early so that we avoid any big conflicts
in omp-low.c.  But if this drags on for a while and if our git mirror
recovers meanwhile, I can.


> When trying to apply the patch, git notes a few remaining whitespace issues:
> 
> $ zcat 0001-Split-omp-low-into-multiple-files.patch.gz | git apply --reject -
> 
> <stdin>:2734: space before tab in indent.
>         # BLOCK 2 (PAR_ENTRY_BB)
> <stdin>:5346: space before tab in indent.
>                                 true, GSI_SAME_STMT);
> <stdin>:8129: space before tab in indent.
>              after a stmt, not before.  */
> <stdin>:9060: space before tab in indent.
>                                   GOMP_atomic_start ();
> <stdin>:9061: space before tab in indent.
>                                   *addr = rhs;
> 

I hope I have addressed this when doing what Jakub suggested, please
let me know if not.

Thanks for looking at the big patch!

Martin

Re: [PATCH] omp-low.c split

[Jakub Jelinek]

On Tue, Dec 13, 2016 at 11:15:43AM +0100, Martin Jambor wrote:
> I have bootstrapped the two patches on aarch64-linux and bootstrapped
> and tested them on x86_64-linux.  What do you think?

Thanks a lot for the work.  If you wouldn't mind doing a couple of further
changes (see below), I'd appreciate it, but if you want to commit it right
away, I'm ok with that too.

@@ -4321,7 +4322,7 @@ expand_cilk_for (struct omp_region *region, struct omp_for_data *fd)
 
   tree child_fndecl
     = gimple_omp_parallel_child_fn (
-        as_a <gomp_parallel *> (last_stmt (region->outer->entry)));
+	as_a <gomp_parallel *> (last_stmt (region->outer->entry)));
   tree t, low_val = NULL_TREE, high_val = NULL_TREE;
   for (t = DECL_ARGUMENTS (child_fndecl); t; t = TREE_CHAIN (t))
     {

My preference for the above would be

  gomp_parallel *par_stmt
    = as_a <gomp_parallel *> (last_stmt (region->outer->entry));
  tree child_fndecl = gimple_omp_parallel_child_fn (par_stmt);

@@ -6428,7 +6429,7 @@ expand_omp_atomic_pipeline (basic_block load_bb, basic_block store_bb,
      floating point.  This allows the atomic operation to properly
      succeed even with NaNs and -0.0.  */
   stmt = gimple_build_cond_empty
-           (build2 (NE_EXPR, boolean_type_node,
+	   (build2 (NE_EXPR, boolean_type_node,
 		    new_storedi, old_vali));
   gsi_insert_before (&si, stmt, GSI_SAME_STMT);
 
And here

  tree ne = build2 (NE_EXPR, boolean_type_node, new_storedi, old_vali);
  stmt = gimple_build_cond_empty (ne);

@@ -442,7 +442,7 @@ omp_max_simt_vf (void)
   if (!optimize)
     return 0;
   if (ENABLE_OFFLOADING)
-    for (const char *c = getenv ("OFFLOAD_TARGET_NAMES"); c; )
+    for (const char *c = getenv ("OFFLOAD_TARGET_NAMES"); c;)

I admit I don't know what the coding style we have in this case.  Ok either way.

@@ -5860,7 +5860,7 @@ lower_omp_sections (gimple_stmt_iterator *gsi_p, omp_context *ctx)
   new_body = maybe_catch_exception (new_body);
 
   t = gimple_build_omp_return
-        (!!omp_find_clause (gimple_omp_sections_clauses (stmt),
+	(!!omp_find_clause (gimple_omp_sections_clauses (stmt),
 			    OMP_CLAUSE_NOWAIT));
   gimple_seq_add_stmt (&new_body, t);
   maybe_add_implicit_barrier_cancel (ctx, &new_body);
@@ -6023,7 +6023,7 @@ lower_omp_single (gimple_stmt_iterator *gsi_p, omp_context *ctx)
   bind_body = maybe_catch_exception (bind_body);
 
   t = gimple_build_omp_return
-        (!!omp_find_clause (gimple_omp_single_clauses (single_stmt),
+	(!!omp_find_clause (gimple_omp_single_clauses (single_stmt),
 			    OMP_CLAUSE_NOWAIT));
   gimple_seq_add_stmt (&bind_body_tail, t);
   maybe_add_implicit_barrier_cancel (ctx, &bind_body_tail);

And in the above 2 spots something like:
  bool nowait = omp_find_clause (gimple_omp_single_clauses (single_stmt),
				 OMP_CLAUSE_NOWAIT) != NULL_TREE;
  t = gimple_build_omp_return (nowait);

(wonder why we use t for gimple, renaming t var to g would be also nice).  But
ok either way.

@@ -8668,7 +8669,7 @@ lower_omp_1 (gimple_stmt_iterator *gsi_p, omp_context *ctx)
       break;
     case GIMPLE_TRANSACTION:
       lower_omp (gimple_transaction_body_ptr (
-                   as_a <gtransaction *> (stmt)),
+		   as_a <gtransaction *> (stmt)),
 		 ctx);

Here it fits, so no need to wrap:
      lower_omp (gimple_transaction_body_ptr (as_a <gtransaction *> (stmt)),
		 ctx);


	Jakub

Re: [PATCH] omp-low.c split

[Thomas Schwinge]

Hi!

On Fri, 9 Dec 2016 14:08:21 +0100, Martin Jambor <mjambor@suse.cz> wrote:
> this is the promised attempt at splitting omp-low.c [...]

Yay!  \o/

I have not yet had a chance to review/test this patch, but I plan to.

A few initial comments from the "bike shed departement"; I understand in
GCC sources it will not be easy to rename stuff (such as files) later, so
we should get the names agreed upon early:

Generally, I agree with your division of "omp-low.c" parts.

>   - move everything that is part of pass_oacc_device_lower,
>     pass_omp_device_lower and pass_omp_target_link to a new file
>     omp-device.h,

Should we call this file "omp-offload.c", as offloading is what this
deals with, is the term we agreed to generally use (as far as I can
tell)?

>   - move all pre-lowering gridification stuff to a new file
>     omp-grid.c.  [...]

Is that code generic enough to not call this file "omp-hsa.c" or similar?

>   - I moved stuff that was used from all over the place to a new file
>     omp-general.c (unless it would mean exposing omp_region or
>     omp_context types).

I'd have called that simply "omp.c".

> I am opened to suggestions what to do differently, names of the file
> are for example of course subject to discussion, and I absolutely
> welcome any review and checking, for one I am not going to pretend I
> understand the stuff I put into omp-device.c.  If however there is
> consensus that we should do something like this, I would like to ask
> the community to freeze omp-low.c file until this gets committed, I
> hope you understand that I am afraid of any conflicts.

I very much understand...  :-| When I had worked on the very same thing
months ago, and my changes went without review/approval for a long time,
I had spent numerous hours on keeping my patch up to date.  So, I'm happy
to see that this is now near approval!  (Even though I don't understand
what's different now from when I worked on the same thing back then...)

I hope to have time later today to review/test your actual patch.


Grüße
 Thomas

Re: [PATCH] omp-low.c split

[Jakub Jelinek]

On Tue, Dec 13, 2016 at 12:39:01PM +0100, Thomas Schwinge wrote:
> On Fri, 9 Dec 2016 14:08:21 +0100, Martin Jambor <mjambor@suse.cz> wrote:
> > this is the promised attempt at splitting omp-low.c [...]
> 
> Yay!  \o/
> 
> I have not yet had a chance to review/test this patch, but I plan to.
> 
> A few initial comments from the "bike shed departement"; I understand in
> GCC sources it will not be easy to rename stuff (such as files) later, so
> we should get the names agreed upon early:
> 
> Generally, I agree with your division of "omp-low.c" parts.
> 
> >   - move everything that is part of pass_oacc_device_lower,
> >     pass_omp_device_lower and pass_omp_target_link to a new file
> >     omp-device.h,
> 
> Should we call this file "omp-offload.c", as offloading is what this
> deals with, is the term we agreed to generally use (as far as I can
> tell)?

That would be fine with me too.

> >   - move all pre-lowering gridification stuff to a new file
> >     omp-grid.c.  [...]
> 
> Is that code generic enough to not call this file "omp-hsa.c" or similar?

And this as well.  But omp-grid.c is fine too.

> >   - I moved stuff that was used from all over the place to a new file
> >     omp-general.c (unless it would mean exposing omp_region or
> >     omp_context types).
> 
> I'd have called that simply "omp.c".

The problem with that is that the corresponding header can't be called
omp.h for obvious reasons, we already have one with very different meaning.

	Jakub

Re: [PATCH] omp-low.c split

[Martin Jambor]

Hi,

On Tue, Dec 13, 2016 at 12:43:16PM +0100, Jakub Jelinek wrote:
> On Tue, Dec 13, 2016 at 12:39:01PM +0100, Thomas Schwinge wrote:
> > On Fri, 9 Dec 2016 14:08:21 +0100, Martin Jambor <mjambor@suse.cz> wrote:
> > > this is the promised attempt at splitting omp-low.c [...]
> > 
> > Yay!  \o/
> > 
> > I have not yet had a chance to review/test this patch, but I plan to.
> > 
> > A few initial comments from the "bike shed departement"; I understand in
> > GCC sources it will not be easy to rename stuff (such as files) later, so
> > we should get the names agreed upon early:
> > 
> > Generally, I agree with your division of "omp-low.c" parts.
> > 
> > >   - move everything that is part of pass_oacc_device_lower,
> > >     pass_omp_device_lower and pass_omp_target_link to a new file
> > >     omp-device.h,
> > 
> > Should we call this file "omp-offload.c", as offloading is what this
> > deals with, is the term we agreed to generally use (as far as I can
> > tell)?
> 
> That would be fine with me too.

OK, will do.

> 
> > >   - move all pre-lowering gridification stuff to a new file
> > >     omp-grid.c.  [...]
> > 
> > Is that code generic enough to not call this file "omp-hsa.c" or similar?
> 

Not at the moment, but...

> And this as well.  But omp-grid.c is fine too.

...I prefer omp-grid.c because I plan to use gridification also for
GCN targets, though hopefully only as an optimization rather than a
hard requirement ...and in fact I still think it is a good
optimization of simple loops for execution on all CUDA-like
environments with block/thread grids because it removes conditions
which the run-time can handle better.

> 
> > >   - I moved stuff that was used from all over the place to a new file
> > >     omp-general.c (unless it would mean exposing omp_region or
> > >     omp_context types).
> > 
> > I'd have called that simply "omp.c".
> 
> The problem with that is that the corresponding header can't be called
> omp.h for obvious reasons, we already have one with very different meaning.
> 

That is exactly the reason why I chose omp-general. 

Thanks,

Martin

Re: [PATCH] omp-low.c split

[Alexander Monakov]

On Tue, 13 Dec 2016, Martin Jambor wrote:
> I have bootstrapped the two patches on aarch64-linux and bootstrapped
> and tested them on x86_64-linux.  What do you think?

Sorry for my 'false alarm' about cp/parser.c conflict in the previous mail -- I
thought I was applying your patch to trunk, but now I see my tree was outdated.

In the new patch there are 7 whitespace issues that git complains about:

zcat 0001-Split-omp-low-into-multiple-files.patch.gz |
  git apply --check --whitespace=error-all -

<stdin>:2779: space before tab in indent.
   	# BLOCK 2 (PAR_ENTRY_BB)
<stdin>:5391: space before tab in indent.
			       	true, GSI_SAME_STMT);
<stdin>:8174: space before tab in indent.
  	     after a stmt, not before.  */
<stdin>:9105: space before tab in indent.
		 		  GOMP_atomic_start ();
<stdin>:9106: space before tab in indent.
		 		  *addr = rhs;
<stdin>:9107: space before tab in indent.
		 		  GOMP_atomic_end ();
<stdin>:10327: space before tab in indent.
       	 region.  */
error: 7 lines add whitespace errors.


A couple of typos in the Changelog:

> 	(is_combined_parallel): kMoved to omp-expand.c.

s/k//

> 	* config/nvptx/nvptx.c: Include omp-generic.c.

omp-general.h :)

Alexander

Re: [PATCH] omp-low.c split

[Cesar Philippidis]

On 12/13/2016 04:42 AM, Martin Jambor wrote:

>> And this as well.  But omp-grid.c is fine too.
> 
> ...I prefer omp-grid.c because I plan to use gridification also for
> GCN targets, though hopefully only as an optimization rather than a
> hard requirement ...and in fact I still think it is a good
> optimization of simple loops for execution on all CUDA-like
> environments with block/thread grids because it removes conditions
> which the run-time can handle better.

Regarding gridification, is your cauldron talk from 2015 still current,
or have there been some significant changes?

When we first started with OpenACC we were using a lot of the existing
lower_omp_for* infrastructure handle ACC LOOPs. But there was a couple
of problems with that. First, the chunk partitioning caused a lot of
overhead, and second because of OpenACC execution model it made more
sense to write our own functions (lower_oacc_head_tail /
lower_oacc_reductions). In fact, during lowering gcc only marks where
the loops are. All of those markers get replaced and the loops get
optimized during the oaccdevlow pass which runs on the target compiler.

Right now one of the significant bottlenecks we're experiencing on nvptx
targets is with I/O. First, prior to launching a PTX kernel, libgomp
transfers each data mapping individually in a synchronous manner. I'm
debating whether it makes sense to pass in all of those data mappings to
the accelerator prior to the PTX kernel launch asynchronously, obviously
with an explicit synchronization barrier just prior to launching the kernel.

Another bottleneck involves firstprivate variables. Often, those
variables are 'scalars' and consequently, they shouldn't need explicit
data mappings. I noticed that Jakub introduced a special
GOMP_MAP_FIRSTPRIVATE_INT, which omits data mappings for integral types
with less than or equal precision to pointers. It would probably be
beneficial to expand this to reals.

The last observation is that OpenMP code in general passes a struct with
all of the data mappings to the various OMP regions/offloaded code.
That's fine, but for offloading targets, particularly nvptx, it would
probably be slightly more efficient if those OMP regions took actual
function arguments instead of a single struct. At least on nvptx
targets, in order to pass that struct to the accelerator, the runtime
must first allocate device memory for it, then copy all of the struct
contents to the device each time prior to launching a PTX kernel. A lot
of this could be bypassed because cuLaunchKernel accepts a variable
number of kernel arguments. Obviously, those arguments need to be
transferred to the accelerator one way or another, so I'm not sure yet
how beneficial this optimization would end up being.

To be clear, I'm not proposing any of these changes for gcc7. Any
changes to the above will go to gomp-4_0-branch first, then we'll port
them over to gcc8.

What type of performance problems are you experiencing with HSA?

Cesar

Re: [PATCH] omp-low.c split

[Thomas Schwinge]

Hi!

On Tue, 13 Dec 2016 13:42:23 +0100, Martin Jambor <mjambor@suse.cz> wrote:
> On Tue, Dec 13, 2016 at 12:43:16PM +0100, Jakub Jelinek wrote:
> > On Tue, Dec 13, 2016 at 12:39:01PM +0100, Thomas Schwinge wrote:
> > > On Fri, 9 Dec 2016 14:08:21 +0100, Martin Jambor <mjambor@suse.cz> wrote:
> > > > this is the promised attempt at splitting omp-low.c [...]

> > > >   - move all pre-lowering gridification stuff to a new file
> > > >     omp-grid.c.  [...]
> > > 
> > > Is that code generic enough to not call this file "omp-hsa.c" or similar?
> > 
> 
> Not at the moment, but...
> 
> > And this as well.  But omp-grid.c is fine too.
> 
> ...I prefer omp-grid.c because I plan to use gridification also for
> GCN targets, though hopefully only as an optimization rather than a
> hard requirement ...and in fact I still think it is a good
> optimization of simple loops for execution on all CUDA-like
> environments with block/thread grids because it removes conditions
> which the run-time can handle better.

That certainly is reason enough to go with the generic name -- thanks for
the explanation!

> > > >   - I moved stuff that was used from all over the place to a new file
> > > >     omp-general.c (unless it would mean exposing omp_region or
> > > >     omp_context types).
> > > 
> > > I'd have called that simply "omp.c".
> > 
> > The problem with that is that the corresponding header can't be called
> > omp.h for obvious reasons, we already have one with very different meaning.
> 
> That is exactly the reason why I chose omp-general. 

OK, that wasn't obvious to me, but yeah, it's probably best to avoid the
name clash with libgomp's omp.h.


I still couldn't allocate time to review the patch, but at least I now
have tested it -- no regressions.  As I suppose you want to commit this
as sooner than later ;-) and you already have approval as I understand
it, how about you do commit it, and I'll then later follow up, with any
changes I'd additionally like to get done.


Grüße
 Thomas

Re: [PATCH] omp-low.c split

[Martin Jambor]

Hi,

On Wed, Dec 14, 2016 at 02:14:32PM +0100, Thomas Schwinge wrote:
> 
> I still couldn't allocate time to review the patch, but at least I now
> have tested it -- no regressions.

Great, thanks!

> As I suppose you want to commit this
> as sooner than later ;-) and you already have approval as I understand
> it, how about you do commit it, and I'll then later follow up, with any
> changes I'd additionally like to get done.
> 

I am about to re-base, re-test, add the few formatting fixes that
Jakub suggested to the second patch and commit.  Hopefully all of it
today.

Martin

Re: [PATCH] omp-low.c split

[Martin Jambor]

[-- Attachment #1: Type: text/plain, Size: 18035 bytes --]

Hi,

On Tue, Dec 13, 2016 at 11:35:37AM +0100, Jakub Jelinek wrote:
> On Tue, Dec 13, 2016 at 11:15:43AM +0100, Martin Jambor wrote:
> > I have bootstrapped the two patches on aarch64-linux and bootstrapped
> > and tested them on x86_64-linux.  What do you think?
> 
> Thanks a lot for the work.  If you wouldn't mind doing a couple of further
> changes (see below), I'd appreciate it, but if you want to commit it right
> away, I'm ok with that too.

I have rebased the patches on current trunk and:

  1) updated the first patch to rename omp-device.[ch] to
     omp-offload.[ch] as suggested by Thomas and fixed the mistakes in
     ChangeLog discovered by Alex, and 

  2) updated the second patch to deal with most of the issues below
     and to remove spaces before tabs that Alex found.

I am bootstrapping the result now and because they have been approved
and since it seems that a number of people have had a chance to look
at and even test them, I will commit them after the bootstrap and
testing finishes.

The updated patches are attached, ChangeLogs are at the end of this
message.


> 
> @@ -4321,7 +4322,7 @@ expand_cilk_for (struct omp_region *region, struct omp_for_data *fd)
>  
>    tree child_fndecl
>      = gimple_omp_parallel_child_fn (
> -        as_a <gomp_parallel *> (last_stmt (region->outer->entry)));
> +	as_a <gomp_parallel *> (last_stmt (region->outer->entry)));
>    tree t, low_val = NULL_TREE, high_val = NULL_TREE;
>    for (t = DECL_ARGUMENTS (child_fndecl); t; t = TREE_CHAIN (t))
>      {
> 
> My preference for the above would be
> 
>   gomp_parallel *par_stmt
>     = as_a <gomp_parallel *> (last_stmt (region->outer->entry));
>   tree child_fndecl = gimple_omp_parallel_child_fn (par_stmt);

OK, I have changed it so.

> 
> @@ -6428,7 +6429,7 @@ expand_omp_atomic_pipeline (basic_block load_bb, basic_block store_bb,
>       floating point.  This allows the atomic operation to properly
>       succeed even with NaNs and -0.0.  */
>    stmt = gimple_build_cond_empty
> -           (build2 (NE_EXPR, boolean_type_node,
> +	   (build2 (NE_EXPR, boolean_type_node,
>  		    new_storedi, old_vali));
>    gsi_insert_before (&si, stmt, GSI_SAME_STMT);
>  
> And here
> 
>   tree ne = build2 (NE_EXPR, boolean_type_node, new_storedi, old_vali);
>   stmt = gimple_build_cond_empty (ne);

Likewise.

> 
> @@ -442,7 +442,7 @@ omp_max_simt_vf (void)
>    if (!optimize)
>      return 0;
>    if (ENABLE_OFFLOADING)
> -    for (const char *c = getenv ("OFFLOAD_TARGET_NAMES"); c; )
> +    for (const char *c = getenv ("OFFLOAD_TARGET_NAMES"); c;)
> 
> I admit I don't know what the coding style we have in this case.  Ok
> either way.

I have made this change in order to silence a complaint from
check_GNU_style.sh.  I think the only sane way to write the above is
as a while loop  :-)  But I have left it as it was,  I really wanted
to be changing things only slightly.

> 
> @@ -5860,7 +5860,7 @@ lower_omp_sections (gimple_stmt_iterator *gsi_p, omp_context *ctx)
>    new_body = maybe_catch_exception (new_body);
>  
>    t = gimple_build_omp_return
> -        (!!omp_find_clause (gimple_omp_sections_clauses (stmt),
> +	(!!omp_find_clause (gimple_omp_sections_clauses (stmt),
>  			    OMP_CLAUSE_NOWAIT));
>    gimple_seq_add_stmt (&new_body, t);
>    maybe_add_implicit_barrier_cancel (ctx, &new_body);
> @@ -6023,7 +6023,7 @@ lower_omp_single (gimple_stmt_iterator *gsi_p, omp_context *ctx)
>    bind_body = maybe_catch_exception (bind_body);
>  
>    t = gimple_build_omp_return
> -        (!!omp_find_clause (gimple_omp_single_clauses (single_stmt),
> +	(!!omp_find_clause (gimple_omp_single_clauses (single_stmt),
>  			    OMP_CLAUSE_NOWAIT));
>    gimple_seq_add_stmt (&bind_body_tail, t);
>    maybe_add_implicit_barrier_cancel (ctx, &bind_body_tail);
> 
> And in the above 2 spots something like:
>   bool nowait = omp_find_clause (gimple_omp_single_clauses (single_stmt),
> 				 OMP_CLAUSE_NOWAIT) != NULL_TREE;
>   t = gimple_build_omp_return (nowait);

I have changed it so too.

> 
> (wonder why we use t for gimple, renaming t var to g would be also nice).  But
> ok either way.

I have changed the second occurrence to use g because it was the only
use of t, but left t in the first one because it had other uses.

> 
> @@ -8668,7 +8669,7 @@ lower_omp_1 (gimple_stmt_iterator *gsi_p, omp_context *ctx)
>        break;
>      case GIMPLE_TRANSACTION:
>        lower_omp (gimple_transaction_body_ptr (
> -                   as_a <gtransaction *> (stmt)),
> +		   as_a <gtransaction *> (stmt)),
>  		 ctx);
> 
> Here it fits, so no need to wrap:
>       lower_omp (gimple_transaction_body_ptr (as_a <gtransaction *> (stmt)),
> 		 ctx);
> 
> 

I have changed this as you requested too.

Thanks,

Martin


2016-12-14  Martin Jambor  <mjambor@suse.cz>

	* omp-general.h: New file.
	* omp-general.c: New file.
	* omp-expand.h: Likewise.
	* omp-expand.c: Likewise.
	* omp-offload.h: Likewise.
	* omp-offload.c: Likewise.
	* omp-grid.c: Likewise.
	* omp-grid.c: Likewise.
	* omp-low.h: Include omp-general.h and omp-grid.h.  Removed includes
	of params.h, symbol-summary.h, lto-section-names.h, cilk.h, tree-eh.h,
	ipa-prop.h, tree-cfgcleanup.h, cfgloop.h, except.h, expr.h, stmt.h,
	varasm.h, calls.h, explow.h, dojump.h, flags.h, tree-into-ssa.h,
	tree-cfg.h, cfganal.h, alias.h, emit-rtl.h, optabs.h, expmed.h,
	alloc-pool.h, cfghooks.h, rtl.h and memmodel.h.
	(omp_find_combined_for): Declare.
	(find_omp_clause): Renamed to omp_find_clause and moved to
	omp-general.h.
	(free_omp_regions): Renamed to omp_free_regions and moved to
	omp-expand.h.
	(replace_oacc_fn_attrib): Renamed to oacc_replace_fn_attrib and moved
	to omp-general.h.
	(set_oacc_fn_attrib): Renamed to oacc_set_fn_attrib and moved to
	omp-general.h.
	(build_oacc_routine_dims): Renamed to oacc_build_routine_dims and
	moved to omp-general.h.
	(get_oacc_fn_attrib): Renamed to oacc_get_fn_attrib and moved to
	omp-general.h.
	(oacc_fn_attrib_kernels_p): Moved to omp-general.h.
	(get_oacc_fn_dim_size): Renamed to oacc_get_fn_dim_size and moved to
	omp-general.c.
	(omp_expand_local): Moved to omp-expand.h.
	(make_gimple_omp_edges): Renamed to omp_make_gimple_edges and moved to
	omp-expand.h.
	(omp_finish_file): Moved to omp-offload.h.
	(default_goacc_validate_dims): Renamed to
	oacc_default_goacc_validate_dims and moved to omp-offload.h.
	(offload_funcs, offload_vars): Moved to omp-offload.h.
	* omp-low.c: Include omp-general.h, omp-offload.h and omp-grid.h.
	(omp_region): Moved to omp-expand.c.
	(omp_for_data_loop): Moved to omp-general.h.
	(omp_for_data): Likewise.
	(oacc_loop): Moved to omp-offload.c.
	(oacc_loop_flags): Moved to omp-general.h.
	(offload_funcs, offload_vars): Moved to omp-offload.c.
	(root_omp_region): Moved to omp-expand.c.
	(omp_any_child_fn_dumped): Likewise.
	(find_omp_clause): Renamed to omp_find_clause and moved to
	omp-general.c.
	(is_combined_parallel): Moved to omp-expand.c.
	(is_reference): Renamed to omp_is_reference and and moved to
	omp-general.c.
	(adjust_for_condition): Renamed to omp_adjust_for_condition and moved
	to omp-general.c.
	(get_omp_for_step_from_incr): Renamed to omp_get_for_step_from_incr
	and moved to omp-general.c.
	(extract_omp_for_data): Renamed to omp_extract_for_data and moved to
	omp-general.c.
	(workshare_safe_to_combine_p): Moved to omp-expand.c.
	(omp_adjust_chunk_size): Likewise.
	(get_ws_args_for): Likewise.
	(get_base_type): Removed.
	(dump_omp_region): Moved to omp-expand.c.
	(debug_omp_region): Likewise.
	(debug_all_omp_regions): Likewise.
	(new_omp_region): Likewise.
	(free_omp_region_1): Likewise.
	(free_omp_regions): Renamed to omp_free_regions and moved to
	omp-expand.c.
	(find_combined_for): Renamed to omp_find_combined_for, made global.
	(build_omp_barrier): Renamed to omp_build_barrier and moved to
	omp-general.c.
	(omp_max_vf): Moved to omp-general.c.
	(omp_max_simt_vf): Likewise.
	(gimple_build_cond_empty): Moved to omp-expand.c.
	(parallel_needs_hsa_kernel_p): Likewise.
	(expand_omp_build_assign): Moved declaration to omp-expand.c.
	(expand_parallel_call): Moved to omp-expand.c.
	(expand_cilk_for_call): Likewise.
	(expand_task_call): Likewise.
	(vec2chain): Likewise.
	(remove_exit_barrier): Likewise.
	(remove_exit_barriers): Likewise.
	(optimize_omp_library_calls): Likewise.
	(expand_omp_regimplify_p): Likewise.
	(expand_omp_build_assign): Likewise.
	(expand_omp_taskreg): Likewise.
	(oacc_collapse): Likewise.
	(expand_oacc_collapse_init): Likewise.
	(expand_oacc_collapse_vars): Likewise.
	(expand_omp_for_init_counts): Likewise.
	(expand_omp_for_init_vars): Likewise.
	(extract_omp_for_update_vars): Likewise.
	(expand_omp_ordered_source): Likewise.
	(expand_omp_ordered_sink): Likewise.
	(expand_omp_ordered_source_sink): Likewise.
	(expand_omp_for_ordered_loops): Likewise.
	(expand_omp_for_generic): Likewise.
	(expand_omp_for_static_nochunk): Likewise.
	(find_phi_with_arg_on_edge): Likewise.
	(expand_omp_for_static_chunk): Likewise.
	(expand_cilk_for): Likewise.
	(expand_omp_simd): Likewise.
	(expand_omp_taskloop_for_outer): Likewise.
	(expand_omp_taskloop_for_inner): Likewise.
	(expand_oacc_for): Likewise.
	(expand_omp_for): Likewise.
	(expand_omp_sections): Likewise.
	(expand_omp_single): Likewise.
	(expand_omp_synch): Likewise.
	(expand_omp_atomic_load): Likewise.
	(expand_omp_atomic_store): Likewise.
	(expand_omp_atomic_fetch_op): Likewise.
	(expand_omp_atomic_pipeline): Likewise.
	(expand_omp_atomic_mutex): Likewise.
	(expand_omp_atomic): Likewise.
	(oacc_launch_pack): and moved to omp-general.c, made public.
	(OACC_FN_ATTRIB): Likewise.
	(replace_oacc_fn_attrib): Renamed to oacc_replace_fn_attrib and moved
	to omp-general.c.
	(set_oacc_fn_attrib): Renamed to oacc_set_fn_attrib and moved to
	omp-general.c.
	(build_oacc_routine_dims): Renamed to oacc_build_routine_dims and
	moved to omp-general.c.
	(get_oacc_fn_attrib): Renamed to oacc_get_fn_attrib and moved to
	omp-general.c.
	(oacc_fn_attrib_kernels_p): Moved to omp-general.c.
	(oacc_fn_attrib_level): Moved to omp-offload.c.
	(get_oacc_fn_dim_size): Renamed to oacc_get_fn_dim_size and moved to
	omp-general.c.
	(get_oacc_ifn_dim_arg): Renamed to oacc_get_ifn_dim_arg and moved to
	omp-general.c.
	(mark_loops_in_oacc_kernels_region): Moved to omp-expand.c.
	(grid_launch_attributes_trees): Likewise.
	(grid_attr_trees): Likewise.
	(grid_create_kernel_launch_attr_types): Likewise.
	(grid_insert_store_range_dim): Likewise.
	(grid_get_kernel_launch_attributes): Likewise.
	(get_target_argument_identifier_1): Likewise.
	(get_target_argument_identifier): Likewise.
	(get_target_argument_value): Likewise.
	(push_target_argument_according_to_value): Likewise.
	(get_target_arguments): Likewise.
	(expand_omp_target): Likewise.
	(grid_expand_omp_for_loop): Moved to omp-grid.c.
	(grid_arg_decl_map): Likewise.
	(grid_remap_kernel_arg_accesses): Likewise.
	(grid_expand_target_grid_body): Likewise.
	(expand_omp): Renamed to omp_expand and moved to omp-expand.c.
	(build_omp_regions_1): Moved to omp-expand.c.
	(build_omp_regions_root): Likewise.
	(omp_expand_local): Likewise.
	(build_omp_regions): Likewise.
	(execute_expand_omp): Likewise.
	(pass_data_expand_omp): Likewise.
	(pass_expand_omp): Likewise.
	(make_pass_expand_omp): Likewise.
	(pass_data_expand_omp_ssa): Likewise.
	(pass_expand_omp_ssa): Likewise.
	(make_pass_expand_omp_ssa): Likewise.
	(grid_lastprivate_predicate): Renamed to
	omp_grid_lastprivate_predicate and moved to omp-grid.c, made public.
	(grid_prop): Moved to omp-grid.c.
	(GRID_MISSED_MSG_PREFIX): Likewise.
	(grid_safe_assignment_p): Likewise.
	(grid_seq_only_contains_local_assignments): Likewise.
	(grid_find_single_omp_among_assignments_1): Likewise.
	(grid_find_single_omp_among_assignments): Likewise.
	(grid_find_ungridifiable_statement): Likewise.
	(grid_parallel_clauses_gridifiable): Likewise.
	(grid_inner_loop_gridifiable_p): Likewise.
	(grid_dist_follows_simple_pattern): Likewise.
	(grid_gfor_follows_tiling_pattern): Likewise.
	(grid_call_permissible_in_distribute_p): Likewise.
	(grid_handle_call_in_distribute): Likewise.
	(grid_dist_follows_tiling_pattern): Likewise.
	(grid_target_follows_gridifiable_pattern): Likewise.
	(grid_remap_prebody_decls): Likewise.
	(grid_var_segment): Likewise.
	(grid_mark_variable_segment): Likewise.
	(grid_copy_leading_local_assignments): Likewise.
	(grid_process_grid_body): Likewise.
	(grid_eliminate_combined_simd_part): Likewise.
	(grid_mark_tiling_loops): Likewise.
	(grid_mark_tiling_parallels_and_loops): Likewise.
	(grid_process_kernel_body_copy): Likewise.
	(grid_attempt_target_gridification): Likewise.
	(grid_gridify_all_targets_stmt): Likewise.
	(grid_gridify_all_targets): Renamed to omp_grid_gridify_all_targets
	and moved to omp-grid.c, made public.
	(make_gimple_omp_edges): Renamed to omp_make_gimple_edges and moved to
	omp-expand.c.
	(add_decls_addresses_to_decl_constructor): Moved to omp-offload.c.
	(omp_finish_file): Likewise.
	(oacc_thread_numbers): Likewise.
	(oacc_xform_loop): Likewise.
	(oacc_default_dims, oacc_min_dims): Likewise.
	(oacc_parse_default_dims): Likewise.
	(oacc_validate_dims): Likewise.
	(new_oacc_loop_raw): Likewise.
	(new_oacc_loop_outer): Likewise.
	(new_oacc_loop): Likewise.
	(new_oacc_loop_routine): Likewise.
	(finish_oacc_loop): Likewise.
	(free_oacc_loop): Likewise.
	(dump_oacc_loop_part): Likewise.
	(dump_oacc_loop): Likewise.
	(debug_oacc_loop): Likewise.
	(oacc_loop_discover_walk): Likewise.
	(oacc_loop_sibling_nreverse): Likewise.
	(oacc_loop_discovery): Likewise.
	(oacc_loop_xform_head_tail): Likewise.
	(oacc_loop_xform_loop): Likewise.
	(oacc_loop_process): Likewise.
	(oacc_loop_fixed_partitions): Likewise.
	(oacc_loop_auto_partitions): Likewise.
	(oacc_loop_partition): Likewise.
	(default_goacc_fork_join): Likewise.
	(default_goacc_reduction): Likewise.
	(execute_oacc_device_lower): Likewise.
	(default_goacc_validate_dims): Likewise.
	(default_goacc_dim_limit): Likewise.
	(pass_data_oacc_device_lower): Likewise.
	(pass_oacc_device_lower): Likewise.
	(make_pass_oacc_device_lower): Likewise.
	(execute_omp_device_lower): Likewise.
	(pass_data_omp_device_lower): Likewise.
	(pass_omp_device_lower): Likewise.
	(make_pass_omp_device_lower): Likewise.
	(pass_data_omp_target_link): Likewise.
	(pass_omp_target_link): Likewise.
	(find_link_var_op): Likewise.
	(pass_omp_target_link::execute): Likewise.
	(make_pass_omp_target_link): Likewise.

	* Makefile.in (OBJS): Added omp-offload.o, omp-expand.o, omp-general.o
	and omp-grid.o.
	(GTFILES): Added omp-offload.h, omp-offload.c and omp-expand.c, removed
	omp-low.h.
	* gimple-fold.c: Include omp-general.h instead of omp-low.h.
	(fold_internal_goacc_dim): Adjusted calls to
	get_oacc_ifn_dim_arg and get_oacc_fn_dim_size to use their new names.
	* gimplify.c: Include omp-low.h.
	(omp_notice_variable): Adjust the call to get_oacc_fn_attrib to use
	its new name.
	(gimplify_omp_task): Adjusted calls to find_omp_clause to use its new
	name.
	(gimplify_omp_for): Likewise.
	* lto-cgraph.c: Include omp-offload.h instead of omp-low.h.
	* toplev.c: Include omp-offload.h instead of omp-low.h.
	* tree-cfg.c: Include omp-general.h instead of omp-low.h.  Also
	include omp-expand.h.
	(make_edges_bb): Adjusted the call to make_gimple_omp_edges to use its
	new name.
	(make_edges): Adjust the call to free_omp_regions to use its new name.
	* tree-parloops.c: Include omp-general.h.
	(create_parallel_loop): Adjusted the call to set_oacc_fn_attrib to use
	its new name.
	(parallelize_loops): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	* tree-ssa-loop.c: Include omp-general.h instead of omp-low.h.
	(gate_oacc_kernels): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	* tree-vrp.c: Include omp-general.h instead of omp-low.h.
	(extract_range_basic): Adjusted calls to get_oacc_ifn_dim_arg and
	get_oacc_fn_dim_size to use their new names.
	* varpool.c: Include omp-offload.h instead of omp-low.h.
	* gengtype.c (open_base_files): Replace omp-low.h with omp-offload.h in
	ifiles.
	* config/nvptx/nvptx.c: Include omp-general.c.
	(nvptx_expand_call): Adjusted the call to get_oacc_fn_attrib to use
	its new name.
	(nvptx_reorg): Likewise.
	(nvptx_record_offload_symbol): Likewise.

gcc/c-family:
	* c-omp.c: Include omp-general.h instead of omp-low.h.
	(c_finish_oacc_wait): Adjusted call to find_omp_clause to use its new
	name.

gcc/c/
	* c-parser.c: Include omp-general.h and omp-offload.h instead of
	omp-low.h.
	(c_finish_oacc_routine): Adjusted call to
	get_oacc_fn_attrib, build_oacc_routine_dims and replace_oacc_fn_attrib
	to use their new names.
	(c_parser_oacc_enter_exit_data): Adjusted call to find_omp_clause to
	use its new name.
	(c_parser_oacc_update): Likewise.
	(c_parser_omp_simd): Likewise.
	(c_parser_omp_target_update): Likewise.
	* c-typeck.c: Include omp-general.h instead of omp-low.h.
	(c_finish_omp_cancel): Adjusted call to find_omp_clause to use its new
	name.
	(c_finish_omp_cancellation_point): Likewise.
	* gimple-parser.c: Do not include omp-low.h

gcc/cp/
	* parser.c: Include omp-general.h and omp-offload.h instead of
	omp-low.h.
	(cp_parser_omp_simd): Adjusted calls to find_omp_clause to use its new
	name.
	(cp_parser_omp_target_update): Likewise.
	(cp_parser_oacc_declare): Likewise.
	(cp_parser_oacc_enter_exit_data): Likewise.
	(cp_parser_oacc_update): Likewise.
	(cp_finalize_oacc_routine): Adjusted call to get_oacc_fn_attrib,
	build_oacc_routine_dims and replace_oacc_fn_attrib to use their new
	names.
	* semantics.c: Include omp-general insteda of omp-low.h.
	(finish_omp_for): Adjusted calls to find_omp_clause to use its new
	name.
	(finish_omp_cancel): Likewise.
	(finish_omp_cancellation_point): Likewise.

fortran/
	* trans-openmp.c: Include omp-general.h.



2016-12-14  Martin Jambor  <mjambor@suse.cz>

	    * omp-device.c: Fix coding style.
	    * omp-expand.c: Likewise.
	    * omp-general.c: Likewise.
	    * omp-grid.c: Likewise.
	    * omp-low.c: Fix coding style of parts touched by the
	      previous splitting patch.



[-- Attachment #2: 0001-Split-omp-low-into-multiple-files.patch.gz --]
[-- Type: application/x-gzip, Size: 187474 bytes --]

[-- Attachment #3: 0002-Coding-style-fixes.patch.gz --]
[-- Type: application/x-gzip, Size: 9877 bytes --]

Re: [PATCH] omp-low.c split

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

Hi!

On 2016-12-09T14:08:21+0100, Martin Jambor <mjambor@suse.cz> wrote:
> this is the promised attempt at splitting omp-low.c [...]

> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in

> @@ -2479,8 +2483,10 @@ GTFILES = $(CPP_ID_DATA_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
>    $(srcdir)/tree-scalar-evolution.c \
>    $(srcdir)/tree-ssa-operands.h \
>    $(srcdir)/tree-profile.c $(srcdir)/tree-nested.c \
> +  $(srcdir)/omp-device.h \
> +  $(srcdir)/omp-device.c \
> +  $(srcdir)/omp-expand.c \
>    $(srcdir)/omp-low.c \
> -  $(srcdir)/omp-low.h \
>    $(srcdir)/targhooks.c $(out_file) $(srcdir)/passes.c $(srcdir)/cgraphunit.c \
>    $(srcdir)/cgraphclones.c \
>    $(srcdir)/tree-phinodes.c \

'gcc/omp-device.*' eventually got renamed to 'gcc/omp-offload.*'.

OK to push the attached "Remove 'gcc/omp-offload.c' from 'GTFILES'"?

| Given that it doesn't contain any 'GTY' markers, no 'gcc/gt-omp-offload.h' file
| gets generated (and '#include'd anywhere).


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-gcc-omp-offload.c-from-GTFILES.patch --]
[-- Type: text/x-diff, Size: 1068 bytes --]

From 1af32cf74a008a48328e82a6730b984f602b9979 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 4 Aug 2021 13:41:22 +0200
Subject: [PATCH] Remove 'gcc/omp-offload.c' from 'GTFILES'

Given that it doesn't contain any 'GTY' markers, no 'gcc/gt-omp-offload.h' file
gets generated (and '#include'd anywhere).

Small fix-up for r243673 (Git commit 629b3d75c8c5a244d891a9c292bca6912d4b0dd9)
"Split omp-low into multiple files".

	gcc/
	* Makefile.in (GTFILES): Remove '$(srcdir)/omp-offload.c'.
---
 gcc/Makefile.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index a9c9b506034..a3d9ee797df 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2693,7 +2693,6 @@ GTFILES = $(CPPLIB_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
   $(srcdir)/tree-ssa-operands.h \
   $(srcdir)/tree-profile.c $(srcdir)/tree-nested.c \
   $(srcdir)/omp-offload.h \
-  $(srcdir)/omp-offload.c \
   $(srcdir)/omp-general.c \
   $(srcdir)/omp-low.c \
   $(srcdir)/targhooks.c $(out_file) $(srcdir)/passes.c \
-- 
2.30.2

Re: [PATCH] omp-low.c split

[Jakub Jelinek]

On Wed, Aug 04, 2021 at 02:40:27PM +0200, Thomas Schwinge wrote:
> Small fix-up for r243673 (Git commit 629b3d75c8c5a244d891a9c292bca6912d4b0dd9)
> "Split omp-low into multiple files".
> 
> 	gcc/
> 	* Makefile.in (GTFILES): Remove '$(srcdir)/omp-offload.c'.

Ok, thanks.
> ---
>  gcc/Makefile.in | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index a9c9b506034..a3d9ee797df 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -2693,7 +2693,6 @@ GTFILES = $(CPPLIB_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
>    $(srcdir)/tree-ssa-operands.h \
>    $(srcdir)/tree-profile.c $(srcdir)/tree-nested.c \
>    $(srcdir)/omp-offload.h \
> -  $(srcdir)/omp-offload.c \
>    $(srcdir)/omp-general.c \
>    $(srcdir)/omp-low.c \
>    $(srcdir)/targhooks.c $(out_file) $(srcdir)/passes.c \
> -- 
> 2.30.2
> 


	Jakub