This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 9e] Update "startwith" logic for pass-skipping to handle __RTL functions
On 01/09/2017 07:38 PM, David Malcolm wrote:
gcc/ChangeLog:
* passes.c: Include "insn-addr.h".
(should_skip_pass_p): Add logging. Update logic for running
"expand" to be compatible with both __GIMPLE and __RTL. Guard
property-provider override so it is only done for gimple passes.
Don't skip dfinit.
(skip_pass): New function.
(execute_one_pass): Call skip_pass when skipping passes.
---
gcc/passes.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 58 insertions(+), 7 deletions(-)
diff --git a/gcc/passes.c b/gcc/passes.c
index 31262ed..6954d1e 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -59,6 +59,7 @@ along with GCC; see the file COPYING3. If not see
#include "cfgrtl.h"
#include "tree-ssa-live.h" /* For remove_unused_locals. */
#include "tree-cfgcleanup.h"
+#include "insn-addr.h" /* for INSN_ADDRESSES_ALLOC. */
insn-addr? Yuk.
using namespace gcc;
@@ -2315,26 +2316,73 @@ should_skip_pass_p (opt_pass *pass)
if (!cfun->pass_startwith)
return false;
- /* We can't skip the lowering phase yet -- ideally we'd
- drive that phase fully via properties. */
- if (!(cfun->curr_properties & PROP_ssa))
- return false;
+ /* For __GIMPLE functions, we have to at least start when we leave
+ SSA. */
+ if (pass->properties_destroyed & PROP_ssa)
+ {
+ if (!quiet_flag)
+ fprintf (stderr, "starting anyway when leaving SSA: %s\n", pass->name);
+ cfun->pass_startwith = NULL;
+ return false;
+ }
This seems to need a comment -- it's not obvious how destroying the SSA
property maps to a pass that can not be skipped.
- /* And also run any property provider. */
- if (pass->properties_provided != 0)
+ /* Run any property provider. */
+ if (pass->type == GIMPLE_PASS
+ && pass->properties_provided != 0)
return false;
So comment needed here too. I read this as "if a gimple pass provides a
property then it should not be skipped. Which means that an RTL pass
that provides a property can?
+ /* Don't skip df init; later RTL passes need it. */
+ if (strstr (pass->name, "dfinit") != NULL)
+ return false;
Which seems like a failing in RTL passes saying they need DF init.
+/* Skip the given pass, for handling passes before "startwith"
+ in __GIMPLE and__RTL-marked functions.
+ In theory, this ought to be a no-op, but some of the RTL passes
+ need additional processing here. */
+
+static void
+skip_pass (opt_pass *pass)
...
This all feels like a failing in how we handle state in the RTL world.
And I suspect it's prone to error. Imagine if I'm hacking on something
in the RTL world and my code depends on something else being set up. I
really ought to have a way within my pass to indicate what I depend on.
Having it hidden away in passes.c makes it easy to miss/forget.
+{
+ /* Pass "reload" sets the global "reload_completed", and many
+ things depend on this (e.g. instructions in .md files). */
+ if (strcmp (pass->name, "reload") == 0)
+ reload_completed = 1;
Seems like this ought to be a property provided by LRA/reload.
+
+ /* The INSN_ADDRESSES vec is normally set up by
+ shorten_branches; set it up for the benefit of passes that
+ run after this. */
+ if (strcmp (pass->name, "shorten") == 0)
+ INSN_ADDRESSES_ALLOC (get_max_uid ());
Similarly ought to be provided by shorten-branches
+
+ /* Update the cfg hooks as appropriate. */
+ if (strcmp (pass->name, "into_cfglayout") == 0)
+ {
+ cfg_layout_rtl_register_cfg_hooks ();
+ cfun->curr_properties |= PROP_cfglayout;
+ }
+ if (strcmp (pass->name, "outof_cfglayout") == 0)
+ {
+ rtl_register_cfg_hooks ();
+ cfun->curr_properties &= ~PROP_cfglayout;
+ }
+}
This feels somewhat different, but still a hack.
I don't have strong suggestions on how to approach this, but what we've
got here feels like a hack and one prone to bitrot.
jeff