On Fri, Nov 28, 2014 at 11:37 AM, Kaz Kojima <kkoj...@rr.iij4u.or.jp> wrote:
> Hi,
>
> I've got odd ICEs with some bitmap operations when working on
> sh-lra branch.
> bitmap_ior and bitmap_ior_and_compl are the bitmap operations
> for DST = A | B and DST = A | (B & ~KILL) respectively. It
> looks they could make bitmaps with the wrong "current" pointer.
> One small example is
>
> DST: [(bit 0, bit 127), indx=0] <-> [(bit 151), indx=1] -> 0
> A: [(bit 141), indx=1] -> 0
> B: [(bit 142), indx=1] -> 0
>
> where assuming that BITMAP_ELEMENT_ALL_BITS is 128.
> In this case, bitmap_ior makes DST into the list
>
> [(bit 141, bit 142), indx=1] <-> [(bit 151), indx=1] -> 0
>
> and unlinks the second element with bitmap_elt_clear_from.
> If the "current" pointer of DST pointed the 2nd element,
> the current pointer points the element freed from the list:
>
> DST: [(bit 141, bit 142), indx=1] -> 0
> DST->current: [(bit 151), indx=1]
>
> even after bitmap_elt_clear_from done.
> bitmap_elt_clear_from has the code to fix the current pointer
> for the correct list of which elements have strictly ascending
> indx values. It doesn't work for the above list because its
> elements have the same indx.
> It seems that bitmap_and, bitmap_and_compl and bitmap_xor have
> the line to avoid this issue already. The patch below adds
> the same line to bitmap_ior and bitmap_ior_and_compl.
> The patch is tested on i686-pc-linux-gnu with bootstrap and
> the top level "make -k check".
Huh? Wasn't this fixed by Mike a few weeks ago?
Richard.
> Regards,
> kaz
> --
> * bitmap.c (bitmap_ior): Reset the current pointer before calling
> bitmap_elt_clear_from.
> (bitmap_ior_and_compl): Likewise.
>
> diff --git a/bitmap.c b/bitmap.c
> index 8f7f306..ace6aea 100644
> --- a/bitmap.c
> +++ b/bitmap.c
> @@ -1595,6 +1595,8 @@ bitmap_ior (bitmap dst, const_bitmap a, const_bitmap b)
> if (dst_elt)
> {
> changed = true;
> + /* Ensure that dst->current is valid. */
> + dst->current = dst->first;
> bitmap_elt_clear_from (dst, dst_elt);
> }
> gcc_checking_assert (!dst->current == !dst->first);
> @@ -1951,6 +1953,8 @@ bitmap_ior_and_compl (bitmap dst, const_bitmap a,
> const_bitmap b, const_bitmap k
> if (dst_elt)
> {
> changed = true;
> + /* Ensure that dst->current is valid. */
> + dst->current = dst->first;
> bitmap_elt_clear_from (dst, dst_elt);
> }
> gcc_checking_assert (!dst->current == !dst->first);
