Re: [RFC] redesigning callbacks for make_forwarder_block

On Thu, Apr 16, 2026 at 2:08 AM Andrew Stubbs <[email protected]> wrote:
>
> On 15/04/2026 01:44, Andrew Pinski via Gcc wrote:
> > Hi all,
> >    Right now make_forwarder_block takes two callbacks functions. The
> > second one (new_bb_cbk) looks unused; from what I can tell the last
> > usage was r0-78960-g89f8f30f356532 which allowed it to be NULL even
> > :). So removing that is not an issue.
> >
> > The other (redirect_edge_p), is what I am getting at here. Currently
> > the callback almost always (except in cleanup_tree_cfg_noloop), uses a
> > global variable to check against for the return value.
> >
> > E.g.
> > mfb_redirect_edges_in_set in cfgloop.cc has a `hash_set<edge> *` to
> > check against to return true.
> > mfb_keep_just in cfgloopmanip.cc has a edge to see if we should return false.
> >
> > Would it be ok to use `void*` here for the callback or should we do
> > something more type safe like a class with virtual function?
> >
> > I am adding another use of make_forwarder_block and I am not a fan of
> > the global which is why I am asking.
>
> I too encountered this recently and wondered "why?", when the rest of
> the compiler uses the usual C "void *data" trick.
>
> > The virtual function case would be something like:
> > struct redirect_edge_p
> > {
> >    virtual bool callback (edge) = 0;
> > };
> >
> > and then each place would do something like:
> > struct mfb_keep_just : redirect_edge_p
> > {
> >    edge mfb_kj_edge;
> >    virtual bool callback (edge e) overload { return e != mfb_kj_edge; }
> > };
> >
> > What do others think? Note a lambda won't work here as that is a local
> > type only.
>
> This is quite verbose, but it does allow a nested definition close to
> the call site, stack allocation of the "captured" variables, and is
> nearly as readable as a lambda, so I could live with it.
>
> I am not a C++ expert though.

Anyways I submitted
https://gcc.gnu.org/pipermail/gcc-patches/2026-April/713024.html which
just adds the `void*` argument.
Note it does depend on the patch which removes the unused argument
too: https://gcc.gnu.org/pipermail/gcc-patches/2026-April/712949.html
.
Thanks,
Andrew

>
> Andrew