This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH: use type-safe vectors for basic block edges
- From: Steven Bosscher <stevenb at suse dot de>
- To: "Ben Elliston" <bje at au1 dot ibm dot com>,Richard Henderson <rth at redhat dot com>
- Cc: Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>,gcc-patches at gcc dot gnu dot org, mark at codesourcery dot com
- Date: Tue, 14 Sep 2004 08:50:11 +0200
- Subject: Re: PATCH: use type-safe vectors for basic block edges
- Organization: SUSE Labs
- References: <87k6uyfi83.fsf@au.ibm.com> <20040913183229.GB26642@redhat.com> <87zn3t6220.fsf@au.ibm.com>
On Tuesday 14 September 2004 04:01, Ben Elliston wrote:
> How does this revised patch to basic-block.h look?
>
> The only unfortunate consequence of this patch is that there are no
> longer any safeguards to prevent the user from accidentally using the
> same iterator variable in a nest of FOR_EACH_EDGE loops. By
> previously having an automatic variable declared inside a dedicated
> scoping block meant that this class of errors are eradicated.
It is a real shame to lose this checking.
In the initial works on this branch, almost all bugs were of the
kind where inside a FOR_EACH_EDGE-style iterator much like the
proposed patch (ie. without the END_FOR_EACH_EDGE stuff), an edge
would be removed from the vector, and there was no way for the
iterator to know about it.
Take for example:
for (e = EXIT_BLOCK_PTR->pred; e; e = next)
{
next = e->pred_next;
if (!(e->flags & EDGE_ABNORMAL))
redirect_edge_succ (e, exit_block);
}
This has no easy representation with the FOR_EACH_EDGE iterators,
a successor edge of the predecessors of EXIT_BLOCK_PTR would be
redirected, with the implicit side effect that the edge no longer
lead to the EXIT_BLOCK_PTR. So another edge would take the place
of the redirected edge in the EXIT_BLOCK predecessor edge vector.
But the iterator would move on to the "next" edge, ie. the next
element in the vector - stepping over the edge that moved into the
slot of the redirected edge.
I fixed at least half a dozen of these bugs before I got fed up
with it and discussed a checking mechanism with Ben. He told me
that he still found many bugs like this with the checking code.
My idea at the time was an iterator macro modelled after
EXECUTE_IF_SET_IN_BITMAP, where a "CODE" argument is embedded the
generic bitmap walking code from the EXECUTE_IF_SET_IN_BITMAP macro:
/* Loop over all elements of SBITSET, starting with MIN. */
#define EXECUTE_IF_SET_IN_SBITMAP(SBITMAP, MIN, N, CODE) \
do { \
unsigned int word_num_; \
unsigned int bit_num_ = (MIN) % (unsigned int) SBITMAP_ELT_BITS; \
unsigned int size_ = (SBITMAP)->size; \
SBITMAP_ELT_TYPE *ptr_ = (SBITMAP)->elms; \
\
for (word_num_ = (MIN) / (unsigned int) SBITMAP_ELT_BITS; \
word_num_ < size_; word_num_++, bit_num_ = 0) \
{ \
SBITMAP_ELT_TYPE word_ = ptr_[word_num_]; \
\
if (word_ != 0) \
for (; bit_num_ < SBITMAP_ELT_BITS; bit_num_++) \
{ \
SBITMAP_ELT_TYPE _mask = (SBITMAP_ELT_TYPE) 1 << bit_num_; \
\
if ((word_ & _mask) != 0) \
{ \
word_ &= ~ _mask; \
(N) = word_num_ * SBITMAP_ELT_BITS + bit_num_; \
CODE; \
if (word_ == 0) \
break; \
} \
} \
} \
} while (0)
A similar thing would work for walking the edge vectors, something
like the following,
#define FOR_EACH_EDGE (EDGE_VEC, EDGE, CODE)
do { \
VEC(edge) *__ev = (EDGE_VEC); \
edge __check_edge; \
unsigned int __ix; \
unsigned int __num_edges = EDGE_COUNT (__ev); \
(EDGE) = NULL; \
for (__ix = 0; VEC_iterate (edge, __ev, __ix, (EDGE)); __ix++) \
{ \
if (ENABLE_VEC_CHECKING) \
__check_edge = (EDGE); \
(EDGE) = __check_edge = (EDGE); \
CODE; \
if (ENABLE_VEC_CHECKING \
&& (__ix >= EDGE_COUNT (__ev) \
|| EDGE_I (__ev, __ix) != __check_edge)) \
internal_error ("edge modified in FOR_EACH_EDGE: %s:%s", \
__FILE__, __FUNCTION__); \
} \
if (ENABLE_VEC_CHECKING \
&& __num_edges > EDGE_COUNT (__ev)) \
internal_error ("insufficient edges FOR_EACH_EDGE: %s:%s", \
__FILE__, __FUNCTION__); \
if (ENABLE_VEC_CHECKING \
&& __num_edges < EDGE_COUNT (__ev)) \
internal_error ("excess edges FOR_EACH_EDGE: %s:%s", \
__FILE__, __FUNCTION__); \
} \
while (0)
Some people did not like this idea because it is apparently not
always possible to debug the code in "CODE". That is what Ben to
invent END_FOR_EACH_EDGE.
I would really like to not lose this kind of checking :-/
Gr.
Steven