This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Improve target pass registration


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)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]