public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Improve target pass registration
@ 2016-09-30 21:12 Jakub Jelinek
  2016-10-04  8:54 ` Richard Biener
  2016-10-04 14:49 ` Alexander Monakov
  0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2016-09-30 21:12 UTC (permalink / raw)
  To: Richard Biener, Uros Bizjak; +Cc: gcc-patches

Hi!

As discussed earlier on IRC, the current way of registering target specific
passes has various issues:
1) for -da, the target specific dump files appear last, regardless on where
   exactly they appear in the pass queue, so one has to look up the sources
   or remember where the pass is invoked if e.g. looking for when certain
   change in the IL first appears
2) -fdump-rtl-stv-details and similar options don't work, the option
   processing happens before the passes are registered

This patch allows backends to provide their *-passes.def file with
instructions how to ammend passes.def, which then can be inspected in
pass-instances.def the script generates.

I've only converted the i386 backend so far, other maintainers can easily
convert their backends later on.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-30  Jakub Jelinek  <jakub@redhat.com>

	* gen-pass-instances.awk: Rewritten.
	* Makefile.in (pass-instances.def): Depend on $(PASSES_EXTRA), pass
	$(PASSES_EXTRA) after passes.def to the script.
	* config/i386/t-i386 (PASSES_EXTRA): Add i386-passes.def.
	* config/i386/i386-passes.def: New file.
	* config/i386/i386-protos.h (make_pass_insert_vzeroupper,
	make_pass_stv): Declare.
	* config/i386/i386.c (pass_stv::pass_stv): Initialize timode_p to
	false.
	(pass_stv::gate): Depending on timode_p member require TARGET_64BIT
	or !TARGET_64BIT.
	(pass_stv::clone, pass_stv::set_pass_param): New methods.
	(pass_stv::timode_p): New non-static data member.
	(ix86_option_override): Don't register passes here.

--- gcc/gen-pass-instances.awk.jj	2016-09-29 22:53:10.264776158 +0200
+++ gcc/gen-pass-instances.awk	2016-09-30 13:22:53.745373889 +0200
@@ -17,6 +17,8 @@
 # This Awk script takes passes.def and writes pass-instances.def,
 # counting the instances of each kind of pass, adding an instance number
 # to everywhere that NEXT_PASS is used.
+# Also handle INSERT_PASS_AFTER, INSERT_PASS_BEFORE and REPLACE_PASS
+# directives.
 #
 # For example, the single-instanced pass:
 #     NEXT_PASS (pass_warn_unused_result);
@@ -30,81 +32,201 @@
 # through:
 #   NEXT_PASS (pass_copy_prop, 8);
 # (currently there are 8 instances of that pass)
+#
+#     INSERT_PASS_AFTER (pass_copy_prop, 1, pass_stv);
+# will insert
+#     NEXT_PASS (pass_stv, 1);
+# after immediately after the NEXT_PASS (pass_copy_prop, 1) line,
+# similarly INSERT_PASS_BEFORE inserts immediately before that line.
+#     REPLACE_PASS (pass_copy_prop, 1, pass_stv, true);
+# will replace NEXT_PASS (pass_copy_prop, 1) line with
+#     NEXT_PASS (pass_stv, 1, true);
+# line and renumber all higher pass_copy_prop instances if any.
 
 # Usage: awk -f gen-pass-instances.awk passes.def
 
 BEGIN {
-	print "/* This file is auto-generated by gen-pass-instances.awk";
-	print "   from passes.def.  */";
+  print "/* This file is auto-generated by gen-pass-instances.awk";
+  print "   from passes.def.  */";
+  lineno = 1;
 }
 
-function handle_line()
+function parse_line(line, fnname,	len_of_call, len_of_start,
+					len_of_open, len_of_close,
+					len_of_args, args_start_at,
+					args_str, len_of_prefix,
+					call_starts_at,
+					postfix_starts_at)
 {
-	line = $0;
+  # Find call expression.
+  call_starts_at = match(line, fnname " \\(.+\\)");
+  if (call_starts_at == 0)
+    return 0;
+
+  # Length of the call expression.
+  len_of_call = RLENGTH;
+
+  len_of_start = length(fnname " (");
+  len_of_open = length("(");
+  len_of_close = length(")");
+
+  # Find arguments
+  len_of_args = len_of_call - (len_of_start + len_of_close);
+  args_start_at = call_starts_at + len_of_start;
+  args_str = substr(line, args_start_at, len_of_args);
+  split(args_str, args, ",");
+
+  # Find call expression prefix
+  len_of_prefix = call_starts_at - 1;
+  prefix = substr(line, 1, len_of_prefix);
+
+  # Find call expression postfix
+  postfix_starts_at = call_starts_at + len_of_call;
+  postfix = substr(line, postfix_starts_at);
+  return 1;
+}
 
-	# Find call expression.
-	call_starts_at = match(line, /NEXT_PASS \(.+\)/);
-	if (call_starts_at == 0)
-	{
-		print line;
-		return;
-	}
+function adjust_linenos(above, increment,	p, i)
+{
+  for (p in pass_lines)
+    if (pass_lines[p] >= above)
+      pass_lines[p] += pass_lines[p];
+  if (increment > 0)
+    for (i = lineno - 1; i >= above; i--)
+      lines[i + increment] = lines[i];
+  else
+    for (i = above; i < lineno; i++)
+      lines[i + increment] = lines[i];
+  lineno += increment;
+}
 
-	# Length of the call expression.
-	len_of_call = RLENGTH;
+function insert_remove_pass(line, fnname)
+{
+  parse_line($0, fnname);
+  pass_name = args[1];
+  if (pass_name == "PASS")
+    return 1;
+  pass_num = args[2] + 0;
+  new_line = prefix "NEXT_PASS (" args[3];
+  if (args[4])
+    new_line = new_line ", " args[4];
+  new_line = new_line ")" postfix;
+  if (!pass_lines[pass_name, pass_num])
+    {
+      print "ERROR: Can't locate instance of the pass mentioned in " fnname;
+      return 1;
+    }
+  return 0;
+}
 
-	len_of_start = length("NEXT_PASS (");
-	len_of_open = length("(");
-	len_of_close = length(")");
-
-	# Find arguments
-	len_of_args = len_of_call - (len_of_start + len_of_close);
-	args_start_at = call_starts_at + len_of_start;
-	args_str = substr(line, args_start_at, len_of_args);
-	split(args_str, args, ",");
-
-	# Set pass_name argument, an optional with_arg argument
-	pass_name = args[1];
-	with_arg = args[2];
-
-	# Find call expression prefix
-	len_of_prefix = call_starts_at - 1;
-	prefix = substr(line, 1, len_of_prefix);
-
-	# Find call expression postfix
-	postfix_starts_at = call_starts_at + len_of_call;
-	postfix = substr(line, postfix_starts_at);
-
-	# Set pass_counts
-	if (pass_name in pass_counts)
-		pass_counts[pass_name]++;
-	else
-		pass_counts[pass_name] = 1;
-
-	pass_num = pass_counts[pass_name];
-
-	# Print call expression with extra pass_num argument
-	printf "%s", prefix;
-	if (with_arg)
-	{
-		printf "NEXT_PASS_WITH_ARG";
-	}
-	else
-	{
-		printf "NEXT_PASS";
-	}
-	printf " (";
-	printf "%s", pass_name;
-	printf ", %s", pass_num;
-	if (with_arg)
+function insert_pass(line, fnname, after,		num)
+{
+  if (insert_remove_pass(line, fnname))
+    return;
+  num = pass_lines[pass_name, pass_num];
+  adjust_linenos(num + after, 1);
+  pass_name = args[3];
+  # Set pass_counts
+  if (args[3] in pass_counts)
+    pass_counts[pass_name]++;
+  else
+    pass_counts[pass_name] = 1;
+
+  pass_lines[pass_name, pass_counts[pass_name]] = num + after;
+  lines[num + after] = new_line;
+}
+
+function replace_pass(line, fnname,			num, i)
+{
+  if (insert_remove_pass(line, "REPLACE_PASS"))
+    return;
+  num = pass_lines[pass_name, pass_num];
+  for (i = pass_counts[pass_name]; i > pass_num; i--)
+    pass_lines[pass_name, i - 1] = pass_lines[pass_name, i];
+  delete pass_lines[pass_name, pass_counts[pass_name]];
+  if (pass_counts[pass_name] == 1)
+    delete pass_counts[pass_name];
+  else
+    pass_counts[pass_name]--;
+
+  pass_name = args[3];
+  # Set pass_counts
+  if (args[3] in pass_counts)
+    pass_counts[pass_name]++;
+  else
+    pass_counts[pass_name] = 1;
+
+  pass_lines[pass_name, pass_counts[pass_name]] = num;
+  lines[num] = new_line;
+}
+
+/INSERT_PASS_AFTER \(.+\)/ {
+  insert_pass($0, "INSERT_PASS_AFTER", 1);
+  next;
+}
+
+/INSERT_PASS_BEFORE \(.+\)/ {
+  insert_pass($0, "INSERT_PASS_BEFORE", 0);
+  next;
+}
+
+/REPLACE_PASS \(.+\)/ {
+  replace_pass($0, "REPLACE_PASS");
+  next;
+}
+
+{
+  ret = parse_line($0, "NEXT_PASS");
+  if (ret)
+    {
+      pass_name = args[1];
+
+      # Set pass_counts
+      if (pass_name in pass_counts)
+	pass_counts[pass_name]++;
+      else
+	pass_counts[pass_name] = 1;
+
+      pass_lines[pass_name, pass_counts[pass_name]] = lineno;
+    }
+  lines[lineno++] = $0;
+}
+
+END {
+  delete pass_counts;
+  for (i = 1; i < lineno; i++)
+    {
+      ret = parse_line(lines[i], "NEXT_PASS");
+      if (ret)
 	{
-		printf ", %s", with_arg;
+	  # Set pass_name argument, an optional with_arg argument
+	  pass_name = args[1];
+	  with_arg = args[2];
+
+	  # Set pass_counts
+	  if (pass_name in pass_counts)
+	    pass_counts[pass_name]++;
+	  else
+	    pass_counts[pass_name] = 1;
+
+	  pass_num = pass_counts[pass_name];
+
+	  # Print call expression with extra pass_num argument
+	  printf "%s", prefix;
+	  if (with_arg)
+	    printf "NEXT_PASS_WITH_ARG";
+	  else
+	    printf "NEXT_PASS";
+	  printf " (%s, %s", pass_name, pass_num;
+	  if (with_arg)
+	    printf ", %s", with_arg;
+	  printf ")%s\n", postfix;
 	}
-	printf ")%s\n", postfix;
+      else
+	print lines[i];
+    }
 }
 
