This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Improve target pass registration
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Uros Bizjak <ubizjak at gmail dot com>, gcc-patches at gcc dot gnu dot org
- Date: Tue, 4 Oct 2016 10:53:53 +0200 (CEST)
- Subject: Re: [PATCH] Improve target pass registration
- Authentication-results: sourceware.org; auth=none
- References: <20160930201105.GH7282@tucnak.redhat.com>
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)