Re: PING^2 : Re: [PATCH 08/10] i386: add 'final' and 'override' to scalar_chain vfunc impls

[David Malcolm <dmalcolm@redhat.com>]

On Fri, 2022-06-24 at 22:58 +0200, Uros Bizjak wrote:
> On Fri, Jun 24, 2022 at 8:19 PM David Malcolm <dmalcolm@redhat.com>
> wrote:
> > 
> > I'd like to ping this patch:
> >    https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595440.html
> > 
> > OK for trunk?
> 
> I have no idea what patch does, 

Sorry for any confusion; there was a description in the cover letter to
the kit, but you won't have seen that due to me CCing you to just this
one.

This is a C++11 feature, which helps document our intentions in those 
places where we're using virtual functions.  We can add "final" and
"override" to the decls of virtual function in derived classes, which
document to both human and automated readers of the code that the decl
is intended to override a virtual function in a base class.  In
particular, it can help catch commom mistakes where we intended to
override a virtual function, but messed up the prototypes and so merely
introduced a similar-looking function that never gets called.  For
example, G++ has a -Wsuggest-override warning (which I hope we'll
eventually be able to turn on during our bootstraps).


> but if all other targets do the same,
> x86 shouldn't be left behind.
> So, rubber-stamping OK.


Thanks.  As it happens, x86 is actually the only patch I've done this
cleanup for - but Jeff declared this cleanup to be obvious when
reviewing another patch in the kit, so I've pushed this (as r13-1309-
g0a8333ade9a03f), along with the rest of the kit.

Hope this makes sense
Dave



> 
> Thanks,
> Uros.
> 
> > 
> > Thanks
> > Dave
> > 
> > 
> > On Mon, 2022-06-13 at 14:30 -0400, David Malcolm wrote:
> > > Ping for this patch:
> > >   https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595440.html
> > > 
> > > OK for trunk?
> > > 
> > > Thanks
> > > Dave
> > > 
> > > On Mon, 2022-05-23 at 15:28 -0400, David Malcolm wrote:
> > > > gcc/ChangeLog:
> > > >         * config/i386/i386-features.h: Add "final" and "override"
> > > > to
> > > >         scalar_chain vfunc implementations as appropriate.
> > > > 
> > > > Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> > > > ---
> > > >  gcc/config/i386/i386-features.h | 16 ++++++++--------
> > > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/gcc/config/i386/i386-features.h
> > > > b/gcc/config/i386/i386-
> > > > features.h
> > > > index 5c307607ae5..f46a6d95b74 100644
> > > > --- a/gcc/config/i386/i386-features.h
> > > > +++ b/gcc/config/i386/i386-features.h
> > > > @@ -169,18 +169,18 @@ class general_scalar_chain : public
> > > > scalar_chain
> > > >   public:
> > > >    general_scalar_chain (enum machine_mode smode_, enum
> > > > machine_mode
> > > > vmode_);
> > > >    ~general_scalar_chain ();
> > > > -  int compute_convert_gain ();
> > > > +  int compute_convert_gain () final override;
> > > >   private:
> > > >    hash_map<rtx, rtx> defs_map;
> > > >    bitmap insns_conv;
> > > >    unsigned n_sse_to_integer;
> > > >    unsigned n_integer_to_sse;
> > > > -  void mark_dual_mode_def (df_ref def);
> > > > -  void convert_insn (rtx_insn *insn);
> > > > +  void mark_dual_mode_def (df_ref def) final override;
> > > > +  void convert_insn (rtx_insn *insn) final override;
> > > >    void convert_op (rtx *op, rtx_insn *insn);
> > > >    void convert_reg (rtx_insn *insn, rtx dst, rtx src);
> > > >    void make_vector_copies (rtx_insn *, rtx);
> > > > -  void convert_registers ();
> > > > +  void convert_registers () final override;
> > > >    int vector_const_cost (rtx exp);
> > > >  };
> > > > 
> > > > @@ -190,14 +190,14 @@ class timode_scalar_chain : public
> > > > scalar_chain
> > > >    timode_scalar_chain () : scalar_chain (TImode, V1TImode) {}
> > > > 
> > > >    /* Convert from TImode to V1TImode is always faster.  */
> > > > -  int compute_convert_gain () { return 1; }
> > > > +  int compute_convert_gain () final override { return 1; }
> > > > 
> > > >   private:
> > > > -  void mark_dual_mode_def (df_ref def);
> > > > +  void mark_dual_mode_def (df_ref def) final override;
> > > >    void fix_debug_reg_uses (rtx reg);
> > > > -  void convert_insn (rtx_insn *insn);
> > > > +  void convert_insn (rtx_insn *insn) final override;
> > > >    /* We don't convert registers to difference size.  */
> > > > -  void convert_registers () {}
> > > > +  void convert_registers () final override {}
> > > >  };
> > > > 
> > > >  } // anon namespace
> > > 
> > 
> > 
>