Hi!
As part of this:
On 2013-07-26T11:04:33-0400, David Malcolm <dmalc...@redhat.com> wrote:
> This patch is the hand-written part of the conversion of passes from
> C structs to C++ classes.
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
..., we did hard-code 'PUSH_INSERT_PASSES_WITHIN(PASS)' to always refer
to the first instance of 'PASS':
> #define PUSH_INSERT_PASSES_WITHIN(PASS) \
> { \
> - struct opt_pass **p = &(PASS).pass.sub;
> + struct opt_pass **p = &(PASS ## _1)->sub;
..., however we did change 'NEXT_PASS(PASS, NUM)' to actually use 'NUM':
> -#define NEXT_PASS(PASS, NUM) (p = next_pass_1 (p, &((PASS).pass)))
> +#define NEXT_PASS(PASS, NUM) \
> + do { \
> + gcc_assert (NULL == PASS ## _ ## NUM); \
> + if ((NUM) == 1) \
> + PASS ## _1 = make_##PASS (ctxt_); \
> + else \
> + { \
> + gcc_assert (PASS ## _1); \
> + PASS ## _ ## NUM = PASS ## _1->clone (); \
> + } \
> + p = next_pass_1 (p, PASS ## _ ## NUM); \
> + } while (0)
This was never re-synchronized later on, and is problematic if you try to
do something like this; change:
[...]
NEXT_PASS (pass_postreload);
PUSH_INSERT_PASSES_WITHIN (pass_postreload)
NEXT_PASS (pass_postreload_cse);
[...]
NEXT_PASS (pass_cprop_hardreg);
NEXT_PASS (pass_fast_rtl_dce);
NEXT_PASS (pass_reorder_blocks);
[...]
POP_INSERT_PASSES ()
[...]
... into:
[...]
NEXT_PASS (pass_postreload);
PUSH_INSERT_PASSES_WITHIN (pass_postreload)
NEXT_PASS (pass_postreload_cse);
[...]
NEXT_PASS (pass_cprop_hardreg);
POP_INSERT_PASSES ()
NEXT_PASS (pass_fast_rtl_dce);
NEXT_PASS (pass_postreload);
PUSH_INSERT_PASSES_WITHIN (pass_postreload)
NEXT_PASS (pass_reorder_blocks);
[...]
POP_INSERT_PASSES ()
[...]
That is, interrupt the pass pipeline within 'pass_postreload', in order
to unconditionally run 'pass_fast_rtl_dce' even if not running
'pass_postreload'. What happens is that the second
'PUSH_INSERT_PASSES_WITHIN (pass_postreload)' overwrites the first
'PUSH_INSERT_PASSES_WITHIN (pass_postreload)' instead of applying to the
second (preceding) 'NEXT_PASS (pass_postreload);'.
(I ran into this in context of what I tried in
<https://inbox.sourceware.org/87ed8i2ekt....@euler.schwinge.ddns.net>
"nvptx vs. [PATCH] Add a late-combine pass [PR106594]"; discuss that
specific use case over there, not here.)
OK to address this with the attached
"Handle 'NUM' in 'PUSH_INSERT_PASSES_WITHIN'"?
This depends on
<https://inbox.sourceware.org/87jzi9tgcw....@euler.schwinge.ddns.net>
"Rewrite usage comment at the top of 'gcc/passes.def'" to avoid running
into the 'ERROR: Can't locate [...]' that I'm adding, while processing
the 'PUSH_INSERT_PASSES_WITHIN (PASS)' in the usage comment at the top of
'gcc/passes.def', where 'NEXT_PASS (PASS)' only appears later. ;-)
I've verified this does the expected thing for the main 'gcc/passes.def',
and that 'PUSH_INSERT_PASSES_WITHIN' is not used/not applicable for
'PASSES_EXTRA' ('gcc/config/*/*-passes.def').
Grüße
Thomas
>From e368ccba93f5bbaee882076c80849adb55a68fa0 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwi...@baylibre.com>
Date: Fri, 28 Jun 2024 12:10:12 +0200
Subject: [PATCH] Handle 'NUM' in 'PUSH_INSERT_PASSES_WITHIN'
..., such that also for repeated 'NEXT_PASS', 'PUSH_INSERT_PASSES_WITHIN' for a
given 'PASS', the 'PUSH_INSERT_PASSES_WITHIN' applies to the preceeding
'NEXT_PASS', and not unconditionally applies to the first 'NEXT_PASS'.
gcc/
* gen-pass-instances.awk: Handle 'PUSH_INSERT_PASSES_WITHIN'.
* pass_manager.h (PUSH_INSERT_PASSES_WITHIN): Adjust.
* passes.cc (PUSH_INSERT_PASSES_WITHIN): Likewise.
---
gcc/gen-pass-instances.awk | 28 +++++++++++++++++++++++++---
gcc/pass_manager.h | 2 +-
gcc/passes.cc | 6 +++---
3 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/gcc/gen-pass-instances.awk b/gcc/gen-pass-instances.awk
index 449889663f7..871ac0cdb52 100644
--- a/gcc/gen-pass-instances.awk
+++ b/gcc/gen-pass-instances.awk
@@ -16,7 +16,7 @@
# 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.
+# to everywhere that NEXT_PASS or PUSH_INSERT_PASSES_WITHIN are used.
# Also handle INSERT_PASS_AFTER, INSERT_PASS_BEFORE and REPLACE_PASS
# directives.
#
@@ -222,9 +222,31 @@ END {
if (with_arg)
printf ",%s", with_arg;
printf ")%s\n", postfix;
+
+ continue;
}
- else
- print lines[i];
+
+ ret = parse_line(lines[i], "PUSH_INSERT_PASSES_WITHIN");
+ if (ret)
+ {
+ pass_name = args[1];
+
+ pass_num = pass_final_counts[pass_name];
+ if (!pass_num)
+ {
+ print "ERROR: Can't locate instance of the pass mentioned in " pass_name;
+ exit 1;
+ }
+
+ printf "%s", prefix;
+ printf "PUSH_INSERT_PASSES_WITHIN";
+ printf " (%s, %s", pass_name, pass_num;
+ printf ")%s\n", postfix;
+
+ continue;
+ }
+
+ print lines[i];
}
}
diff --git a/gcc/pass_manager.h b/gcc/pass_manager.h
index be324d5dff7..edd775e9a9c 100644
--- a/gcc/pass_manager.h
+++ b/gcc/pass_manager.h
@@ -126,7 +126,7 @@ private:
opt_pass *pass_copy_prop_8; */
#define INSERT_PASSES_AFTER(PASS)
-#define PUSH_INSERT_PASSES_WITHIN(PASS)
+#define PUSH_INSERT_PASSES_WITHIN(PASS, NUM)
#define POP_INSERT_PASSES()
#define NEXT_PASS(PASS, NUM) opt_pass *PASS ## _ ## NUM
#define NEXT_PASS_WITH_ARG(PASS, NUM, ARG) NEXT_PASS (PASS, NUM)
diff --git a/gcc/passes.cc b/gcc/passes.cc
index b01a79ef96c..023631046a6 100644
--- a/gcc/passes.cc
+++ b/gcc/passes.cc
@@ -1574,7 +1574,7 @@ pass_manager::pass_manager (context *ctxt)
/* Zero-initialize pass members. */
#define INSERT_PASSES_AFTER(PASS)
-#define PUSH_INSERT_PASSES_WITHIN(PASS)
+#define PUSH_INSERT_PASSES_WITHIN(PASS, NUM)
#define POP_INSERT_PASSES()
#define NEXT_PASS(PASS, NUM) PASS ## _ ## NUM = NULL
#define NEXT_PASS_WITH_ARG(PASS, NUM, ARG) NEXT_PASS (PASS, NUM)
@@ -1604,9 +1604,9 @@ pass_manager::pass_manager (context *ctxt)
*p = NULL; \
}
-#define PUSH_INSERT_PASSES_WITHIN(PASS) \
+#define PUSH_INSERT_PASSES_WITHIN(PASS, NUM) \
{ \
- opt_pass **p = &(PASS ## _1)->sub;
+ opt_pass **p = &(PASS ## _ ## NUM)->sub;
#define POP_INSERT_PASSES() \
}
--
2.34.1
