Re: PING Re: [PATCH 07/10] value-relation.h: add 'final' and 'override' to relation_oracle vfunc impls

[David Malcolm <dmalcolm@redhat.com>]

On Mon, 2022-06-13 at 20:45 -0400, Aldy Hernandez wrote:
> Final implies we can't further derive from the derived class, right?

"final" on a vfunc implies that nothing overrides that vfunc, but you
could still have further derived classes.

You can think of it as "nothing further overrides this vfunc", as both
a hint to the human reader, and an optimization hint to the compiler.  

You can always just remove the "final" if you want to override it in
the future (unless the override is happening in a plugin, I suppose).

> If so
> we may want just override.

"override" is indeed probably more useful, in that it documents to
compilers and human readers that you intend this to override a vfunc.

FWIW I wrote the patch by using both "final" and "override", and then
dropping the "final" everywhere I needed to to get it to compile.

Dave


> 
> Andrew, what are your thoughts?
> 
> Thanks for doing this.
> 
> Aldy
> 
> On Mon, Jun 13, 2022, 14:28 David Malcolm <dmalcolm@redhat.com> wrote:
> 
> > Ping re this patch:
> >   https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595438.html
> > 
> > OK for trunk?
> > 
> > Thanks
> > Dave
> > 
> > 
> > On Mon, 2022-05-23 at 15:28 -0400, David Malcolm wrote:
> > > gcc/ChangeLog:
> > >         * value-relation.h: Add "final" and "override" to
> > > relation_oracle
> > >         vfunc implementations as appropriate.
> > > 
> > > Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> > > ---
> > >  gcc/value-relation.h | 38 +++++++++++++++++++++-----------------
> > >  1 file changed, 21 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/gcc/value-relation.h b/gcc/value-relation.h
> > > index 19762d8ce2b..478729be0bf 100644
> > > --- a/gcc/value-relation.h
> > > +++ b/gcc/value-relation.h
> > > @@ -130,14 +130,15 @@ public:
> > >    equiv_oracle ();
> > >    ~equiv_oracle ();
> > > 
> > > -  const_bitmap equiv_set (tree ssa, basic_block bb);
> > > +  const_bitmap equiv_set (tree ssa, basic_block bb) final
> > > override;
> > >    void register_relation (basic_block bb, relation_kind k, tree
> > > ssa1,
> > > -                         tree ssa2);
> > > +                         tree ssa2) override;
> > > 
> > > -  relation_kind query_relation (basic_block, tree, tree);
> > > -  relation_kind query_relation (basic_block, const_bitmap,
> > > const_bitmap);
> > > -  void dump (FILE *f, basic_block bb) const;
> > > -  void dump (FILE *f) const;
> > > +  relation_kind query_relation (basic_block, tree, tree) override;
> > > +  relation_kind query_relation (basic_block, const_bitmap,
> > > const_bitmap)
> > > +    override;
> > > +  void dump (FILE *f, basic_block bb) const override;
> > > +  void dump (FILE *f) const override;
> > > 
> > >  protected:
> > >    bitmap_obstack m_bitmaps;
> > > @@ -185,14 +186,16 @@ public:
> > >    dom_oracle ();
> > >    ~dom_oracle ();
> > > 
> > > -  void register_relation (basic_block bb, relation_kind k, tree
> > > op1,
> > > tree op2);
> > > +  void register_relation (basic_block bb, relation_kind k, tree
> > > op1,
> > > tree op2)
> > > +    final override;
> > > 
> > > -  relation_kind query_relation (basic_block bb, tree ssa1, tree
> > > ssa2);
> > > +  relation_kind query_relation (basic_block bb, tree ssa1, tree
> > > ssa2)
> > > +    final override;
> > >    relation_kind query_relation (basic_block bb, const_bitmap b1,
> > > -                                  const_bitmap b2);
> > > +                               const_bitmap b2) final override;
> > > 
> > > -  void dump (FILE *f, basic_block bb) const;
> > > -  void dump (FILE *f) const;
> > > +  void dump (FILE *f, basic_block bb) const final override;
> > > +  void dump (FILE *f) const final override;
> > >  private:
> > >    bitmap m_tmp, m_tmp2;
> > >    bitmap m_relation_set;  // Index by ssa-name. True if a relation
> > > exists
> > > @@ -229,15 +232,16 @@ class path_oracle : public relation_oracle
> > >  public:
> > >    path_oracle (relation_oracle *oracle = NULL);
> > >    ~path_oracle ();
> > > -  const_bitmap equiv_set (tree, basic_block);
> > > -  void register_relation (basic_block, relation_kind, tree, tree);
> > > +  const_bitmap equiv_set (tree, basic_block) final override;
> > > +  void register_relation (basic_block, relation_kind, tree, tree)
> > > final override;
> > >    void killing_def (tree);
> > > -  relation_kind query_relation (basic_block, tree, tree);
> > > -  relation_kind query_relation (basic_block, const_bitmap,
> > > const_bitmap);
> > > +  relation_kind query_relation (basic_block, tree, tree) final
> > > override;
> > > +  relation_kind query_relation (basic_block, const_bitmap,
> > > const_bitmap)
> > > +    final override;
> > >    void reset_path ();
> > >    void set_root_oracle (relation_oracle *oracle) { m_root =
> > > oracle; }
> > > -  void dump (FILE *, basic_block) const;
> > > -  void dump (FILE *) const;
> > > +  void dump (FILE *, basic_block) const final override;
> > > +  void dump (FILE *) const final override;
> > >  private:
> > >    void register_equiv (basic_block bb, tree ssa1, tree ssa2);
> > >    equiv_chain m_equiv;
> > 
> > 
> >