* [PATCH] Remove verify_no_unreachable_blocks
@ 2018-08-23 11:18 Richard Biener
2018-08-23 11:36 ` Steven Bosscher
2018-08-23 11:46 ` Tom de Vries
0 siblings, 2 replies; 6+ messages in thread
From: Richard Biener @ 2018-08-23 11:18 UTC (permalink / raw)
To: gcc-patches; +Cc: Tom de Vries
This removes verify_no_unreachable_blocks and implements checking
for unreachable blocks in inverted_post_order_compute by simply
looking if we reach a block without predecessors that is not the
entry block.
This solves a problem I ran into when (ab-)using BB_REACHABLE
in a pass and I got comparison failues because of -fchecking vs.
-fno-checking. It also should speed up checking builds.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Tom, does this make sense?
Thanks,
Richard.
2018-08-23 Richard Biener <rguenther@suse.de>
* cfganal.h (verify_no_unreachable_blocks): Remove.
* cfganal.c (verify_no_unreachable_blocks): Likewise.
(inverted_post_order_compute): Do not call verify_no_unreachable_blocks
instead assert when we reach a block without predecessor that is not
the entry block.
diff --git a/gcc/cfganal.c b/gcc/cfganal.c
index 3b80758e8f2..baf9f0562f9 100644
--- a/gcc/cfganal.c
+++ b/gcc/cfganal.c
@@ -186,18 +186,6 @@ find_unreachable_blocks (void)
free (worklist);
}
-/* Verify that there are no unreachable blocks in the current function. */
-
-void
-verify_no_unreachable_blocks (void)
-{
- find_unreachable_blocks ();
-
- basic_block bb;
- FOR_EACH_BB_FN (bb, cfun)
- gcc_assert ((bb->flags & BB_REACHABLE) != 0);
-}
-
\f
/* Functions to access an edge list with a vector representation.
Enough data is kept such that given an index number, the
@@ -800,9 +788,6 @@ inverted_post_order_compute (vec<int> *post_order,
basic_block bb;
post_order->reserve_exact (n_basic_blocks_for_fn (cfun));
- if (flag_checking)
- verify_no_unreachable_blocks ();
-
/* Allocate stack for back-tracking up CFG. */
auto_vec<edge_iterator, 20> stack (n_basic_blocks_for_fn (cfun) + 1);
@@ -866,7 +851,10 @@ inverted_post_order_compute (vec<int> *post_order,
time, check its predecessors. */
stack.quick_push (ei_start (pred->preds));
else
- post_order->quick_push (pred->index);
+ {
+ gcc_assert (pred->index == ENTRY_BLOCK);
+ post_order->quick_push (pred->index);
+ }
}
else
{
diff --git a/gcc/cfganal.h b/gcc/cfganal.h
index 122c665f7f6..ac3fe8f4617 100644
--- a/gcc/cfganal.h
+++ b/gcc/cfganal.h
@@ -50,7 +50,6 @@ private:
extern bool mark_dfs_back_edges (void);
extern void find_unreachable_blocks (void);
-extern void verify_no_unreachable_blocks (void);
struct edge_list * create_edge_list (void);
void free_edge_list (struct edge_list *);
void print_edge_list (FILE *, struct edge_list *);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Remove verify_no_unreachable_blocks
2018-08-23 11:18 [PATCH] Remove verify_no_unreachable_blocks Richard Biener
@ 2018-08-23 11:36 ` Steven Bosscher
2018-08-23 11:43 ` Richard Biener
2018-08-23 11:46 ` Tom de Vries
1 sibling, 1 reply; 6+ messages in thread
From: Steven Bosscher @ 2018-08-23 11:36 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, tdevries
On Thu, Aug 23, 2018 at 1:18 PM Richard Biener <> wrote:
> -/* Verify that there are no unreachable blocks in the current function. */
> -
> -void
> -verify_no_unreachable_blocks (void)
> -{
> - find_unreachable_blocks ();
> -
> - basic_block bb;
> - FOR_EACH_BB_FN (bb, cfun)
> - gcc_assert ((bb->flags & BB_REACHABLE) != 0);
Alternatively, just clear BB_REACHABLE here?
FOR_EACH_BB_FN (bb, cfun)
{
gcc_assert ((bb->flags & BB_REACHABLE) != 0);
bb->flags &= ~BB_REACHABLE;
}
The function is quite useful for debugging, I wouldn't remove it.
Ciao!
Steven
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Remove verify_no_unreachable_blocks
2018-08-23 11:36 ` Steven Bosscher
@ 2018-08-23 11:43 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2018-08-23 11:43 UTC (permalink / raw)
To: Steven Bosscher; +Cc: GCC Patches, tdevries
On Thu, 23 Aug 2018, Steven Bosscher wrote:
> On Thu, Aug 23, 2018 at 1:18 PM Richard Biener <> wrote:
> > -/* Verify that there are no unreachable blocks in the current function. */
> > -
> > -void
> > -verify_no_unreachable_blocks (void)
> > -{
> > - find_unreachable_blocks ();
> > -
> > - basic_block bb;
> > - FOR_EACH_BB_FN (bb, cfun)
> > - gcc_assert ((bb->flags & BB_REACHABLE) != 0);
>
> Alternatively, just clear BB_REACHABLE here?
That doesn't help me of course since I need BB_REACHABLE to be
preserved ;)
> FOR_EACH_BB_FN (bb, cfun)
> {
> gcc_assert ((bb->flags & BB_REACHABLE) != 0);
> bb->flags &= ~BB_REACHABLE;
> }
>
> The function is quite useful for debugging, I wouldn't remove it.
I certainly can keep it (unused).
Richard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Remove verify_no_unreachable_blocks
2018-08-23 11:18 [PATCH] Remove verify_no_unreachable_blocks Richard Biener
2018-08-23 11:36 ` Steven Bosscher
@ 2018-08-23 11:46 ` Tom de Vries
2018-08-23 12:06 ` Richard Biener
1 sibling, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2018-08-23 11:46 UTC (permalink / raw)
To: Richard Biener, gcc-patches
On 08/23/2018 01:18 PM, Richard Biener wrote:
> This removes verify_no_unreachable_blocks and implements checking
> for unreachable blocks in inverted_post_order_compute by simply
> looking if we reach a block without predecessors that is not the
> entry block.
>
I think that doesn't detect unreachable cyles: say you have blocks A and
B, and A -> B -> A.
> This solves a problem I ran into when (ab-)using BB_REACHABLE
> in a pass and I got comparison failues because of -fchecking vs.
> -fno-checking. It also should speed up checking builds.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Tom, does this make sense?
>
The intent of the check I added was to verify the assumption stated in
the comment. I don't know how serious it is if the assumption is violated.
Thanks,
- Tom
> Thanks,
> Richard.
>
> 2018-08-23 Richard Biener <rguenther@suse.de>
>
> * cfganal.h (verify_no_unreachable_blocks): Remove.
> * cfganal.c (verify_no_unreachable_blocks): Likewise.
> (inverted_post_order_compute): Do not call verify_no_unreachable_blocks
> instead assert when we reach a block without predecessor that is not
> the entry block.
>
> diff --git a/gcc/cfganal.c b/gcc/cfganal.c
> index 3b80758e8f2..baf9f0562f9 100644
> --- a/gcc/cfganal.c
> +++ b/gcc/cfganal.c
> @@ -186,18 +186,6 @@ find_unreachable_blocks (void)
> free (worklist);
> }
>
> -/* Verify that there are no unreachable blocks in the current function. */
> -
> -void
> -verify_no_unreachable_blocks (void)
> -{
> - find_unreachable_blocks ();
> -
> - basic_block bb;
> - FOR_EACH_BB_FN (bb, cfun)
> - gcc_assert ((bb->flags & BB_REACHABLE) != 0);
> -}
> -
> \f
> /* Functions to access an edge list with a vector representation.
> Enough data is kept such that given an index number, the
> @@ -800,9 +788,6 @@ inverted_post_order_compute (vec<int> *post_order,
> basic_block bb;
> post_order->reserve_exact (n_basic_blocks_for_fn (cfun));
>
> - if (flag_checking)
> - verify_no_unreachable_blocks ();
> -
> /* Allocate stack for back-tracking up CFG. */
> auto_vec<edge_iterator, 20> stack (n_basic_blocks_for_fn (cfun) + 1);
>
> @@ -866,7 +851,10 @@ inverted_post_order_compute (vec<int> *post_order,
> time, check its predecessors. */
> stack.quick_push (ei_start (pred->preds));
> else
> - post_order->quick_push (pred->index);
> + {
> + gcc_assert (pred->index == ENTRY_BLOCK);
> + post_order->quick_push (pred->index);
> + }
> }
> else
> {
> diff --git a/gcc/cfganal.h b/gcc/cfganal.h
> index 122c665f7f6..ac3fe8f4617 100644
> --- a/gcc/cfganal.h
> +++ b/gcc/cfganal.h
> @@ -50,7 +50,6 @@ private:
>
> extern bool mark_dfs_back_edges (void);
> extern void find_unreachable_blocks (void);
> -extern void verify_no_unreachable_blocks (void);
> struct edge_list * create_edge_list (void);
> void free_edge_list (struct edge_list *);
> void print_edge_list (FILE *, struct edge_list *);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Remove verify_no_unreachable_blocks
2018-08-23 11:46 ` Tom de Vries
@ 2018-08-23 12:06 ` Richard Biener
2018-08-23 13:29 ` Tom de Vries
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2018-08-23 12:06 UTC (permalink / raw)
To: Tom de Vries; +Cc: gcc-patches
On Thu, 23 Aug 2018, Tom de Vries wrote:
> On 08/23/2018 01:18 PM, Richard Biener wrote:
> > This removes verify_no_unreachable_blocks and implements checking
> > for unreachable blocks in inverted_post_order_compute by simply
> > looking if we reach a block without predecessors that is not the
> > entry block.
> >
>
> I think that doesn't detect unreachable cyles: say you have blocks A and
> B, and A -> B -> A.
Ah, true. Bah, I guess I can use some other flag in my pass.
> > This solves a problem I ran into when (ab-)using BB_REACHABLE
> > in a pass and I got comparison failues because of -fchecking vs.
> > -fno-checking. It also should speed up checking builds.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > Tom, does this make sense?
> >
>
> The intent of the check I added was to verify the assumption stated in
> the comment. I don't know how serious it is if the assumption is violated.
I think if you have reverse-not-reachable blocks (infinite loops w/o
fake exit edges) that are not reachable from entry it will ICE
or loop infintely.
Richard.
> Thanks,
> - Tom
>
> > Thanks,
> > Richard.
> >
> > 2018-08-23 Richard Biener <rguenther@suse.de>
> >
> > * cfganal.h (verify_no_unreachable_blocks): Remove.
> > * cfganal.c (verify_no_unreachable_blocks): Likewise.
> > (inverted_post_order_compute): Do not call verify_no_unreachable_blocks
> > instead assert when we reach a block without predecessor that is not
> > the entry block.
> >
> > diff --git a/gcc/cfganal.c b/gcc/cfganal.c
> > index 3b80758e8f2..baf9f0562f9 100644
> > --- a/gcc/cfganal.c
> > +++ b/gcc/cfganal.c
> > @@ -186,18 +186,6 @@ find_unreachable_blocks (void)
> > free (worklist);
> > }
> >
> > -/* Verify that there are no unreachable blocks in the current function. */
> > -
> > -void
> > -verify_no_unreachable_blocks (void)
> > -{
> > - find_unreachable_blocks ();
> > -
> > - basic_block bb;
> > - FOR_EACH_BB_FN (bb, cfun)
> > - gcc_assert ((bb->flags & BB_REACHABLE) != 0);
> > -}
> > -
> > \f
> > /* Functions to access an edge list with a vector representation.
> > Enough data is kept such that given an index number, the
> > @@ -800,9 +788,6 @@ inverted_post_order_compute (vec<int> *post_order,
> > basic_block bb;
> > post_order->reserve_exact (n_basic_blocks_for_fn (cfun));
> >
> > - if (flag_checking)
> > - verify_no_unreachable_blocks ();
> > -
> > /* Allocate stack for back-tracking up CFG. */
> > auto_vec<edge_iterator, 20> stack (n_basic_blocks_for_fn (cfun) + 1);
> >
> > @@ -866,7 +851,10 @@ inverted_post_order_compute (vec<int> *post_order,
> > time, check its predecessors. */
> > stack.quick_push (ei_start (pred->preds));
> > else
> > - post_order->quick_push (pred->index);
> > + {
> > + gcc_assert (pred->index == ENTRY_BLOCK);
> > + post_order->quick_push (pred->index);
> > + }
> > }
> > else
> > {
> > diff --git a/gcc/cfganal.h b/gcc/cfganal.h
> > index 122c665f7f6..ac3fe8f4617 100644
> > --- a/gcc/cfganal.h
> > +++ b/gcc/cfganal.h
> > @@ -50,7 +50,6 @@ private:
> >
> > extern bool mark_dfs_back_edges (void);
> > extern void find_unreachable_blocks (void);
> > -extern void verify_no_unreachable_blocks (void);
> > struct edge_list * create_edge_list (void);
> > void free_edge_list (struct edge_list *);
> > void print_edge_list (FILE *, struct edge_list *);
> >
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Remove verify_no_unreachable_blocks
2018-08-23 12:06 ` Richard Biener
@ 2018-08-23 13:29 ` Tom de Vries
0 siblings, 0 replies; 6+ messages in thread
From: Tom de Vries @ 2018-08-23 13:29 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On 08/23/2018 02:06 PM, Richard Biener wrote:
> On Thu, 23 Aug 2018, Tom de Vries wrote:
>
>> On 08/23/2018 01:18 PM, Richard Biener wrote:
>>> This removes verify_no_unreachable_blocks and implements checking
>>> for unreachable blocks in inverted_post_order_compute by simply
>>> looking if we reach a block without predecessors that is not the
>>> entry block.
>>>
>>
>> I think that doesn't detect unreachable cyles: say you have blocks A and
>> B, and A -> B -> A.
>
> Ah, true. Bah, I guess I can use some other flag in my pass.
>
>>> This solves a problem I ran into when (ab-)using BB_REACHABLE
>>> in a pass and I got comparison failues because of -fchecking vs.
>>> -fno-checking. It also should speed up checking builds.
>>>
>>> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>>>
>>> Tom, does this make sense?
>>>
>>
>> The intent of the check I added was to verify the assumption stated in
>> the comment. I don't know how serious it is if the assumption is violated.
>
> I think if you have reverse-not-reachable blocks (infinite loops w/o
> fake exit edges) that are not reachable from entry it will ICE
> or loop infintely.
>
Hmm, looking at the code a bit more, there's an infinite loop detection
mechanism, and I think the case I described above, would run into this
assert:
...
if (has_unvisited_bb && stack.is_empty ())
{
/* No blocks are reachable from EXIT at all.
Find a dead-end from the ENTRY, and restart the
iteration. */
basic_block be
= dfs_find_deadend (ENTRY_BLOCK_PTR_FOR_FN (cfun));
gcc_assert (be != NULL);
bitmap_set_bit (visited, be->index);
stack.quick_push (ei_start (be->preds));
}
...
So perhaps the code already verifies that there are no unreachable
blocks, and the comment needs to be updated to:
...
This function requires that all blocks in the CFG are reachable
from the ENTRY (but not necessarily from EXIT). Otherwise, it ICEs.
...
Thanks,
- Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-23 13:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-23 11:18 [PATCH] Remove verify_no_unreachable_blocks Richard Biener
2018-08-23 11:36 ` Steven Bosscher
2018-08-23 11:43 ` Richard Biener
2018-08-23 11:46 ` Tom de Vries
2018-08-23 12:06 ` Richard Biener
2018-08-23 13:29 ` Tom de Vries
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).