Re: Don't dump low gimple functions in gimple dump

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Jun 04, 2015 at 05:02:42PM +0200, Tom de Vries wrote:
> >So why does add_new_function not do the dumping and to the correct
> >place?  That is,
> >you are dumping things twice here, once in omp expansion and then again when the
> >new function reaches omp expansion?
> >
> 
> Dumping twice doesn't happen for omp-annotated source code. But indeed,
> dumping twice does happen in ompexpssa for the parloops/ompexpssa case with
> this patch, which is confusing.  And even if we would eliminate the dumping
> when the new function reaches omp expansion, still it would be confusing
> because the dump of the function would not be the version inbetween the
> preceding and succeeding dump.

I've committed following patch to gomp-4_5-branch for this.

The state before the patch is:
1) the omp_fn children created during the pre-SSA ompexp pass are dumped
   first in the *.ssa dump and in all the following ones (these are created
   as low gimple, non-SSA)
2) the omp_cpyfn children created during the pre-SSA ompexp pass are dumped
   first in the *.omplower dump and in all the following ones (these are
   created as high gimple)
3) the omp_fn children created during the parloops/ompexpssa passes
   are dumped first post-IPA (or during IPA?) and in all the following ones
   (these are created as SSA gimple)

2) and 3) is fine, on 1) I don't like the fact that one can see the child
functions only in the SSA form, not before it.  Thus, following patch
arranges for only the 1) case to change, those functions are dumped now
into the *.ompexp dump.  As e.g. for C++ sometimes the dumped function name
is not too descriptive (e.g. <built-in>), I've hacked things up so that also
the
;; Function
headers are dumped including the assembler name, and because there can be
many child functions emitted for a single original function, I chose to
dump again the
;; Function
header for the original function if any child functions were dumped.
The original function will have then two headers in the dump file, but I
think it is more readable that way.

2015-10-22  Jakub Jelinek  <jakub@redhat.com>

	* omp-low.c: Include tree-pretty-print.h.
	(omp_any_child_fn_dumped): New variable.
	(expand_omp_taskreg, expand_omp_target): Call
	assign_assembler_name_if_neeeded on child_fn if current_function_decl
	has assembler name set, but child_fn does not.  Dump the header
	and IL of the child function when not in SSA form.
	(expand_omp): Clear omp_any_child_fn_dumped.  Dump function header
	again if we have dumped any child functions.

--- gcc/omp-low.c.jj	2015-10-22 13:41:23.279881315 +0200
+++ gcc/omp-low.c	2015-10-22 14:11:23.488003543 +0200
@@ -81,6 +81,7 @@ along with GCC; see the file COPYING3.
 #include "context.h"
 #include "lto-section-names.h"
 #include "gomp-constants.h"
+#include "tree-pretty-print.h"
 
 /* Lowering of OMP parallel and workshare constructs proceeds in two
    phases.  The first phase scans the function looking for OMP statements
@@ -244,6 +245,7 @@ static int target_nesting_level;
 static struct omp_region *root_omp_region;
 static bitmap task_shared_vars;
 static vec<omp_context *> taskreg_contexts;
+static bool omp_any_child_fn_dumped;
 
 static void scan_omp (gimple_seq *, omp_context *);
 static tree scan_omp_1_op (tree *, int *, void *);
@@ -6739,9 +6741,15 @@ expand_omp_taskreg (struct omp_region *r
       node->parallelized_function = 1;
       cgraph_node::add_new_function (child_fn, true);
 
+      bool need_asm = DECL_ASSEMBLER_NAME_SET_P (current_function_decl)
+		      && !DECL_ASSEMBLER_NAME_SET_P (child_fn);
+
       /* Fix the callgraph edges for child_cfun.  Those for cfun will be
 	 fixed in a following pass.  */
       push_cfun (child_cfun);
+      if (need_asm)
+	assign_assembler_name_if_neeeded (child_fn);
+
       if (optimize)
 	optimize_omp_library_calls (entry_stmt);
       cgraph_edge::rebuild_edges ();
@@ -6767,6 +6775,13 @@ expand_omp_taskreg (struct omp_region *r
 	verify_loop_structure ();
 #endif
       pop_cfun ();
+
+      if (dump_file && !gimple_in_ssa_p (cfun))
+	{
+	  omp_any_child_fn_dumped = true;
+	  dump_function_header (dump_file, child_fn, dump_flags);
+	  dump_function_to_file (child_fn, dump_file, dump_flags);
+	}
     }
 
   /* Emit a library call to launch the children threads.  */
@@ -11575,9 +11590,14 @@ expand_omp_target (struct omp_region *re
       vec_safe_push (offload_funcs, child_fn);
 #endif
 
+      bool need_asm = DECL_ASSEMBLER_NAME_SET_P (current_function_decl)
+		      && !DECL_ASSEMBLER_NAME_SET_P (child_fn);
+
       /* Fix the callgraph edges for child_cfun.  Those for cfun will be
 	 fixed in a following pass.  */
       push_cfun (child_cfun);
+      if (need_asm)
+	assign_assembler_name_if_neeeded (child_fn);
       cgraph_edge::rebuild_edges ();
 
 #ifdef ENABLE_OFFLOADING
@@ -11605,6 +11625,13 @@ expand_omp_target (struct omp_region *re
 	verify_loop_structure ();
 #endif
       pop_cfun ();
+
+      if (dump_file && !gimple_in_ssa_p (cfun))
+	{
+	  omp_any_child_fn_dumped = true;
+	  dump_function_header (dump_file, child_fn, dump_flags);
+	  dump_function_to_file (child_fn, dump_file, dump_flags);
+	}
     }
 
   /* Emit a library call to launch the offloading region, or do data
@@ -11892,6 +11919,7 @@ expand_omp_target (struct omp_region *re
 static void
 expand_omp (struct omp_region *region)
 {
+  omp_any_child_fn_dumped = false;
   while (region)
     {
       location_t saved_location;
@@ -11975,6 +12003,12 @@ expand_omp (struct omp_region *region)
       input_location = saved_location;
       region = region->next;
     }
+  if (omp_any_child_fn_dumped)
+    {
+      if (dump_file)
+	dump_function_header (dump_file, current_function_decl, dump_flags);
+      omp_any_child_fn_dumped = false;
+    }
 }
 
 

	Jakub