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  <ja...@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 <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Reply via email to