On Tue, Oct 4, 2016 at 10:53 AM, Richard Biener <rguent...@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 <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)
