On Fri, Oct 09, 2026 at 09:51:01AM +0200, Richard Biener wrote:
> On Thu, 8 Oct 2026, Aldy Hernandez wrote:
>
> > RPO VN is driven through a couple free functions that we can abstract
> > out to a class to make things a bit tidier, and to make it easier to
> > call the back threader from within FRE to get value numbering for
> > free.
> >
> > Make it an object that we can later sub-class into a path aware VN
> > object.
> >
> > OK for trunk?
>
> Hmm. I think .walk () should be part of constructing (it's a bad
> name anyway), that makes it obvious it's a single-shot thing.
>
> Overall it feels a bit of a half-way transition, merely covering
> parts of the API, so I don't think it's much of an improvement.
>
> As it is more like a "VN driver" class, maybe naming it accordingly
> and _not_ move functions into it as member functions, just encapsulate
> the toplevel flow, construct (with value-number), (optional) eliminate,
> destruction.
>
> So I think this patch goes too far and too little.
>
> Not knowing how you'd use it exactly doesn't help making a constructive
> suggestion, of course ;)
Ughhh sorry, reviewing blind is a bit unfair :). I should provide
some more context.
The idea is that the value numbers outlive the walk, and the path_vn
subclass value numbers each candidate path's blocks again using the
walk's rpo_elim and RPO map.
If I understand your suggestion, how about something like this:
class vn_driver
{
public:
vn_driver (function *, bool iterate, bool eliminate,
vn_lookup_kind = VN_WALKREWRITE);
vn_driver (function *, edge entry, bitmap exit_bbs, bool iterate,
bool eliminate, bool skip_entry_phis, vn_lookup_kind);
virtual ~vn_driver ();
unsigned todo () const;
// Split out the last part of elimination so the threader can see IL
// consistent with the value numbers. Otherwise it can trip
// over released SSA names, or a CFG the VNs no longer describe.
unsigned eliminate_finish ();
protected:
// path_vn will use these
rpo_elim *m_avail;
int *m_bb_to_rpo;
};
class path_vn : public vn_driver
{
public:
bool reset_path (const vec<basic_block> &path);
void clear_path ();
tree valnum (tree name);
};
do_rpo_vn_1 would need to be passed down rpo_elim and bb_to_rpo from
the driver so path_vn could use them.
The driver's use would be something like this:
// do_rpo_vn:
vn_driver vn (fn, entry, exit_bbs, iterate, eliminate,
skip_entry_phis, kind);
return vn.todo ();
// PRE
vn_driver vn (fun, true, false, VN_WALK);
// What's left of run_rpo_vn
assign_value_ids ();
...
todo |= eliminate_with_rpo_vn (inserted_exprs);
// FRE + threading
path_vn vn (fun, iterate_p, true);
// The path solver calls reset_path and valnum
threader.search (vn);
todo = vn.todo () | vn.eliminate_finish ();
todo |= threader.apply ();
Is that closer to what you had in mind?
Aldy
