This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
basic_block flags, BB_VISITED
- From: Thomas Schwinge <thomas at codesourcery dot com>
- To: <gcc at gcc dot gnu dot org>, <gcc-patches at gcc dot gnu dot org>
- Cc: <kugan dot vivekanandarajah at linaro dot org>, Nathan Sidwell <nathan at acm dot org>
- Date: Fri, 14 Oct 2016 10:01:39 +0200
- Subject: basic_block flags, BB_VISITED
- Authentication-results: sourceware.org; auth=none
Hi!
After the "Add Early VRP" GCC trunk commit r240291 (Kugan CC for your
information), I've been observing all kinds of OpenACC offloading
failures. I now figured out what's going on.
The "evrp" pass uses basic_block's BB_VISITED flag. It first clears
these all, gcc/tree-vrp.c:execute_early_vrp:
FOR_EACH_BB_FN (bb, cfun)
{
bb->flags &= ~BB_VISITED;
..., then does its processing, and at the end, clears these again:
FOR_EACH_BB_FN (bb, cfun)
bb->flags &= ~BB_VISITED;
I note that this looks slightly different from what
gcc/cfg.c:clear_bb_flags whould be doing, which works from
ENTRY_BLOCK_PTR_FOR_FN onwards:
/* Clear all basic block flags that do not have to be preserved. */
void
clear_bb_flags (void)
{
basic_block bb;
FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR_FOR_FN (cfun), NULL, next_bb)
bb->flags &= BB_FLAGS_TO_PRESERVE;
}
In the specific case that I've been looking at, "evrp" processing doesn't
change the code other than for shifting some IDs because of adding a
(dummy one-argument) PHI node. And the problem now exactly is that the
ENTRY_BLOCK_PTR_FOR_FN's BB_VISITED flag is still set, and so
gcc/omp-low.c:oacc_loop_discover_walk will bail out without doing any
processing:
/* DFS walk of basic blocks BB onwards, creating OpenACC loop
structures as we go. By construction these loops are properly
nested. */
static void
oacc_loop_discover_walk (oacc_loop *loop, basic_block bb)
{
[...]
if (bb->flags & BB_VISITED)
return;
follow:
bb->flags |= BB_VISITED;
[...]
/* Discover the OpenACC loops marked up by HEAD and TAIL markers for
the current function. */
static oacc_loop *
oacc_loop_discovery ()
{
basic_block bb;
oacc_loop *top = new_oacc_loop_outer (current_function_decl);
oacc_loop_discover_walk (top, ENTRY_BLOCK_PTR_FOR_FN (cfun));
[...]
Now, gcc/cfg-flags.def says:
[Most of the basic block] flags may be cleared by clear_bb_flags(). It is generally
a bad idea to rely on any flags being up-to-date. */
[...]
/* A general visited flag for passes to use. */
DEF_BASIC_BLOCK_FLAG(VISITED, 13)
The BB_VISITED flag to me seems to always be of very local use only.
Should we be more strict and strengthen the above "may be"/"bad idea"
wording?
Does the "evrp" pass need to get changes applied, to properly clear
BB_VISITED (ENTRY_BLOCK_PTR_FOR_FN, in particular)? Or, in contrast, as
we're not to "rely on any flags being up-to-date", should the BB_VISITED
clearing at the end of gcc/tree-vrp.c:execute_early_vrp be removed in
fact?
>From (really just) a quick look, I can't tell the exact policy that other
GCC passes are applying/using regarding the basic_block flags...
The following patch does cure the testsuite regressions:
--- gcc/omp-low.c
+++ gcc/omp-low.c
@@ -19342,6 +19342,7 @@ oacc_loop_discovery ()
basic_block bb;
oacc_loop *top = new_oacc_loop_outer (current_function_decl);
+ clear_bb_flags ();
oacc_loop_discover_walk (top, ENTRY_BLOCK_PTR_FOR_FN (cfun));
/* The siblings were constructed in reverse order, reverse them so
Is something like that what we should do? Should clear_bb_flags be
called here, or early in gcc/omp-low.c:execute_oacc_device_lower (like
some other GCC passes seem to be doing)?
Grüße
Thomas