-{ handle_line() }
-
 # Local Variables:
 # mode:awk
 # c-basic-offset:8
--- gcc/Makefile.in.jj	2016-09-29 22:53:15.000000000 +0200
+++ gcc/Makefile.in	2016-09-30 13:25:05.100723726 +0200
@@ -2158,9 +2158,10 @@ s-bversion: BASE-VER
 
 CFLAGS-toplev.o += -DTARGET_NAME=\"$(target_noncanonical)\"
 
-pass-instances.def: $(srcdir)/passes.def $(srcdir)/gen-pass-instances.awk
+pass-instances.def: $(srcdir)/passes.def $(PASSES_EXTRA) \
+		    $(srcdir)/gen-pass-instances.awk
 	$(AWK) -f $(srcdir)/gen-pass-instances.awk \
-	  $(srcdir)/passes.def > pass-instances.def
+	  $(srcdir)/passes.def $(PASSES_EXTRA) > pass-instances.def
 
 $(out_object_file): $(out_file)
 	$(COMPILE) $<
--- gcc/config/i386/t-i386.jj	2016-08-19 17:23:17.000000000 +0200
+++ gcc/config/i386/t-i386	2016-09-30 13:20:45.748981856 +0200
@@ -18,6 +18,7 @@
 
 OPTIONS_H_EXTRA += $(srcdir)/config/i386/stringop.def
 TM_H += $(srcdir)/config/i386/x86-tune.def
+PASSES_EXTRA += $(srcdir)/config/i386/i386-passes.def
 
 i386-c.o: $(srcdir)/config/i386/i386-c.c
 	  $(COMPILE) $<
--- gcc/config/i386/i386-passes.def.jj	2016-09-30 12:54:29.959793738 +0200
+++ gcc/config/i386/i386-passes.def	2016-09-30 14:01:31.237200032 +0200
@@ -0,0 +1,31 @@
+/* Description of target passes for IA-32 
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/*
+ Macros that should be defined used in this file:
+   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
+   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
+   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
+ */
+
+  INSERT_PASS_AFTER (pass_reload, 1, pass_insert_vzeroupper);
+  INSERT_PASS_AFTER (pass_combine, 1, pass_stv, false /* timode_p */);
+  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
+     CONSTM1_RTX generated by the STV pass can be CSEed.  */
+  INSERT_PASS_BEFORE (pass_cse2, 1, pass_stv, true /* timode_p */);
--- gcc/config/i386/i386-protos.h.jj	2016-06-24 12:59:29.000000000 +0200
+++ gcc/config/i386/i386-protos.h	2016-09-30 14:00:54.759659671 +0200
@@ -338,3 +338,9 @@ struct ix86_first_cycle_multipass_data_
 
 const addr_space_t ADDR_SPACE_SEG_FS = 1;
 const addr_space_t ADDR_SPACE_SEG_GS = 2;
+
+namespace gcc { class context; }
+class rtl_opt_pass;
+
+extern rtl_opt_pass *make_pass_insert_vzeroupper (gcc::context *);
+extern rtl_opt_pass *make_pass_stv (gcc::context *);
--- gcc/config/i386/i386.c.jj	2016-09-27 10:14:35.000000000 +0200
+++ gcc/config/i386/i386.c	2016-09-30 14:07:36.413598596 +0200
@@ -4105,13 +4105,15 @@ class pass_stv : public rtl_opt_pass
 {
 public:
   pass_stv (gcc::context *ctxt)
-    : rtl_opt_pass (pass_data_stv, ctxt)
+    : rtl_opt_pass (pass_data_stv, ctxt),
+      timode_p (false)
   {}
 
   /* opt_pass methods: */
   virtual bool gate (function *)
     {
-      return TARGET_STV && TARGET_SSE2 && optimize > 1;
+      return (timode_p ^ (TARGET_64BIT == 0))
+	     && TARGET_STV && TARGET_SSE2 && optimize > 1;
     }
 
   virtual unsigned int execute (function *)
@@ -4119,6 +4121,19 @@ public:
       return convert_scalars_to_vector ();
     }
 
+  opt_pass *clone ()
+    {
+      return new pass_stv (m_ctxt);
+    }
+
+  void set_pass_param (unsigned int n, bool param)
+    {
+      gcc_assert (n == 0);
+      timode_p = param;
+    }
+
+private:
+  bool timode_p;
 }; // class pass_stv
 
 } // anon namespace
