* [patch,committed] Work around problem in gen-pass-instances.awk
@ 2016-10-26 9:54 Georg-Johann Lay
2016-10-26 10:48 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Georg-Johann Lay @ 2016-10-26 9:54 UTC (permalink / raw)
To: gcc-patches
gen-pass-instances.awk is sensitive to the order in which directives are
written down, e.g. in target-pass.def: If a pass that runs first is added
first, then the last pass is skipped and not added to pass-instances.def.
Work around is to add the 2nd pass before adding the 1st pass...
http://gcc.gnu.org/r241547
No so obvious, but committed anyway...
Johann
gen-pass-instances.awk is sensitive to the order in which
passes are added; passes that appear later have to be added first.
PR target/71676
PR target/71678
* config/avr/avr-passes.def: Swap order of directives for
gen-pass-instances.awk.
Index: config/avr/avr-passes.def
===================================================================
--- config/avr/avr-passes.def (revision 241546)
+++ config/avr/avr-passes.def (revision 241547)
@@ -17,6 +17,19 @@
along with GCC; see the file COPYING3. If not see
<http://www.gnu.org/licenses/>. */
+/* FIXME: We have to add the last pass first, otherwise
+ gen-pass-instances.awk won't work as expected. */
+
+/* This avr-specific pass (re)computes insn notes, in particular REG_DEAD
+ notes which are used by `avr.c::reg_unused_after' and branch offset
+ computations. These notes must be correct, i.e. there must be no
+ dangling REG_DEAD notes; otherwise wrong code might result, cf. PR64331.
+
+ DF needs (correct) CFG, hence right before free_cfg is the last
+ opportunity to rectify notes. */
+
+INSERT_PASS_BEFORE (pass_free_cfg, 1, avr_pass_recompute_notes);
+
/* casesi uses a SImode switch index which is quite costly as most code will
work on HImode or QImode. The following pass runs right after .expand and
tries to fix such situations by operating on the original mode. This
@@ -27,13 +40,3 @@
insns withaout any insns in between. */
INSERT_PASS_AFTER (pass_expand, 1, avr_pass_casesi);
-
-/* This avr-specific pass (re)computes insn notes, in particular REG_DEAD
- notes which are used by `avr.c::reg_unused_after' and branch offset
- computations. These notes must be correct, i.e. there must be no
- dangling REG_DEAD notes; otherwise wrong code might result, cf. PR64331.
-
- DF needs (correct) CFG, hence right before free_cfg is the last
- opportunity to rectify notes. */
-
-INSERT_PASS_BEFORE (pass_free_cfg, 1, avr_pass_recompute_notes);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch,committed] Work around problem in gen-pass-instances.awk
2016-10-26 9:54 [patch,committed] Work around problem in gen-pass-instances.awk Georg-Johann Lay
@ 2016-10-26 10:48 ` Jakub Jelinek
2016-10-26 11:08 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2016-10-26 10:48 UTC (permalink / raw)
To: Richard Biener, Georg-Johann Lay; +Cc: gcc-patches
On Wed, Oct 26, 2016 at 11:54:36AM +0200, Georg-Johann Lay wrote:
> gen-pass-instances.awk is sensitive to the order in which directives are
> written down, e.g. in target-pass.def: If a pass that runs first is added
> first, then the last pass is skipped and not added to pass-instances.def.
>
> Work around is to add the 2nd pass before adding the 1st pass...
>
> http://gcc.gnu.org/r241547
>
> No so obvious, but committed anyway...
We shouldn't be working around bugs, but fixing them.
Here is a fix (so far tested only with running
for i in alpha avr i386 sparc aarch64; do
awk -f ./gen-pass-instances.awk.jj passes.def config/$i/${i}-passes.def > /tmp/1
awk -f ./gen-pass-instances.awk passes.def config/$i/${i}-passes.def > /tmp/2
diff -up /tmp/1 /tmp/2 # and diff -upb
done
), for avr for both avr-passes.def before your workaround and after it.
Except for the desirable whitespace changes, the only change is in the
pre-workaround avr-passes.def having the same output as after-workaround
(except for the comments at the end of file).
The fix for the avr issue is just the first hunk, the first part of the
second hunk attempts to deal with
NEXT_PASS ( avr_pass_casesi, 1);
instead of the desirable
NEXT_PASS (avr_pass_casesi, 1);
(all arguments have the whitespace they have in *.def before them (after ,
or ( characters). So the first part of the second hunk strips away
whitespace, so that one doesn't need to type
INSERT_PASS_BEFORE (pass_free_cfg, 1,avr_pass_casesi);
and the second part of the second hunk and third hunk deal with similar
issue for the optional argument, we used to emit
NEXT_PASS_WITH_ARG (pass_dominator, 1, false /* may_peel_loop_headers_p */);
rather than
NEXT_PASS_WITH_ARG (pass_dominator, 1, false /* may_peel_loop_headers_p */);
because the space from
NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */);
(in between , and false) is already there and we add another one.
Ok for trunk if it passes bootstrap/regtest (though, as it doesn't change
anything but whitespace on x86_64, it shouldn't make a difference)?
2016-10-26 Jakub Jelinek <jakub@redhat.com>
* gen-pass-instances.awk (adjust_linenos): INcrement pass_lines[p]
by increment rather than double it.
(insert_remove_pass): Strip leading whitespace from args[3]. Don't
emit a space before args[4].
(END): Don't emit a space before with_arg.
--- gcc/gen-pass-instances.awk.jj 2016-10-10 10:41:32.000000000 +0200
+++ gcc/gen-pass-instances.awk 2016-10-26 12:30:20.637310242 +0200
@@ -90,7 +90,7 @@ function adjust_linenos(above, increment
{
for (p in pass_lines)
if (pass_lines[p] >= above)
- pass_lines[p] += pass_lines[p];
+ pass_lines[p] += increment;
if (increment > 0)
for (i = lineno - 1; i >= above; i--)
lines[i + increment] = lines[i];
@@ -100,16 +100,18 @@ function adjust_linenos(above, increment
lineno += increment;
}
-function insert_remove_pass(line, fnname)
+function insert_remove_pass(line, fnname, arg3)
{
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];
+ arg3 = args[3];
+ sub(/^[ \t]*/, "", arg3);
+ new_line = prefix "NEXT_PASS (" arg3;
if (args[4])
- new_line = new_line ", " args[4];
+ new_line = new_line "," args[4];
new_line = new_line ")" postfix;
if (!pass_lines[pass_name, pass_num])
{
@@ -218,7 +220,7 @@ END {
printf "NEXT_PASS";
printf " (%s, %s", pass_name, pass_num;
if (with_arg)
- printf ", %s", with_arg;
+ printf ",%s", with_arg;
printf ")%s\n", postfix;
}
else
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch,committed] Work around problem in gen-pass-instances.awk
2016-10-26 10:48 ` Jakub Jelinek
@ 2016-10-26 11:08 ` Richard Biener
0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2016-10-26 11:08 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Georg-Johann Lay, gcc-patches
On Wed, 26 Oct 2016, Jakub Jelinek wrote:
> On Wed, Oct 26, 2016 at 11:54:36AM +0200, Georg-Johann Lay wrote:
> > gen-pass-instances.awk is sensitive to the order in which directives are
> > written down, e.g. in target-pass.def: If a pass that runs first is added
> > first, then the last pass is skipped and not added to pass-instances.def.
> >
> > Work around is to add the 2nd pass before adding the 1st pass...
> >
> > http://gcc.gnu.org/r241547
> >
> > No so obvious, but committed anyway...
>
> We shouldn't be working around bugs, but fixing them.
>
> Here is a fix (so far tested only with running
> for i in alpha avr i386 sparc aarch64; do
> awk -f ./gen-pass-instances.awk.jj passes.def config/$i/${i}-passes.def > /tmp/1
> awk -f ./gen-pass-instances.awk passes.def config/$i/${i}-passes.def > /tmp/2
> diff -up /tmp/1 /tmp/2 # and diff -upb
> done
> ), for avr for both avr-passes.def before your workaround and after it.
> Except for the desirable whitespace changes, the only change is in the
> pre-workaround avr-passes.def having the same output as after-workaround
> (except for the comments at the end of file).
>
> The fix for the avr issue is just the first hunk, the first part of the
> second hunk attempts to deal with
> NEXT_PASS ( avr_pass_casesi, 1);
> instead of the desirable
> NEXT_PASS (avr_pass_casesi, 1);
> (all arguments have the whitespace they have in *.def before them (after ,
> or ( characters). So the first part of the second hunk strips away
> whitespace, so that one doesn't need to type
> INSERT_PASS_BEFORE (pass_free_cfg, 1,avr_pass_casesi);
> and the second part of the second hunk and third hunk deal with similar
> issue for the optional argument, we used to emit
> NEXT_PASS_WITH_ARG (pass_dominator, 1, false /* may_peel_loop_headers_p */);
> rather than
> NEXT_PASS_WITH_ARG (pass_dominator, 1, false /* may_peel_loop_headers_p */);
> because the space from
> NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */);
> (in between , and false) is already there and we add another one.
>
> Ok for trunk if it passes bootstrap/regtest (though, as it doesn't change
> anything but whitespace on x86_64, it shouldn't make a difference)?
Ok.
Richard.
> 2016-10-26 Jakub Jelinek <jakub@redhat.com>
>
> * gen-pass-instances.awk (adjust_linenos): INcrement pass_lines[p]
> by increment rather than double it.
> (insert_remove_pass): Strip leading whitespace from args[3]. Don't
> emit a space before args[4].
> (END): Don't emit a space before with_arg.
>
> --- gcc/gen-pass-instances.awk.jj 2016-10-10 10:41:32.000000000 +0200
> +++ gcc/gen-pass-instances.awk 2016-10-26 12:30:20.637310242 +0200
> @@ -90,7 +90,7 @@ function adjust_linenos(above, increment
> {
> for (p in pass_lines)
> if (pass_lines[p] >= above)
> - pass_lines[p] += pass_lines[p];
> + pass_lines[p] += increment;
> if (increment > 0)
> for (i = lineno - 1; i >= above; i--)
> lines[i + increment] = lines[i];
> @@ -100,16 +100,18 @@ function adjust_linenos(above, increment
> lineno += increment;
> }
>
> -function insert_remove_pass(line, fnname)
> +function insert_remove_pass(line, fnname, arg3)
> {
> 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];
> + arg3 = args[3];
> + sub(/^[ \t]*/, "", arg3);
> + new_line = prefix "NEXT_PASS (" arg3;
> if (args[4])
> - new_line = new_line ", " args[4];
> + new_line = new_line "," args[4];
> new_line = new_line ")" postfix;
> if (!pass_lines[pass_name, pass_num])
> {
> @@ -218,7 +220,7 @@ END {
> printf "NEXT_PASS";
> printf " (%s, %s", pass_name, pass_num;
> if (with_arg)
> - printf ", %s", with_arg;
> + printf ",%s", with_arg;
> printf ")%s\n", postfix;
> }
> else
>
>
> 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] 3+ messages in thread
end of thread, other threads:[~2016-10-26 11:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-26 9:54 [patch,committed] Work around problem in gen-pass-instances.awk Georg-Johann Lay
2016-10-26 10:48 ` Jakub Jelinek
2016-10-26 11:08 ` Richard Biener
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).