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.
Andrew