[07/10] value-relation.h: add 'final' and 'override' to relation_oracle vfunc impls
| Message ID | 20220523192834.3785673-8-dmalcolm@redhat.com |
|---|---|
| State | Committed |
| Commit | b06b84dbca5a11615da7193d74abfb74beaaf7ac |
| Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C3D953857413 for <patchwork@sourceware.org>; Mon, 23 May 2022 19:33:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C3D953857413 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1653334381; bh=4LCLrGMzggmHA8Fa36BTBm78dOCFPBbgYHfjB3/cPC4=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Wh7TL4T4lAHyBNEhzy+WxFDSYaRPntv6XwgrrFUJt9ibTBkuB1gB3HdgHm4rqf+4e vtb5ISN29rwyoNiO2pAIMFOX7PWnlN7RXfAi2Km9lRGYKXPLVB1oR+CfV7GdCCm52L 2WyWSsUW3XPWv7fbQyrEyE+oE5F1qGsi2UqZOKWc= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 221823858C56 for <gcc-patches@gcc.gnu.org>; Mon, 23 May 2022 19:28:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 221823858C56 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-586-SXzVlYUxMMSw4yOi3SVp-Q-1; Mon, 23 May 2022 15:28:40 -0400 X-MC-Unique: SXzVlYUxMMSw4yOi3SVp-Q-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2598085A5AA for <gcc-patches@gcc.gnu.org>; Mon, 23 May 2022 19:28:40 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.17.26]) by smtp.corp.redhat.com (Postfix) with ESMTP id 010772166B25; Mon, 23 May 2022 19:28:39 +0000 (UTC) To: gcc-patches@gcc.gnu.org Subject: [PATCH 07/10] value-relation.h: add 'final' and 'override' to relation_oracle vfunc impls Date: Mon, 23 May 2022 15:28:31 -0400 Message-Id: <20220523192834.3785673-8-dmalcolm@redhat.com> In-Reply-To: <20220523192834.3785673-1-dmalcolm@redhat.com> References: <20220523192834.3785673-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: David Malcolm via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: David Malcolm <dmalcolm@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
| Series |
Add 'final' and 'override' where missing
|
|
Commit Message
David Malcolm
May 23, 2022, 7:28 p.m. UTC
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(-)
Comments
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;
Final implies we can't further derive from the derived class, right? If so we may want just override. 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; > > >
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; > > > > > >
On 6/13/22 21:24, David Malcolm wrote: > 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. OK by me. Andrew
On Wed, 2022-06-15 at 09:33 -0400, Andrew MacLeod wrote: > On 6/13/22 21:24, David Malcolm wrote: > > 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. > > OK by me. Thanks; I've pushed this to trunk as r13-1118-gb06b84dbca5a11, after retesting (bootstrapped & regrtested again on x86_64-pc-linux-gnu). FWIW, as noted in https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595437.html I tried hacking in -Werror=suggest-override to the Makefile, and there were a bunch of places that it suggested could use "override" in gcc/range.op.cc, but I decided not to touch that, because it seems to be under heavy development by you (and also, frankly, out of laziness on my part). So maybe you might want to look at adding those at some point, to avoid snafus where you meant to override something, but got the signature slightly wrong, etc. Dave
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;