@@ -6155,29 +6170,7 @@ ix86_option_override_internal (bool main
 static void
 ix86_option_override (void)
 {
-  opt_pass *pass_insert_vzeroupper = make_pass_insert_vzeroupper (g);
-  struct register_pass_info insert_vzeroupper_info
-    = { pass_insert_vzeroupper, "reload",
-	1, PASS_POS_INSERT_AFTER
-      };
-  opt_pass *pass_stv = make_pass_stv (g);
-  struct register_pass_info stv_info_dimode
-    = { pass_stv, "combine",
-	1, PASS_POS_INSERT_AFTER
-      };
-  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
-     CONSTM1_RTX generated by the STV pass can be CSEed.  */
-  struct register_pass_info stv_info_timode
-    = { pass_stv, "cse2",
-	1, PASS_POS_INSERT_BEFORE
-      };
-
   ix86_option_override_internal (true, &global_options, &global_options_set);
-
-
-  /* This needs to be done at start up.  It's convenient to do it here.  */
-  register_pass (&insert_vzeroupper_info);
-  register_pass (TARGET_64BIT ? &stv_info_timode : &stv_info_dimode);
 }
 
 /* Implement the TARGET_OFFLOAD_OPTIONS hook.  */


	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-09-30 21:12 [PATCH] Improve target pass registration Jakub Jelinek
@ 2016-10-04  8:54 ` Richard Biener
  2016-10-04  9:23   ` Uros Bizjak
  2016-10-04 14:49 ` Alexander Monakov
  1 sibling, 1 reply; 9+ messages in thread
From: Richard Biener @ 2016-10-04  8:54 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Uros Bizjak, gcc-patches

On Fri, 30 Sep 2016, Jakub Jelinek wrote:

> Hi!
> 
> As discussed earlier on IRC, the current way of registering target specific
> passes has various issues:
> 1) for -da, the target specific dump files appear last, regardless on where
>    exactly they appear in the pass queue, so one has to look up the sources
>    or remember where the pass is invoked if e.g. looking for when certain
>    change in the IL first appears
> 2) -fdump-rtl-stv-details and similar options don't work, the option
>    processing happens before the passes are registered
> 
> This patch allows backends to provide their *-passes.def file with
> instructions how to ammend passes.def, which then can be inspected in
> pass-instances.def the script generates.
> 
> I've only converted the i386 backend so far, other maintainers can easily
> convert their backends later on.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

I'm fine with the approach but I can't really review the awk parts
so I'd appreciate a second eye on them.

Thus, ok from the overall view, leaving to Uros and maybe some awk
expert (if none appears within a reasonable amount of time consider
this as approval anyway).

Thanks,
Richard.

> 2016-09-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* gen-pass-instances.awk: Rewritten.
> 	* Makefile.in (pass-instances.def): Depend on $(PASSES_EXTRA), pass
> 	$(PASSES_EXTRA) after passes.def to the script.
> 	* config/i386/t-i386 (PASSES_EXTRA): Add i386-passes.def.
> 	* config/i386/i386-passes.def: New file.
> 	* config/i386/i386-protos.h (make_pass_insert_vzeroupper,
> 	make_pass_stv): Declare.
> 	* config/i386/i386.c (pass_stv::pass_stv): Initialize timode_p to
> 	false.
> 	(pass_stv::gate): Depending on timode_p member require TARGET_64BIT
> 	or !TARGET_64BIT.
> 	(pass_stv::clone, pass_stv::set_pass_param): New methods.
> 	(pass_stv::timode_p): New non-static data member.
> 	(ix86_option_override): Don't register passes here.
> 
> --- gcc/gen-pass-instances.awk.jj	2016-09-29 22:53:10.264776158 +0200
> +++ gcc/gen-pass-instances.awk	2016-09-30 13:22:53.745373889 +0200
> @@ -17,6 +17,8 @@
>  # This Awk script takes passes.def and writes pass-instances.def,
>  # counting the instances of each kind of pass, adding an instance number
>  # to everywhere that NEXT_PASS is used.
> +# Also handle INSERT_PASS_AFTER, INSERT_PASS_BEFORE and REPLACE_PASS
> +# directives.
>  #
>  # For example, the single-instanced pass:
>  #     NEXT_PASS (pass_warn_unused_result);
> @@ -30,81 +32,201 @@
>  # through:
>  #   NEXT_PASS (pass_copy_prop, 8);
>  # (currently there are 8 instances of that pass)
> +#
> +#     INSERT_PASS_AFTER (pass_copy_prop, 1, pass_stv);
> +# will insert
> +#     NEXT_PASS (pass_stv, 1);
> +# after immediately after the NEXT_PASS (pass_copy_prop, 1) line,
> +# similarly INSERT_PASS_BEFORE inserts immediately before that line.
> +#     REPLACE_PASS (pass_copy_prop, 1, pass_stv, true);
> +# will replace NEXT_PASS (pass_copy_prop, 1) line with
> +#     NEXT_PASS (pass_stv, 1, true);
> +# line and renumber all higher pass_copy_prop instances if any.
>  
>  # Usage: awk -f gen-pass-instances.awk passes.def
>  
>  BEGIN {
> -	print "/* This file is auto-generated by gen-pass-instances.awk";
> -	print "   from passes.def.  */";
> +  print "/* This file is auto-generated by gen-pass-instances.awk";
> +  print "   from passes.def.  */";
> +  lineno = 1;
>  }
>  
> -function handle_line()
> +function parse_line(line, fnname,	len_of_call, len_of_start,
> +					len_of_open, len_of_close,
> +					len_of_args, args_start_at,
> +					args_str, len_of_prefix,
> +					call_starts_at,
> +					postfix_starts_at)
>  {
> -	line = $0;
> +  # Find call expression.
> +  call_starts_at = match(line, fnname " \\(.+\\)");
> +  if (call_starts_at == 0)
> +    return 0;
> +
> +  # Length of the call expression.
> +  len_of_call = RLENGTH;
> +
> +  len_of_start = length(fnname " (");
> +  len_of_open = length("(");
> +  len_of_close = length(")");
> +
> +  # Find arguments
> +  len_of_args = len_of_call - (len_of_start + len_of_close);
> +  args_start_at = call_starts_at + len_of_start;
> +  args_str = substr(line, args_start_at, len_of_args);
> +  split(args_str, args, ",");
> +
> +  # Find call expression prefix
> +  len_of_prefix = call_starts_at - 1;
> +  prefix = substr(line, 1, len_of_prefix);
> +
> +  # Find call expression postfix
> +  postfix_starts_at = call_starts_at + len_of_call;
> +  postfix = substr(line, postfix_starts_at);
> +  return 1;
> +}
>  
> -	# Find call expression.
> -	call_starts_at = match(line, /NEXT_PASS \(.+\)/);
> -	if (call_starts_at == 0)
> -	{
> -		print line;
> -		return;
> -	}
> +function adjust_linenos(above, increment,	p, i)
> +{
> +  for (p in pass_lines)
> +    if (pass_lines[p] >= above)
> +      pass_lines[p] += pass_lines[p];
> +  if (increment > 0)
> +    for (i = lineno - 1; i >= above; i--)
> +      lines[i + increment] = lines[i];
> +  else
> +    for (i = above; i < lineno; i++)
> +      lines[i + increment] = lines[i];
> +  lineno += increment;
> +}
>  
> -	# Length of the call expression.
> -	len_of_call = RLENGTH;
> +function insert_remove_pass(line, fnname)
> +{
> +  parse_line($0, fnname);
> +  pass_name = args[1];
> +  if (pass_name == "PASS")
> +    return 1;
> +  pass_num = args[2] + 0;
> +  new_line = prefix "NEXT_PASS (" args[3];
> +  if (args[4])
> +    new_line = new_line ", " args[4];
> +  new_line = new_line ")" postfix;
> +  if (!pass_lines[pass_name, pass_num])
> +    {
> +      print "ERROR: Can't locate instance of the pass mentioned in " fnname;
> +      return 1;
> +    }
> +  return 0;
> +}
>  
> -	len_of_start = length("NEXT_PASS (");
> -	len_of_open = length("(");
> -	len_of_close = length(")");
> -
> -	# Find arguments
> -	len_of_args = len_of_call - (len_of_start + len_of_close);
> -	args_start_at = call_starts_at + len_of_start;
> -	args_str = substr(line, args_start_at, len_of_args);
> -	split(args_str, args, ",");
> -
> -	# Set pass_name argument, an optional with_arg argument
> -	pass_name = args[1];
> -	with_arg = args[2];
> -
> -	# Find call expression prefix
> -	len_of_prefix = call_starts_at - 1;
> -	prefix = substr(line, 1, len_of_prefix);
> -
> -	# Find call expression postfix
> -	postfix_starts_at = call_starts_at + len_of_call;
> -	postfix = substr(line, postfix_starts_at);
> -
> -	# Set pass_counts
> -	if (pass_name in pass_counts)
> -		pass_counts[pass_name]++;
> -	else
> -		pass_counts[pass_name] = 1;
> -
> -	pass_num = pass_counts[pass_name];
> -
> -	# Print call expression with extra pass_num argument
> -	printf "%s", prefix;
> -	if (with_arg)
> -	{
> -		printf "NEXT_PASS_WITH_ARG";
> -	}
> -	else
> -	{
> -		printf "NEXT_PASS";
> -	}
> -	printf " (";
> -	printf "%s", pass_name;
> -	printf ", %s", pass_num;
> -	if (with_arg)
> +function insert_pass(line, fnname, after,		num)
> +{
> +  if (insert_remove_pass(line, fnname))
> +    return;
> +  num = pass_lines[pass_name, pass_num];
> +  adjust_linenos(num + after, 1);
> +  pass_name = args[3];
> +  # Set pass_counts
> +  if (args[3] in pass_counts)
> +    pass_counts[pass_name]++;
> +  else
> +    pass_counts[pass_name] = 1;
> +
> +  pass_lines[pass_name, pass_counts[pass_name]] = num + after;
> +  lines[num + after] = new_line;
> +}
> +
> +function replace_pass(line, fnname,			num, i)
> +{
> +  if (insert_remove_pass(line, "REPLACE_PASS"))
> +    return;
> +  num = pass_lines[pass_name, pass_num];
> +  for (i = pass_counts[pass_name]; i > pass_num; i--)
> +    pass_lines[pass_name, i - 1] = pass_lines[pass_name, i];
> +  delete pass_lines[pass_name, pass_counts[pass_name]];
> +  if (pass_counts[pass_name] == 1)
> +    delete pass_counts[pass_name];
> +  else
> +    pass_counts[pass_name]--;
> +
> +  pass_name = args[3];
> +  # Set pass_counts
> +  if (args[3] in pass_counts)
> +    pass_counts[pass_name]++;
> +  else
> +    pass_counts[pass_name] = 1;
> +
> +  pass_lines[pass_name, pass_counts[pass_name]] = num;
> +  lines[num] = new_line;
> +}
> +
> +/INSERT_PASS_AFTER \(.+\)/ {
> +  insert_pass($0, "INSERT_PASS_AFTER", 1);
> +  next;
> +}
> +
> +/INSERT_PASS_BEFORE \(.+\)/ {
> +  insert_pass($0, "INSERT_PASS_BEFORE", 0);
> +  next;
> +}
> +
> +/REPLACE_PASS \(.+\)/ {
> +  replace_pass($0, "REPLACE_PASS");
> +  next;
> +}
> +
> +{
> +  ret = parse_line($0, "NEXT_PASS");
> +  if (ret)
> +    {
> +      pass_name = args[1];
> +
> +      # Set pass_counts
> +      if (pass_name in pass_counts)
> +	pass_counts[pass_name]++;
> +      else
> +	pass_counts[pass_name] = 1;
> +
> +      pass_lines[pass_name, pass_counts[pass_name]] = lineno;
> +    }
> +  lines[lineno++] = $0;
> +}
> +
> +END {
> +  delete pass_counts;
> +  for (i = 1; i < lineno; i++)
> +    {
> +      ret = parse_line(lines[i], "NEXT_PASS");
> +      if (ret)
>  	{
> -		printf ", %s", with_arg;
> +	  # Set pass_name argument, an optional with_arg argument
> +	  pass_name = args[1];
> +	  with_arg = args[2];
> +
> +	  # Set pass_counts
> +	  if (pass_name in pass_counts)
> +	    pass_counts[pass_name]++;
> +	  else
> +	    pass_counts[pass_name] = 1;
> +
> +	  pass_num = pass_counts[pass_name];
> +
> +	  # Print call expression with extra pass_num argument
> +	  printf "%s", prefix;
> +	  if (with_arg)
> +	    printf "NEXT_PASS_WITH_ARG";
> +	  else
> +	    printf "NEXT_PASS";
> +	  printf " (%s, %s", pass_name, pass_num;
> +	  if (with_arg)
> +	    printf ", %s", with_arg;
> +	  printf ")%s\n", postfix;
>  	}
> -	printf ")%s\n", postfix;
> +      else
> +	print lines[i];
> +    }
>  }
>  
> -{ handle_line() }
> -
>  # Local Variables:
>  # mode:awk
>  # c-basic-offset:8
> --- gcc/Makefile.in.jj	2016-09-29 22:53:15.000000000 +0200
> +++ gcc/Makefile.in	2016-09-30 13:25:05.100723726 +0200
> @@ -2158,9 +2158,10 @@ s-bversion: BASE-VER
>  
>  CFLAGS-toplev.o += -DTARGET_NAME=\"$(target_noncanonical)\"
>  
> -pass-instances.def: $(srcdir)/passes.def $(srcdir)/gen-pass-instances.awk
> +pass-instances.def: $(srcdir)/passes.def $(PASSES_EXTRA) \
> +		    $(srcdir)/gen-pass-instances.awk
>  	$(AWK) -f $(srcdir)/gen-pass-instances.awk \
> -	  $(srcdir)/passes.def > pass-instances.def
> +	  $(srcdir)/passes.def $(PASSES_EXTRA) > pass-instances.def
>  
>  $(out_object_file): $(out_file)
>  	$(COMPILE) $<
> --- gcc/config/i386/t-i386.jj	2016-08-19 17:23:17.000000000 +0200
> +++ gcc/config/i386/t-i386	2016-09-30 13:20:45.748981856 +0200
> @@ -18,6 +18,7 @@
>  
>  OPTIONS_H_EXTRA += $(srcdir)/config/i386/stringop.def
>  TM_H += $(srcdir)/config/i386/x86-tune.def
> +PASSES_EXTRA += $(srcdir)/config/i386/i386-passes.def
>  
>  i386-c.o: $(srcdir)/config/i386/i386-c.c
>  	  $(COMPILE) $<
> --- gcc/config/i386/i386-passes.def.jj	2016-09-30 12:54:29.959793738 +0200
> +++ gcc/config/i386/i386-passes.def	2016-09-30 14:01:31.237200032 +0200
> @@ -0,0 +1,31 @@
> +/* Description of target passes for IA-32 
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free
> +Software Foundation; either version 3, or (at your option) any later
> +version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +/*
> + Macros that should be defined used in this file:
> +   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
> +   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
> +   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
> + */
> +
> +  INSERT_PASS_AFTER (pass_reload, 1, pass_insert_vzeroupper);
> +  INSERT_PASS_AFTER (pass_combine, 1, pass_stv, false /* timode_p */);
> +  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
> +     CONSTM1_RTX generated by the STV pass can be CSEed.  */
> +  INSERT_PASS_BEFORE (pass_cse2, 1, pass_stv, true /* timode_p */);
> --- gcc/config/i386/i386-protos.h.jj	2016-06-24 12:59:29.000000000 +0200
> +++ gcc/config/i386/i386-protos.h	2016-09-30 14:00:54.759659671 +0200
> @@ -338,3 +338,9 @@ struct ix86_first_cycle_multipass_data_
>  
>  const addr_space_t ADDR_SPACE_SEG_FS = 1;
>  const addr_space_t ADDR_SPACE_SEG_GS = 2;
> +
> +namespace gcc { class context; }
> +class rtl_opt_pass;
> +
> +extern rtl_opt_pass *make_pass_insert_vzeroupper (gcc::context *);
> +extern rtl_opt_pass *make_pass_stv (gcc::context *);
> --- gcc/config/i386/i386.c.jj	2016-09-27 10:14:35.000000000 +0200
> +++ gcc/config/i386/i386.c	2016-09-30 14:07:36.413598596 +0200
> @@ -4105,13 +4105,15 @@ class pass_stv : public rtl_opt_pass
>  {
>  public:
>    pass_stv (gcc::context *ctxt)
> -    : rtl_opt_pass (pass_data_stv, ctxt)
> +    : rtl_opt_pass (pass_data_stv, ctxt),
> +      timode_p (false)
>    {}
>  
>    /* opt_pass methods: */
>    virtual bool gate (function *)
>      {
> -      return TARGET_STV && TARGET_SSE2 && optimize > 1;
> +      return (timode_p ^ (TARGET_64BIT == 0))
> +	     && TARGET_STV && TARGET_SSE2 && optimize > 1;
>      }
>  
>    virtual unsigned int execute (function *)
> @@ -4119,6 +4121,19 @@ public:
>        return convert_scalars_to_vector ();
>      }
>  
> +  opt_pass *clone ()
> +    {
> +      return new pass_stv (m_ctxt);
> +    }
> +
> +  void set_pass_param (unsigned int n, bool param)
> +    {
> +      gcc_assert (n == 0);
> +      timode_p = param;
> +    }
> +
> +private:
> +  bool timode_p;
>  }; // class pass_stv
>  
>  } // anon namespace
> @@ -6155,29 +6170,7 @@ ix86_option_override_internal (bool main
>  static void
>  ix86_option_override (void)
>  {
> -  opt_pass *pass_insert_vzeroupper = make_pass_insert_vzeroupper (g);
> -  struct register_pass_info insert_vzeroupper_info
> -    = { pass_insert_vzeroupper, "reload",
> -	1, PASS_POS_INSERT_AFTER
> -      };
> -  opt_pass *pass_stv = make_pass_stv (g);
> -  struct register_pass_info stv_info_dimode
> -    = { pass_stv, "combine",
> -	1, PASS_POS_INSERT_AFTER
> -      };
> -  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
> -     CONSTM1_RTX generated by the STV pass can be CSEed.  */
> -  struct register_pass_info stv_info_timode
> -    = { pass_stv, "cse2",
> -	1, PASS_POS_INSERT_BEFORE
> -      };
> -
>    ix86_option_override_internal (true, &global_options, &global_options_set);
> -
> -
> -  /* This needs to be done at start up.  It's convenient to do it here.  */
> -  register_pass (&insert_vzeroupper_info);
> -  register_pass (TARGET_64BIT ? &stv_info_timode : &stv_info_dimode);
>  }
>  
>  /* Implement the TARGET_OFFLOAD_OPTIONS hook.  */
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-10-04  8:54 ` Richard Biener
@ 2016-10-04  9:23   ` Uros Bizjak
  0 siblings, 0 replies; 9+ messages in thread
From: Uros Bizjak @ 2016-10-04  9:23 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jakub Jelinek, gcc-patches

On Tue, Oct 4, 2016 at 10:53 AM, Richard Biener <rguenther@suse.de> wrote:
> On Fri, 30 Sep 2016, Jakub Jelinek wrote:
>
>> Hi!
>>
>> As discussed earlier on IRC, the current way of registering target specific
>> passes has various issues:
>> 1) for -da, the target specific dump files appear last, regardless on where
>>    exactly they appear in the pass queue, so one has to look up the sources
>>    or remember where the pass is invoked if e.g. looking for when certain
>>    change in the IL first appears
>> 2) -fdump-rtl-stv-details and similar options don't work, the option
>>    processing happens before the passes are registered
>>
>> This patch allows backends to provide their *-passes.def file with
>> instructions how to ammend passes.def, which then can be inspected in
>> pass-instances.def the script generates.
>>
>> I've only converted the i386 backend so far, other maintainers can easily
>> convert their backends later on.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> I'm fine with the approach but I can't really review the awk parts
> so I'd appreciate a second eye on them.
>
> Thus, ok from the overall view, leaving to Uros and maybe some awk
> expert (if none appears within a reasonable amount of time consider
> this as approval anyway).

I thought I have already approved what little is of x86 changes. But
as you said, most of the changes are target-independent awk scripts.

That said, the change is surely beneficial, I've been annoyed by dump
file order for quite some time...

Thanks,
Uros.

>
> Thanks,
> Richard.
>
>> 2016-09-30  Jakub Jelinek  <jakub@redhat.com>
>>
>>       * gen-pass-instances.awk: Rewritten.
>>       * Makefile.in (pass-instances.def): Depend on $(PASSES_EXTRA), pass
>>       $(PASSES_EXTRA) after passes.def to the script.
>>       * config/i386/t-i386 (PASSES_EXTRA): Add i386-passes.def.
>>       * config/i386/i386-passes.def: New file.
>>       * config/i386/i386-protos.h (make_pass_insert_vzeroupper,
>>       make_pass_stv): Declare.
>>       * config/i386/i386.c (pass_stv::pass_stv): Initialize timode_p to
>>       false.
>>       (pass_stv::gate): Depending on timode_p member require TARGET_64BIT
>>       or !TARGET_64BIT.
>>       (pass_stv::clone, pass_stv::set_pass_param): New methods.
>>       (pass_stv::timode_p): New non-static data member.
>>       (ix86_option_override): Don't register passes here.
>>
>> --- gcc/gen-pass-instances.awk.jj     2016-09-29 22:53:10.264776158 +0200
>> +++ gcc/gen-pass-instances.awk        2016-09-30 13:22:53.745373889 +0200
>> @@ -17,6 +17,8 @@
>>  # This Awk script takes passes.def and writes pass-instances.def,
>>  # counting the instances of each kind of pass, adding an instance number
>>  # to everywhere that NEXT_PASS is used.
>> +# Also handle INSERT_PASS_AFTER, INSERT_PASS_BEFORE and REPLACE_PASS
>> +# directives.
>>  #
>>  # For example, the single-instanced pass:
>>  #     NEXT_PASS (pass_warn_unused_result);
>> @@ -30,81 +32,201 @@
>>  # through:
>>  #   NEXT_PASS (pass_copy_prop, 8);
>>  # (currently there are 8 instances of that pass)
>> +#
>> +#     INSERT_PASS_AFTER (pass_copy_prop, 1, pass_stv);
>> +# will insert
>> +#     NEXT_PASS (pass_stv, 1);
>> +# after immediately after the NEXT_PASS (pass_copy_prop, 1) line,
>> +# similarly INSERT_PASS_BEFORE inserts immediately before that line.
>> +#     REPLACE_PASS (pass_copy_prop, 1, pass_stv, true);
>> +# will replace NEXT_PASS (pass_copy_prop, 1) line with
>> +#     NEXT_PASS (pass_stv, 1, true);
>> +# line and renumber all higher pass_copy_prop instances if any.
>>
>>  # Usage: awk -f gen-pass-instances.awk passes.def
>>
>>  BEGIN {
>> -     print "/* This file is auto-generated by gen-pass-instances.awk";
>> -     print "   from passes.def.  */";
>> +  print "/* This file is auto-generated by gen-pass-instances.awk";
>> +  print "   from passes.def.  */";
>> +  lineno = 1;
>>  }
>>
>> -function handle_line()
>> +function parse_line(line, fnname,    len_of_call, len_of_start,
>> +                                     len_of_open, len_of_close,
>> +                                     len_of_args, args_start_at,
>> +                                     args_str, len_of_prefix,
>> +                                     call_starts_at,
>> +                                     postfix_starts_at)
>>  {
>> -     line = $0;
>> +  # Find call expression.
>> +  call_starts_at = match(line, fnname " \\(.+\\)");
>> +  if (call_starts_at == 0)
>> +    return 0;
>> +
>> +  # Length of the call expression.
>> +  len_of_call = RLENGTH;
>> +
>> +  len_of_start = length(fnname " (");
>> +  len_of_open = length("(");
>> +  len_of_close = length(")");
>> +
>> +  # Find arguments
>> +  len_of_args = len_of_call - (len_of_start + len_of_close);
>> +  args_start_at = call_starts_at + len_of_start;
>> +  args_str = substr(line, args_start_at, len_of_args);
>> +  split(args_str, args, ",");
>> +
>> +  # Find call expression prefix
>> +  len_of_prefix = call_starts_at - 1;
>> +  prefix = substr(line, 1, len_of_prefix);
>> +
>> +  # Find call expression postfix
>> +  postfix_starts_at = call_starts_at + len_of_call;
>> +  postfix = substr(line, postfix_starts_at);
>> +  return 1;
>> +}
>>
>> -     # Find call expression.
>> -     call_starts_at = match(line, /NEXT_PASS \(.+\)/);
>> -     if (call_starts_at == 0)
>> -     {
>> -             print line;
>> -             return;
>> -     }
>> +function adjust_linenos(above, increment,    p, i)
>> +{
>> +  for (p in pass_lines)
>> +    if (pass_lines[p] >= above)
>> +      pass_lines[p] += pass_lines[p];
>> +  if (increment > 0)
>> +    for (i = lineno - 1; i >= above; i--)
>> +      lines[i + increment] = lines[i];
>> +  else
>> +    for (i = above; i < lineno; i++)
>> +      lines[i + increment] = lines[i];
>> +  lineno += increment;
>> +}
>>
>> -     # Length of the call expression.
>> -     len_of_call = RLENGTH;
>> +function insert_remove_pass(line, fnname)
>> +{
>> +  parse_line($0, fnname);
>> +  pass_name = args[1];
>> +  if (pass_name == "PASS")
>> +    return 1;
>> +  pass_num = args[2] + 0;
>> +  new_line = prefix "NEXT_PASS (" args[3];
>> +  if (args[4])
>> +    new_line = new_line ", " args[4];
>> +  new_line = new_line ")" postfix;
>> +  if (!pass_lines[pass_name, pass_num])
>> +    {
>> +      print "ERROR: Can't locate instance of the pass mentioned in " fnname;
>> +      return 1;
>> +    }
>> +  return 0;
>> +}
>>
>> -     len_of_start = length("NEXT_PASS (");
>> -     len_of_open = length("(");
>> -     len_of_close = length(")");
>> -
>> -     # Find arguments
>> -     len_of_args = len_of_call - (len_of_start + len_of_close);
>> -     args_start_at = call_starts_at + len_of_start;
>> -     args_str = substr(line, args_start_at, len_of_args);
>> -     split(args_str, args, ",");
>> -
>> -     # Set pass_name argument, an optional with_arg argument
>> -     pass_name = args[1];
>> -     with_arg = args[2];
>> -
>> -     # Find call expression prefix
>> -     len_of_prefix = call_starts_at - 1;
>> -     prefix = substr(line, 1, len_of_prefix);
>> -
>> -     # Find call expression postfix
>> -     postfix_starts_at = call_starts_at + len_of_call;
>> -     postfix = substr(line, postfix_starts_at);
>> -
>> -     # Set pass_counts
>> -     if (pass_name in pass_counts)
>> -             pass_counts[pass_name]++;
>> -     else
>> -             pass_counts[pass_name] = 1;
>> -
>> -     pass_num = pass_counts[pass_name];
>> -
>> -     # Print call expression with extra pass_num argument
>> -     printf "%s", prefix;
>> -     if (with_arg)
>> -     {
>> -             printf "NEXT_PASS_WITH_ARG";
>> -     }
>> -     else
>> -     {
>> -             printf "NEXT_PASS";
>> -     }
>> -     printf " (";
>> -     printf "%s", pass_name;
>> -     printf ", %s", pass_num;
>> -     if (with_arg)
>> +function insert_pass(line, fnname, after,            num)
>> +{
>> +  if (insert_remove_pass(line, fnname))
>> +    return;
>> +  num = pass_lines[pass_name, pass_num];
>> +  adjust_linenos(num + after, 1);
>> +  pass_name = args[3];
>> +  # Set pass_counts
>> +  if (args[3] in pass_counts)
>> +    pass_counts[pass_name]++;
>> +  else
>> +    pass_counts[pass_name] = 1;
>> +
>> +  pass_lines[pass_name, pass_counts[pass_name]] = num + after;
>> +  lines[num + after] = new_line;
>> +}
>> +
>> +function replace_pass(line, fnname,                  num, i)
>> +{
>> +  if (insert_remove_pass(line, "REPLACE_PASS"))
>> +    return;
>> +  num = pass_lines[pass_name, pass_num];
>> +  for (i = pass_counts[pass_name]; i > pass_num; i--)
>> +    pass_lines[pass_name, i - 1] = pass_lines[pass_name, i];
>> +  delete pass_lines[pass_name, pass_counts[pass_name]];
>> +  if (pass_counts[pass_name] == 1)
>> +    delete pass_counts[pass_name];
>> +  else
>> +    pass_counts[pass_name]--;
>> +
>> +  pass_name = args[3];
>> +  # Set pass_counts
>> +  if (args[3] in pass_counts)
>> +    pass_counts[pass_name]++;
>> +  else
>> +    pass_counts[pass_name] = 1;
>> +
>> +  pass_lines[pass_name, pass_counts[pass_name]] = num;
>> +  lines[num] = new_line;
>> +}
>> +
>> +/INSERT_PASS_AFTER \(.+\)/ {
>> +  insert_pass($0, "INSERT_PASS_AFTER", 1);
>> +  next;
>> +}
>> +
>> +/INSERT_PASS_BEFORE \(.+\)/ {
>> +  insert_pass($0, "INSERT_PASS_BEFORE", 0);
>> +  next;
>> +}
>> +
>> +/REPLACE_PASS \(.+\)/ {
>> +  replace_pass($0, "REPLACE_PASS");
>> +  next;
>> +}
>> +
>> +{
>> +  ret = parse_line($0, "NEXT_PASS");
>> +  if (ret)
>> +    {
>> +      pass_name = args[1];
>> +
>> +      # Set pass_counts
>> +      if (pass_name in pass_counts)
>> +     pass_counts[pass_name]++;
>> +      else
>> +     pass_counts[pass_name] = 1;
>> +
>> +      pass_lines[pass_name, pass_counts[pass_name]] = lineno;
>> +    }
>> +  lines[lineno++] = $0;
>> +}
>> +
>> +END {
>> +  delete pass_counts;
>> +  for (i = 1; i < lineno; i++)
>> +    {
>> +      ret = parse_line(lines[i], "NEXT_PASS");
>> +      if (ret)
>>       {
>> -             printf ", %s", with_arg;
>> +       # Set pass_name argument, an optional with_arg argument
>> +       pass_name = args[1];
>> +       with_arg = args[2];
>> +
>> +       # Set pass_counts
>> +       if (pass_name in pass_counts)
>> +         pass_counts[pass_name]++;
>> +       else
>> +         pass_counts[pass_name] = 1;
>> +
>> +       pass_num = pass_counts[pass_name];
>> +
>> +       # Print call expression with extra pass_num argument
>> +       printf "%s", prefix;
>> +       if (with_arg)
>> +         printf "NEXT_PASS_WITH_ARG";
>> +       else
>> +         printf "NEXT_PASS";
>> +       printf " (%s, %s", pass_name, pass_num;
>> +       if (with_arg)
>> +         printf ", %s", with_arg;
>> +       printf ")%s\n", postfix;
>>       }
>> -     printf ")%s\n", postfix;
>> +      else
>> +     print lines[i];
>> +    }
>>  }
>>
>> -{ handle_line() }
>> -
>>  # Local Variables:
>>  # mode:awk
>>  # c-basic-offset:8
>> --- gcc/Makefile.in.jj        2016-09-29 22:53:15.000000000 +0200
>> +++ gcc/Makefile.in   2016-09-30 13:25:05.100723726 +0200
>> @@ -2158,9 +2158,10 @@ s-bversion: BASE-VER
>>
>>  CFLAGS-toplev.o += -DTARGET_NAME=\"$(target_noncanonical)\"
>>
>> -pass-instances.def: $(srcdir)/passes.def $(srcdir)/gen-pass-instances.awk
>> +pass-instances.def: $(srcdir)/passes.def $(PASSES_EXTRA) \
>> +                 $(srcdir)/gen-pass-instances.awk
>>       $(AWK) -f $(srcdir)/gen-pass-instances.awk \
>> -       $(srcdir)/passes.def > pass-instances.def
>> +       $(srcdir)/passes.def $(PASSES_EXTRA) > pass-instances.def
>>
>>  $(out_object_file): $(out_file)
>>       $(COMPILE) $<
>> --- gcc/config/i386/t-i386.jj 2016-08-19 17:23:17.000000000 +0200
>> +++ gcc/config/i386/t-i386    2016-09-30 13:20:45.748981856 +0200
>> @@ -18,6 +18,7 @@
>>
>>  OPTIONS_H_EXTRA += $(srcdir)/config/i386/stringop.def
>>  TM_H += $(srcdir)/config/i386/x86-tune.def
>> +PASSES_EXTRA += $(srcdir)/config/i386/i386-passes.def
>>
>>  i386-c.o: $(srcdir)/config/i386/i386-c.c
>>         $(COMPILE) $<
>> --- gcc/config/i386/i386-passes.def.jj        2016-09-30 12:54:29.959793738 +0200
>> +++ gcc/config/i386/i386-passes.def   2016-09-30 14:01:31.237200032 +0200
>> @@ -0,0 +1,31 @@
>> +/* Description of target passes for IA-32
>> +   Copyright (C) 2016 Free Software Foundation, Inc.
>> +
>> +This file is part of GCC.
>> +
>> +GCC is free software; you can redistribute it and/or modify it under
>> +the terms of the GNU General Public License as published by the Free
>> +Software Foundation; either version 3, or (at your option) any later
>> +version.
>> +
>> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
>> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>> +for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with GCC; see the file COPYING3.  If not see
>> +<http://www.gnu.org/licenses/>.  */
>> +
>> +/*
>> + Macros that should be defined used in this file:
>> +   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
>> +   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
>> +   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
>> + */
>> +
>> +  INSERT_PASS_AFTER (pass_reload, 1, pass_insert_vzeroupper);
>> +  INSERT_PASS_AFTER (pass_combine, 1, pass_stv, false /* timode_p */);
>> +  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
>> +     CONSTM1_RTX generated by the STV pass can be CSEed.  */
>> +  INSERT_PASS_BEFORE (pass_cse2, 1, pass_stv, true /* timode_p */);
>> --- gcc/config/i386/i386-protos.h.jj  2016-06-24 12:59:29.000000000 +0200
>> +++ gcc/config/i386/i386-protos.h     2016-09-30 14:00:54.759659671 +0200
>> @@ -338,3 +338,9 @@ struct ix86_first_cycle_multipass_data_
>>
>>  const addr_space_t ADDR_SPACE_SEG_FS = 1;
>>  const addr_space_t ADDR_SPACE_SEG_GS = 2;
>> +
>> +namespace gcc { class context; }
>> +class rtl_opt_pass;
>> +
>> +extern rtl_opt_pass *make_pass_insert_vzeroupper (gcc::context *);
>> +extern rtl_opt_pass *make_pass_stv (gcc::context *);
>> --- gcc/config/i386/i386.c.jj 2016-09-27 10:14:35.000000000 +0200
>> +++ gcc/config/i386/i386.c    2016-09-30 14:07:36.413598596 +0200
>> @@ -4105,13 +4105,15 @@ class pass_stv : public rtl_opt_pass
>>  {
>>  public:
>>    pass_stv (gcc::context *ctxt)
>> -    : rtl_opt_pass (pass_data_stv, ctxt)
>> +    : rtl_opt_pass (pass_data_stv, ctxt),
>> +      timode_p (false)
>>    {}
>>
>>    /* opt_pass methods: */
>>    virtual bool gate (function *)
>>      {
>> -      return TARGET_STV && TARGET_SSE2 && optimize > 1;
>> +      return (timode_p ^ (TARGET_64BIT == 0))
>> +          && TARGET_STV && TARGET_SSE2 && optimize > 1;
>>      }
>>
>>    virtual unsigned int execute (function *)
>> @@ -4119,6 +4121,19 @@ public:
>>        return convert_scalars_to_vector ();
>>      }
>>
>> +  opt_pass *clone ()
>> +    {
>> +      return new pass_stv (m_ctxt);
>> +    }
>> +
>> +  void set_pass_param (unsigned int n, bool param)
>> +    {
>> +      gcc_assert (n == 0);
>> +      timode_p = param;
>> +    }
>> +
>> +private:
>> +  bool timode_p;
>>  }; // class pass_stv
>>
>>  } // anon namespace
>> @@ -6155,29 +6170,7 @@ ix86_option_override_internal (bool main
>>  static void
>>  ix86_option_override (void)
>>  {
>> -  opt_pass *pass_insert_vzeroupper = make_pass_insert_vzeroupper (g);
>> -  struct register_pass_info insert_vzeroupper_info
>> -    = { pass_insert_vzeroupper, "reload",
>> -     1, PASS_POS_INSERT_AFTER
>> -      };
>> -  opt_pass *pass_stv = make_pass_stv (g);
>> -  struct register_pass_info stv_info_dimode
>> -    = { pass_stv, "combine",
>> -     1, PASS_POS_INSERT_AFTER
>> -      };
>> -  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
>> -     CONSTM1_RTX generated by the STV pass can be CSEed.  */
>> -  struct register_pass_info stv_info_timode
>> -    = { pass_stv, "cse2",
>> -     1, PASS_POS_INSERT_BEFORE
>> -      };
>> -
>>    ix86_option_override_internal (true, &global_options, &global_options_set);
>> -
>> -
>> -  /* This needs to be done at start up.  It's convenient to do it here.  */
>> -  register_pass (&insert_vzeroupper_info);
>> -  register_pass (TARGET_64BIT ? &stv_info_timode : &stv_info_dimode);
>>  }
>>
>>  /* Implement the TARGET_OFFLOAD_OPTIONS hook.  */
>>
>>
>>       Jakub
>>
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-09-30 21:12 [PATCH] Improve target pass registration Jakub Jelinek
  2016-10-04  8:54 ` Richard Biener
@ 2016-10-04 14:49 ` Alexander Monakov
  2016-10-04 14:54   ` Jakub Jelinek
  1 sibling, 1 reply; 9+ messages in thread
From: Alexander Monakov @ 2016-10-04 14:49 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, Uros Bizjak, gcc-patches

On Fri, 30 Sep 2016, Jakub Jelinek wrote:
> This patch allows backends to provide their *-passes.def file with
> instructions how to ammend passes.def, which then can be inspected in
> pass-instances.def the script generates.

A few minor comments:

> --- gcc/gen-pass-instances.awk.jj	2016-09-29 22:53:10.264776158 +0200
> +++ gcc/gen-pass-instances.awk	2016-09-30 13:22:53.745373889 +0200
> @@ -30,81 +32,201 @@
>  # through:
>  #   NEXT_PASS (pass_copy_prop, 8);
>  # (currently there are 8 instances of that pass)
> +#
> +#     INSERT_PASS_AFTER (pass_copy_prop, 1, pass_stv);

I think it would be nice to mention that a 4th argument can be supplied and if
so, it will be passed to the pass at run time (although I see the existing
comment doesn't mention that either).

> +# will insert
> +#     NEXT_PASS (pass_stv, 1);
> +# after immediately after the NEXT_PASS (pass_copy_prop, 1) line,

Typo in the comment: duplicated 'after'.

> --- gcc/config/i386/i386-passes.def.jj	2016-09-30 12:54:29.959793738 +0200
> +++ gcc/config/i386/i386-passes.def	2016-09-30 14:01:31.237200032 +0200
> @@ -0,0 +1,31 @@
[snip]
> +
> +/*
> + Macros that should be defined used in this file:
> +   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
> +   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
> +   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
> + */

REPLACE_PASS isn't actually used in this file.

> --- gcc/config/i386/i386-protos.h.jj	2016-06-24 12:59:29.000000000 +0200
> +++ gcc/config/i386/i386-protos.h	2016-09-30 14:00:54.759659671 +0200

> @@ -4105,13 +4105,15 @@ class pass_stv : public rtl_opt_pass
[snip]
>  
>    /* opt_pass methods: */
>    virtual bool gate (function *)
>      {
> -      return TARGET_STV && TARGET_SSE2 && optimize > 1;
> +      return (timode_p ^ (TARGET_64BIT == 0))
> +	     && TARGET_STV && TARGET_SSE2 && optimize > 1;

The first line could also be 'timode_p == !!TARGET_64BIT'.
Also I believe this needs parens around the whole expression.

Alexander

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-10-04 14:49 ` Alexander Monakov
@ 2016-10-04 14:54   ` Jakub Jelinek
  2016-10-05 15:31     ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-10-04 14:54 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: Richard Biener, Uros Bizjak, gcc-patches

On Tue, Oct 04, 2016 at 05:48:15PM +0300, Alexander Monakov wrote:
> On Fri, 30 Sep 2016, Jakub Jelinek wrote:
> > This patch allows backends to provide their *-passes.def file with
> > instructions how to ammend passes.def, which then can be inspected in
> > pass-instances.def the script generates.
> 
> A few minor comments:
> 
> > --- gcc/gen-pass-instances.awk.jj	2016-09-29 22:53:10.264776158 +0200
> > +++ gcc/gen-pass-instances.awk	2016-09-30 13:22:53.745373889 +0200
> > @@ -30,81 +32,201 @@
> >  # through:
> >  #   NEXT_PASS (pass_copy_prop, 8);
> >  # (currently there are 8 instances of that pass)
> > +#
> > +#     INSERT_PASS_AFTER (pass_copy_prop, 1, pass_stv);
> 
> I think it would be nice to mention that a 4th argument can be supplied and if
> so, it will be passed to the pass at run time (although I see the existing
> comment doesn't mention that either).

Yeah, I've followed the earlier comments.  The 4th argument is quite rare
thing.

> > +# will insert
> > +#     NEXT_PASS (pass_stv, 1);
> > +# after immediately after the NEXT_PASS (pass_copy_prop, 1) line,
> 
> Typo in the comment: duplicated 'after'.

Will fix.

> > --- gcc/config/i386/i386-passes.def.jj	2016-09-30 12:54:29.959793738 +0200
> > +++ gcc/config/i386/i386-passes.def	2016-09-30 14:01:31.237200032 +0200
> > @@ -0,0 +1,31 @@
> [snip]
> > +
> > +/*
> > + Macros that should be defined used in this file:
> > +   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
> > +   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
> > +   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
> > + */
> 
> REPLACE_PASS isn't actually used in this file.

I guess it should be
  Macros that can be used in this file:

> > --- gcc/config/i386/i386-protos.h.jj	2016-06-24 12:59:29.000000000 +0200
> > +++ gcc/config/i386/i386-protos.h	2016-09-30 14:00:54.759659671 +0200
> 
> > @@ -4105,13 +4105,15 @@ class pass_stv : public rtl_opt_pass
> [snip]
> >  
> >    /* opt_pass methods: */
> >    virtual bool gate (function *)
> >      {
> > -      return TARGET_STV && TARGET_SSE2 && optimize > 1;
> > +      return (timode_p ^ (TARGET_64BIT == 0))
> > +	     && TARGET_STV && TARGET_SSE2 && optimize > 1;
> 
> The first line could also be 'timode_p == !!TARGET_64BIT'.

Yeah, that is probably more readable.

> Also I believe this needs parens around the whole expression.

Ok, will do.

	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-10-04 14:54   ` Jakub Jelinek
@ 2016-10-05 15:31     ` Jakub Jelinek
  2016-10-09  8:58       ` Eric Botcazou
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-10-05 15:31 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: Richard Biener, Uros Bizjak, gcc-patches

On Tue, Oct 04, 2016 at 04:54:34PM +0200, Jakub Jelinek wrote:
> > Typo in the comment: duplicated 'after'.
> 
> Will fix.

...

Here is updated patch, bootstrapped/regtested on x86_64-linux and
i686-linux.  I'll still wait a few days before committing to see if somebody
likes to comment on the awk implementation of the script.

2016-10-05  Jakub Jelinek  <jakub@redhat.com>

	* gen-pass-instances.awk: Rewritten.
	* Makefile.in (pass-instances.def): Depend on $(PASSES_EXTRA), pass
	$(PASSES_EXTRA) after passes.def to the script.
	* config/i386/t-i386 (PASSES_EXTRA): Add i386-passes.def.
	* config/i386/i386-passes.def: New file.
	* config/i386/i386-protos.h (make_pass_insert_vzeroupper,
	make_pass_stv): Declare.
	* config/i386/i386.c (pass_stv::pass_stv): Initialize timode_p to
	false.
	(pass_stv::gate): Depending on timode_p member require TARGET_64BIT
	or !TARGET_64BIT.
	(pass_stv::clone, pass_stv::set_pass_param): New methods.
	(pass_stv::timode_p): New non-static data member.
	(ix86_option_override): Don't register passes here.

--- gcc/gen-pass-instances.awk.jj	2016-09-29 22:53:10.264776158 +0200
+++ gcc/gen-pass-instances.awk	2016-09-30 13:22:53.745373889 +0200
@@ -17,6 +17,8 @@
 # This Awk script takes passes.def and writes pass-instances.def,
 # counting the instances of each kind of pass, adding an instance number
 # to everywhere that NEXT_PASS is used.
+# Also handle INSERT_PASS_AFTER, INSERT_PASS_BEFORE and REPLACE_PASS
+# directives.
 #
 # For example, the single-instanced pass:
 #     NEXT_PASS (pass_warn_unused_result);
@@ -30,81 +32,201 @@
 # through:
 #   NEXT_PASS (pass_copy_prop, 8);
 # (currently there are 8 instances of that pass)
+#
+#     INSERT_PASS_AFTER (pass_copy_prop, 1, pass_stv);
+# will insert
+#     NEXT_PASS (pass_stv, 1);
+# immediately after the NEXT_PASS (pass_copy_prop, 1) line,
+# similarly INSERT_PASS_BEFORE inserts immediately before that line.
+#     REPLACE_PASS (pass_copy_prop, 1, pass_stv, true);
+# will replace NEXT_PASS (pass_copy_prop, 1) line with
+#     NEXT_PASS (pass_stv, 1, true);
+# line and renumber all higher pass_copy_prop instances if any.
 
 # Usage: awk -f gen-pass-instances.awk passes.def
 
 BEGIN {
-	print "/* This file is auto-generated by gen-pass-instances.awk";
-	print "   from passes.def.  */";
+  print "/* This file is auto-generated by gen-pass-instances.awk";
+  print "   from passes.def.  */";
+  lineno = 1;
 }
 
-function handle_line()
+function parse_line(line, fnname,	len_of_call, len_of_start,
+					len_of_open, len_of_close,
+					len_of_args, args_start_at,
+					args_str, len_of_prefix,
+					call_starts_at,
+					postfix_starts_at)
 {
-	line = $0;
+  # Find call expression.
+  call_starts_at = match(line, fnname " \\(.+\\)");
+  if (call_starts_at == 0)
+    return 0;
+
+  # Length of the call expression.
+  len_of_call = RLENGTH;
+
+  len_of_start = length(fnname " (");
+  len_of_open = length("(");
+  len_of_close = length(")");
+
+  # Find arguments
+  len_of_args = len_of_call - (len_of_start + len_of_close);
+  args_start_at = call_starts_at + len_of_start;
+  args_str = substr(line, args_start_at, len_of_args);
+  split(args_str, args, ",");
+
+  # Find call expression prefix
+  len_of_prefix = call_starts_at - 1;
+  prefix = substr(line, 1, len_of_prefix);
+
+  # Find call expression postfix
+  postfix_starts_at = call_starts_at + len_of_call;
+  postfix = substr(line, postfix_starts_at);
+  return 1;
+}
 
-	# Find call expression.
-	call_starts_at = match(line, /NEXT_PASS \(.+\)/);
-	if (call_starts_at == 0)
-	{
-		print line;
-		return;
-	}
+function adjust_linenos(above, increment,	p, i)
+{
+  for (p in pass_lines)
+    if (pass_lines[p] >= above)
+      pass_lines[p] += pass_lines[p];
+  if (increment > 0)
+    for (i = lineno - 1; i >= above; i--)
+      lines[i + increment] = lines[i];
+  else
+    for (i = above; i < lineno; i++)
+      lines[i + increment] = lines[i];
+  lineno += increment;
+}
 
-	# Length of the call expression.
-	len_of_call = RLENGTH;
+function insert_remove_pass(line, fnname)
+{
+  parse_line($0, fnname);
+  pass_name = args[1];
+  if (pass_name == "PASS")
+    return 1;
+  pass_num = args[2] + 0;
+  new_line = prefix "NEXT_PASS (" args[3];
+  if (args[4])
+    new_line = new_line ", " args[4];
+  new_line = new_line ")" postfix;
+  if (!pass_lines[pass_name, pass_num])
+    {
+      print "ERROR: Can't locate instance of the pass mentioned in " fnname;
+      return 1;
+    }
+  return 0;
+}
 
-	len_of_start = length("NEXT_PASS (");
-	len_of_open = length("(");
-	len_of_close = length(")");
-
-	# Find arguments
-	len_of_args = len_of_call - (len_of_start + len_of_close);
-	args_start_at = call_starts_at + len_of_start;
-	args_str = substr(line, args_start_at, len_of_args);
-	split(args_str, args, ",");
-
-	# Set pass_name argument, an optional with_arg argument
-	pass_name = args[1];
-	with_arg = args[2];
-
-	# Find call expression prefix
-	len_of_prefix = call_starts_at - 1;
-	prefix = substr(line, 1, len_of_prefix);
-
-	# Find call expression postfix
-	postfix_starts_at = call_starts_at + len_of_call;
-	postfix = substr(line, postfix_starts_at);
-
-	# Set pass_counts
-	if (pass_name in pass_counts)
-		pass_counts[pass_name]++;
-	else
-		pass_counts[pass_name] = 1;
-
-	pass_num = pass_counts[pass_name];
-
-	# Print call expression with extra pass_num argument
-	printf "%s", prefix;
-	if (with_arg)
-	{
-		printf "NEXT_PASS_WITH_ARG";
-	}
-	else
-	{
-		printf "NEXT_PASS";
-	}
-	printf " (";
-	printf "%s", pass_name;
-	printf ", %s", pass_num;
-	if (with_arg)
+function insert_pass(line, fnname, after,		num)
+{
+  if (insert_remove_pass(line, fnname))
+    return;
+  num = pass_lines[pass_name, pass_num];
+  adjust_linenos(num + after, 1);
+  pass_name = args[3];
+  # Set pass_counts
+  if (args[3] in pass_counts)
+    pass_counts[pass_name]++;
+  else
+    pass_counts[pass_name] = 1;
+
+  pass_lines[pass_name, pass_counts[pass_name]] = num + after;
+  lines[num + after] = new_line;
+}
+
+function replace_pass(line, fnname,			num, i)
+{
+  if (insert_remove_pass(line, "REPLACE_PASS"))
+    return;
+  num = pass_lines[pass_name, pass_num];
+  for (i = pass_counts[pass_name]; i > pass_num; i--)
+    pass_lines[pass_name, i - 1] = pass_lines[pass_name, i];
+  delete pass_lines[pass_name, pass_counts[pass_name]];
+  if (pass_counts[pass_name] == 1)
+    delete pass_counts[pass_name];
+  else
+    pass_counts[pass_name]--;
+
+  pass_name = args[3];
+  # Set pass_counts
+  if (args[3] in pass_counts)
+    pass_counts[pass_name]++;
+  else
+    pass_counts[pass_name] = 1;
+
+  pass_lines[pass_name, pass_counts[pass_name]] = num;
+  lines[num] = new_line;
+}
+
+/INSERT_PASS_AFTER \(.+\)/ {
+  insert_pass($0, "INSERT_PASS_AFTER", 1);
+  next;
+}
+
+/INSERT_PASS_BEFORE \(.+\)/ {
+  insert_pass($0, "INSERT_PASS_BEFORE", 0);
+  next;
+}
+
+/REPLACE_PASS \(.+\)/ {
+  replace_pass($0, "REPLACE_PASS");
+  next;
+}
+
+{
+  ret = parse_line($0, "NEXT_PASS");
+  if (ret)
+    {
+      pass_name = args[1];
+
+      # Set pass_counts
+      if (pass_name in pass_counts)
+	pass_counts[pass_name]++;
+      else
+	pass_counts[pass_name] = 1;
+
+      pass_lines[pass_name, pass_counts[pass_name]] = lineno;
+    }
+  lines[lineno++] = $0;
+}
+
+END {
+  delete pass_counts;
+  for (i = 1; i < lineno; i++)
+    {
+      ret = parse_line(lines[i], "NEXT_PASS");
+      if (ret)
 	{
-		printf ", %s", with_arg;
+	  # Set pass_name argument, an optional with_arg argument
+	  pass_name = args[1];
+	  with_arg = args[2];
+
+	  # Set pass_counts
+	  if (pass_name in pass_counts)
+	    pass_counts[pass_name]++;
+	  else
+	    pass_counts[pass_name] = 1;
+
+	  pass_num = pass_counts[pass_name];
+
+	  # Print call expression with extra pass_num argument
+	  printf "%s", prefix;
+	  if (with_arg)
+	    printf "NEXT_PASS_WITH_ARG";
+	  else
+	    printf "NEXT_PASS";
+	  printf " (%s, %s", pass_name, pass_num;
+	  if (with_arg)
+	    printf ", %s", with_arg;
+	  printf ")%s\n", postfix;
 	}
-	printf ")%s\n", postfix;
+      else
+	print lines[i];
+    }
 }
 
-{ handle_line() }
-
 # Local Variables:
 # mode:awk
 # c-basic-offset:8
--- gcc/Makefile.in.jj	2016-09-29 22:53:15.000000000 +0200
+++ gcc/Makefile.in	2016-09-30 13:25:05.100723726 +0200
@@ -2158,9 +2158,10 @@ s-bversion: BASE-VER
 
 CFLAGS-toplev.o += -DTARGET_NAME=\"$(target_noncanonical)\"
 
-pass-instances.def: $(srcdir)/passes.def $(srcdir)/gen-pass-instances.awk
+pass-instances.def: $(srcdir)/passes.def $(PASSES_EXTRA) \
+		    $(srcdir)/gen-pass-instances.awk
 	$(AWK) -f $(srcdir)/gen-pass-instances.awk \
-	  $(srcdir)/passes.def > pass-instances.def
+	  $(srcdir)/passes.def $(PASSES_EXTRA) > pass-instances.def
 
 $(out_object_file): $(out_file)
 	$(COMPILE) $<
--- gcc/config/i386/t-i386.jj	2016-08-19 17:23:17.000000000 +0200
+++ gcc/config/i386/t-i386	2016-09-30 13:20:45.748981856 +0200
@@ -18,6 +18,7 @@
 
 OPTIONS_H_EXTRA += $(srcdir)/config/i386/stringop.def
 TM_H += $(srcdir)/config/i386/x86-tune.def
+PASSES_EXTRA += $(srcdir)/config/i386/i386-passes.def
 
 i386-c.o: $(srcdir)/config/i386/i386-c.c
 	  $(COMPILE) $<
--- gcc/config/i386/i386-passes.def.jj	2016-09-30 12:54:29.959793738 +0200
+++ gcc/config/i386/i386-passes.def	2016-09-30 14:01:31.237200032 +0200
@@ -0,0 +1,31 @@
+/* Description of target passes for IA-32 
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/*
+   Macros that can be used in this file:
+   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
+   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
+   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
+ */
+
+  INSERT_PASS_AFTER (pass_reload, 1, pass_insert_vzeroupper);
+  INSERT_PASS_AFTER (pass_combine, 1, pass_stv, false /* timode_p */);
+  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
+     CONSTM1_RTX generated by the STV pass can be CSEed.  */
+  INSERT_PASS_BEFORE (pass_cse2, 1, pass_stv, true /* timode_p */);
--- gcc/config/i386/i386-protos.h.jj	2016-06-24 12:59:29.000000000 +0200
+++ gcc/config/i386/i386-protos.h	2016-09-30 14:00:54.759659671 +0200
@@ -338,3 +338,9 @@ struct ix86_first_cycle_multipass_data_
 
 const addr_space_t ADDR_SPACE_SEG_FS = 1;
 const addr_space_t ADDR_SPACE_SEG_GS = 2;
+
+namespace gcc { class context; }
+class rtl_opt_pass;
+
+extern rtl_opt_pass *make_pass_insert_vzeroupper (gcc::context *);
+extern rtl_opt_pass *make_pass_stv (gcc::context *);
--- gcc/config/i386/i386.c.jj	2016-09-27 10:14:35.000000000 +0200
+++ gcc/config/i386/i386.c	2016-09-30 14:07:36.413598596 +0200
@@ -4105,13 +4105,15 @@ class pass_stv : public rtl_opt_pass
 {
 public:
   pass_stv (gcc::context *ctxt)
-    : rtl_opt_pass (pass_data_stv, ctxt)
+    : rtl_opt_pass (pass_data_stv, ctxt),
+      timode_p (false)
   {}
 
   /* opt_pass methods: */
   virtual bool gate (function *)
     {
-      return TARGET_STV && TARGET_SSE2 && optimize > 1;
+      return (timode_p == !!TARGET_64BIT
+	      && TARGET_STV && TARGET_SSE2 && optimize > 1);
     }
 
   virtual unsigned int execute (function *)
@@ -4119,6 +4121,19 @@ public:
       return convert_scalars_to_vector ();
     }
 
+  opt_pass *clone ()
+    {
+      return new pass_stv (m_ctxt);
+    }
+
+  void set_pass_param (unsigned int n, bool param)
+    {
+      gcc_assert (n == 0);
+      timode_p = param;
+    }
+
+private:
+  bool timode_p;
 }; // class pass_stv
 
 } // anon namespace
@@ -6155,29 +6170,7 @@ ix86_option_override_internal (bool main
 static void
 ix86_option_override (void)
 {
-  opt_pass *pass_insert_vzeroupper = make_pass_insert_vzeroupper (g);
-  struct register_pass_info insert_vzeroupper_info
-    = { pass_insert_vzeroupper, "reload",
-	1, PASS_POS_INSERT_AFTER
-      };
-  opt_pass *pass_stv = make_pass_stv (g);
-  struct register_pass_info stv_info_dimode
-    = { pass_stv, "combine",
-	1, PASS_POS_INSERT_AFTER
-      };
-  /* Run the 64-bit STV pass before the CSE pass so that CONST0_RTX and
-     CONSTM1_RTX generated by the STV pass can be CSEed.  */
-  struct register_pass_info stv_info_timode
-    = { pass_stv, "cse2",
-	1, PASS_POS_INSERT_BEFORE
-      };
-
   ix86_option_override_internal (true, &global_options, &global_options_set);
-
-
-  /* This needs to be done at start up.  It's convenient to do it here.  */
-  register_pass (&insert_vzeroupper_info);
-  register_pass (TARGET_64BIT ? &stv_info_timode : &stv_info_dimode);
 }
 
 /* Implement the TARGET_OFFLOAD_OPTIONS hook.  */


	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-10-05 15:31     ` Jakub Jelinek
@ 2016-10-09  8:58       ` Eric Botcazou
  2016-10-09 20:51         ` Eric Botcazou
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Botcazou @ 2016-10-09  8:58 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Alexander Monakov, Richard Biener, Uros Bizjak

> Here is updated patch, bootstrapped/regtested on x86_64-linux and
> i686-linux.  I'll still wait a few days before committing to see if somebody
> likes to comment on the awk implementation of the script.
> 
> 2016-10-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* gen-pass-instances.awk: Rewritten.

This breaks bootstrap on Solaris with nawk:

nawk -f /homes/botcazou/gcc-head/src/gcc/gen-pass-instances.awk \
          /homes/botcazou/gcc-head/src/gcc/passes.def  > pass-instances.def
nawk: you can only delete array[element] at source line 196
 context is
          delete >>>  pass_counts; <<< 
nawk: syntax error at source line 196
nawk: illegal statement at source line 196
make[3]: *** [pass-instances.def] Error 2

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-10-09  8:58       ` Eric Botcazou
@ 2016-10-09 20:51         ` Eric Botcazou
  2016-10-09 21:26           ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Botcazou @ 2016-10-09 20:51 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Alexander Monakov, Richard Biener, Uros Bizjak

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

> This breaks bootstrap on Solaris with nawk:
> 
> nawk -f /homes/botcazou/gcc-head/src/gcc/gen-pass-instances.awk \
>           /homes/botcazou/gcc-head/src/gcc/passes.def  > pass-instances.def
> nawk: you can only delete array[element] at source line 196
>  context is
>           delete >>>  pass_counts; <<<
> nawk: syntax error at source line 196
> nawk: illegal statement at source line 196
> make[3]: *** [pass-instances.def] Error 2

Here's what I have installed as obvious after testing on Linux and Solaris.


2016-10-09  Eric Botcazou  <ebotcazou@adacore.com>

	* gen-pass-instances.awk: Remove GNUism.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 894 bytes --]

Index: gen-pass-instances.awk
===================================================================
--- gen-pass-instances.awk	(revision 240888)
+++ gen-pass-instances.awk	(working copy)
@@ -193,7 +193,6 @@ function replace_pass(line, fnname,			nu
 }
 
 END {
-  delete pass_counts;
   for (i = 1; i < lineno; i++)
     {
       ret = parse_line(lines[i], "NEXT_PASS");
@@ -203,13 +202,13 @@ END {
 	  pass_name = args[1];
 	  with_arg = args[2];
 
-	  # Set pass_counts
-	  if (pass_name in pass_counts)
-	    pass_counts[pass_name]++;
+	  # Set pass_final_counts
+	  if (pass_name in pass_final_counts)
+	    pass_final_counts[pass_name]++;
 	  else
-	    pass_counts[pass_name] = 1;
+	    pass_final_counts[pass_name] = 1;
 
-	  pass_num = pass_counts[pass_name];
+	  pass_num = pass_final_counts[pass_name];
 
 	  # Print call expression with extra pass_num argument
 	  printf "%s", prefix;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Improve target pass registration
  2016-10-09 20:51         ` Eric Botcazou
@ 2016-10-09 21:26           ` Jakub Jelinek
  0 siblings, 0 replies; 9+ messages in thread
From: Jakub Jelinek @ 2016-10-09 21:26 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Alexander Monakov, Richard Biener, Uros Bizjak

On Sun, Oct 09, 2016 at 10:51:10PM +0200, Eric Botcazou wrote:
> > This breaks bootstrap on Solaris with nawk:
> > 
> > nawk -f /homes/botcazou/gcc-head/src/gcc/gen-pass-instances.awk \
> >           /homes/botcazou/gcc-head/src/gcc/passes.def  > pass-instances.def
> > nawk: you can only delete array[element] at source line 196
> >  context is
> >           delete >>>  pass_counts; <<<
> > nawk: syntax error at source line 196
> > nawk: illegal statement at source line 196
> > make[3]: *** [pass-instances.def] Error 2
> 
> Here's what I have installed as obvious after testing on Linux and Solaris.
> 
> 
> 2016-10-09  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* gen-pass-instances.awk: Remove GNUism.

Thanks.

	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-10-09 21:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-30 21:12 [PATCH] Improve target pass registration Jakub Jelinek
2016-10-04  8:54 ` Richard Biener
2016-10-04  9:23   ` Uros Bizjak
2016-10-04 14:49 ` Alexander Monakov
2016-10-04 14:54   ` Jakub Jelinek
2016-10-05 15:31     ` Jakub Jelinek
2016-10-09  8:58       ` Eric Botcazou
2016-10-09 20:51         ` Eric Botcazou
2016-10-09 21:26           ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).