On Tue, 24 Jun 2025 14:05:54 +0200, "Robin Dapp" wrote:
> Hi Ma Jin,
>
> thanks for looking into this, it has been on my todo list with very low
> priority since the vsetvl rewrite.
Yes, I've noticed this for quite some time. While the logic itself is sound,
it strikes me as quite odd every time I see it, and I consistently receive a
warning during the Coverity checks. As a result, I decided to refactor it :)
> > + /* Handle case with no predecessors (including ENTRY block). */
> > + if (EDGE_COUNT (b->preds) == 0)
> > {
> > - e = EDGE_PRED (b, ix);
> > - bitmap_copy (dst, src[e->src->index]);
> > - break;
> > + bitmap_clear (dst);
> > + return;
>
> This is ok.
>
> > + /* Initialize with first predecessor's bitmap. */
> > + edge first_pred = EDGE_PRED (b, 0);
> > + bitmap_copy (dst, src[first_pred->src->index]);
> > +
> > + /* Union remaining predecessors' bitmaps. */
> > + for (unsigned ix = 1; ix < EDGE_COUNT (b->preds); ix++)
> > + {
> > + edge e = EDGE_PRED (b, ix);
> > + const sbitmap pred_src = src[e->src->index];
> > +
> > + /* Perform bitmap OR operation element-wise. */
> > + for (unsigned i = 0; i < dst->size; i++)
> > + dst->elms[i] |= pred_src->elms[i];
> > + }
>
> To my taste this could be simplified further like
>
> FOR_EACH_EDGE (e, ei, b->preds)
> {
> if (ei.index == 0)
> {
> bitmap_copy (dst, src[e->src->index]);
> continue;
> }
>
> bitmap_ior (dst, dst, src[e->src->index]);
> }
>
> Does that work as well?
That's an excellent suggestion! I'll make the necessary adjustments and
resubmit shortly. Thanks.
Best regards,
Jin Ma
> --
> Regards
> Robin
