On Thu, Sep 28, 2023 at 09:29:31AM +0000, Richard Biener wrote:
> > The following patch splits the bitmap_head class into a POD
> > struct bitmap_head_pod and bitmap_head derived from it with non-trivial
> > default constexpr constructor. Most code should keep using bitmap_head
> > as before, bitmap_head_pod is there just for the cases where we want to
> > embed the bitmap head into a vector which we want to e.g. {quick,safe}_grow
> > and in a loop bitmap_initialize it afterwards (to avoid having to
> > {quick,safe}_grow_cleared them just to overwrite with bitmap_initialize).
> > The patch is larger than I hoped, because e.g. some code just used bitmap
> > and bitmap_head * or const_bitmap and const bitmap_head * interchangeably.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> OK if there are no comments indicating otherwise.
A counter argument against this patch would be that it weakens the intent
to catch uses of uninitialized bitmaps for saving a few compile time cycles.
If one uses
bitmap_head var;
bitmap_initialize (&var, NULL);
etc., we spend those extra cycles to initialize it and nothing is told that
bitmap_initialize overwrites the whole var without ever using of any of its
elements, so DSE can't eliminate that. And in the vec case which prompted
this patch it was about
vec<bitmap_head> a;
a.create (n);
a.safe_grow (n); // vs. a.safe_grow_cleared (n);
for (int i = 0; i < n; ++i)
bitmap_initialize (&a[i], NULL);
When using bitmap_head_pod, one needs to ensure initialization without
help to catch such mistakes.
Jakub
