From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 99059 invoked by alias); 7 Feb 2019 17:23:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 99038 invoked by uid 89); 7 Feb 2019 17:23:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy= X-HELO: mail-wm1-f65.google.com Received: from mail-wm1-f65.google.com (HELO mail-wm1-f65.google.com) (209.85.128.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 07 Feb 2019 17:23:12 +0000 Received: by mail-wm1-f65.google.com with SMTP id g67so704056wmd.2 for ; Thu, 07 Feb 2019 09:23:11 -0800 (PST) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:4c97:6d52:2cea:997b? ([2001:8a0:f913:f700:4c97:6d52:2cea:997b]) by smtp.gmail.com with ESMTPSA id x22sm9248795wmj.40.2019.02.07.09.23.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 07 Feb 2019 09:23:08 -0800 (PST) Subject: Re: [PATCH 3/8] C++-ify ravenscar_arch_ops To: Tom Tromey , gdb-patches@sourceware.org References: <20190207094016.368-1-tom@tromey.com> <20190207094016.368-4-tom@tromey.com> Cc: Tom Tromey From: Pedro Alves Message-ID: <2a5b8a83-725e-25d0-eca6-ccddef902d23@redhat.com> Date: Thu, 07 Feb 2019 17:23:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190207094016.368-4-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-02/txt/msg00069.txt.bz2 On 02/07/2019 09:40 AM, Tom Tromey wrote: > From: Tom Tromey > > This turns ravenscar_arch_ops into an abstract base class and updates > all the places where it is used. This is an improvement because it > avoids any possibility of forgetting to set one of the function > pointers. It also makes clear that these functions aren't intended to > be changed dynamically. LGTM, though I'd probably eliminate the prepare_to_store method or make it not-abstract -- seems that no architecture does anything with it. Thanks, Pedro Alves > > gdb/ChangeLog > 2019-02-07 Tom Tromey > > * sparc-ravenscar-thread.c (struct sparc_ravenscar_ops): Derive > from ravenscar_arch_ops. > (sparc_ravenscar_ops::fetch_registers) > (sparc_ravenscar_ops::prepare_to_store) > (sparc_ravenscar_ops::store_registers): Now methods. > (sparc_ravenscar_ops): Redefine. > * ravenscar-thread.h (struct ravenscar_arch_ops): Add virtual > methods and destructor. Remove members. > * ravenscar-thread.c (ravenscar_thread_target::fetch_registers) > (ravenscar_thread_target::store_registers) > (ravenscar_thread_target::prepare_to_store): Update. > * ppc-ravenscar-thread.c (ppc_ravenscar_generic_prepare_to_store): > Remove. > (struct ppc_ravenscar_powerpc_ops): Derive from > ravenscar_arch_ops. > (ppc_ravenscar_powerpc_ops::fetch_registers) > (ppc_ravenscar_powerpc_ops::store_registers): Now methods. > (ppc_ravenscar_powerpc_ops): Redefine. > (struct ppc_ravenscar_e500_ops): Derive from ravenscar_arch_ops. > (ppc_ravenscar_e500_ops::fetch_registers) > (ppc_ravenscar_e500_ops::store_registers): Now methods. > (ppc_ravenscar_e500_ops): Redefine. > * aarch64-ravenscar-thread.c > (aarch64_ravenscar_generic_prepare_to_store): Remove. > (struct aarch64_ravenscar_ops): Derive from ravenscar_arch_ops. > (aarch64_ravenscar_fetch_registers) > (aarch64_ravenscar_store_registers): Now methods. > (aarch64_ravenscar_ops): Redefine. > --- > gdb/ChangeLog | 31 ++++++++++++++++ > gdb/aarch64-ravenscar-thread.c | 51 ++++++++++----------------- > gdb/ppc-ravenscar-thread.c | 64 +++++++++++++++------------------- > gdb/ravenscar-thread.c | 6 ++-- > gdb/ravenscar-thread.h | 10 ++++-- > gdb/sparc-ravenscar-thread.c | 30 +++++++--------- > 6 files changed, 100 insertions(+), 92 deletions(-) > > diff --git a/gdb/aarch64-ravenscar-thread.c b/gdb/aarch64-ravenscar-thread.c > index 1234650a2b7..fa99d896ffe 100644 > --- a/gdb/aarch64-ravenscar-thread.c > +++ b/gdb/aarch64-ravenscar-thread.c > @@ -133,15 +133,6 @@ aarch64_ravenscar_generic_fetch_registers > } > } > > -/* to_prepare_to_store when inferior_ptid is different from the running > - thread. */ > - > -static void > -aarch64_ravenscar_generic_prepare_to_store (struct regcache *regcache) > -{ > - /* Nothing to do. */ > -} > - > /* to_store_registers when inferior_ptid is different from the running > thread. */ > > @@ -175,34 +166,28 @@ static const struct ravenscar_reg_info aarch64_reg_info = > ARRAY_SIZE (aarch64_context_offsets), > }; > > -/* Implement the to_fetch_registers ravenscar_arch_ops method > - for most Aarch64 targets. */ > - > -static void > -aarch64_ravenscar_fetch_registers (struct regcache *regcache, int regnum) > +struct aarch64_ravenscar_ops : public ravenscar_arch_ops > { > - aarch64_ravenscar_generic_fetch_registers > - (&aarch64_reg_info, regcache, regnum); > -} > - > -/* Implement the to_store_registers ravenscar_arch_ops method > - for most Aarch64 targets. */ > - > -static void > -aarch64_ravenscar_store_registers (struct regcache *regcache, int regnum) > -{ > - aarch64_ravenscar_generic_store_registers > - (&aarch64_reg_info, regcache, regnum); > -} > + void fetch_registers (struct regcache *regcache, int regnum) override > + { > + aarch64_ravenscar_generic_fetch_registers > + (&aarch64_reg_info, regcache, regnum); > + } > + > + void store_registers (struct regcache *regcache, int regnum) override > + { > + aarch64_ravenscar_generic_store_registers > + (&aarch64_reg_info, regcache, regnum); > + } > + > + void prepare_to_store (struct regcache *) override > + { > + } > +}; > > /* The ravenscar_arch_ops vector for most Aarch64 targets. */ > > -static struct ravenscar_arch_ops aarch64_ravenscar_ops = > -{ > - aarch64_ravenscar_fetch_registers, > - aarch64_ravenscar_store_registers, > - aarch64_ravenscar_generic_prepare_to_store > -}; > +static struct aarch64_ravenscar_ops aarch64_ravenscar_ops; > > /* Register aarch64_ravenscar_ops in GDBARCH. */ > > diff --git a/gdb/ppc-ravenscar-thread.c b/gdb/ppc-ravenscar-thread.c > index eca80c534a5..919b32c94f4 100644 > --- a/gdb/ppc-ravenscar-thread.c > +++ b/gdb/ppc-ravenscar-thread.c > @@ -169,15 +169,6 @@ ppc_ravenscar_generic_fetch_registers > } > } > > -/* to_prepare_to_store when inferior_ptid is different from the running > - thread. */ > - > -static void > -ppc_ravenscar_generic_prepare_to_store (struct regcache *regcache) > -{ > - /* Nothing to do. */ > -} > - > /* to_store_registers when inferior_ptid is different from the running > thread. */ > > @@ -211,32 +202,31 @@ static const struct ravenscar_reg_info ppc_reg_info = > ARRAY_SIZE (powerpc_context_offsets), > }; > > -/* Implement the to_fetch_registers ravenscar_arch_ops method > - for most PowerPC targets. */ > +struct ppc_ravenscar_powerpc_ops : public ravenscar_arch_ops > +{ > + void fetch_registers (struct regcache *, int) override; > + void store_registers (struct regcache *, int) override; > + > + void prepare_to_store (struct regcache *) override > + { > + } > +}; > > -static void > -ppc_ravenscar_powerpc_fetch_registers (struct regcache *regcache, int regnum) > +void > +ppc_ravenscar_powerpc_ops::fetch_registers (struct regcache *regcache, int regnum) > { > ppc_ravenscar_generic_fetch_registers (&ppc_reg_info, regcache, regnum); > } > > -/* Implement the to_store_registers ravenscar_arch_ops method > - for most PowerPC targets. */ > - > -static void > -ppc_ravenscar_powerpc_store_registers (struct regcache *regcache, int regnum) > +void > +ppc_ravenscar_powerpc_ops::store_registers (struct regcache *regcache, int regnum) > { > ppc_ravenscar_generic_store_registers (&ppc_reg_info, regcache, regnum); > } > > /* The ravenscar_arch_ops vector for most PowerPC targets. */ > > -static struct ravenscar_arch_ops ppc_ravenscar_powerpc_ops = > -{ > - ppc_ravenscar_powerpc_fetch_registers, > - ppc_ravenscar_powerpc_store_registers, > - ppc_ravenscar_generic_prepare_to_store > -}; > +static struct ppc_ravenscar_powerpc_ops ppc_ravenscar_powerpc_ops; > > /* Register ppc_ravenscar_powerpc_ops in GDBARCH. */ > > @@ -254,11 +244,18 @@ static const struct ravenscar_reg_info e500_reg_info = > ARRAY_SIZE (e500_context_offsets), > }; > > -/* Implement the to_fetch_registers ravenscar_arch_ops method > - for E500 targets. */ > +struct ppc_ravenscar_e500_ops : public ravenscar_arch_ops > +{ > + void fetch_registers (struct regcache *, int) override; > + void store_registers (struct regcache *, int) override; > > -static void > -ppc_ravenscar_e500_fetch_registers (struct regcache *regcache, int regnum) > + void prepare_to_store (struct regcache *) override > + { > + } > +}; > + > +void > +ppc_ravenscar_e500_ops::fetch_registers (struct regcache *regcache, int regnum) > { > ppc_ravenscar_generic_fetch_registers (&e500_reg_info, regcache, regnum); > } > @@ -266,20 +263,15 @@ ppc_ravenscar_e500_fetch_registers (struct regcache *regcache, int regnum) > /* Implement the to_store_registers ravenscar_arch_ops method > for E500 targets. */ > > -static void > -ppc_ravenscar_e500_store_registers (struct regcache *regcache, int regnum) > +void > +ppc_ravenscar_e500_ops::store_registers (struct regcache *regcache, int regnum) > { > ppc_ravenscar_generic_store_registers (&e500_reg_info, regcache, regnum); > } > > /* The ravenscar_arch_ops vector for E500 targets. */ > > -static struct ravenscar_arch_ops ppc_ravenscar_e500_ops = > -{ > - ppc_ravenscar_e500_fetch_registers, > - ppc_ravenscar_e500_store_registers, > - ppc_ravenscar_generic_prepare_to_store > -}; > +static struct ppc_ravenscar_e500_ops ppc_ravenscar_e500_ops; > > /* Register ppc_ravenscar_e500_ops in GDBARCH. */ > > diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c > index 9d708fd8581..0dc50a41429 100644 > --- a/gdb/ravenscar-thread.c > +++ b/gdb/ravenscar-thread.c > @@ -414,7 +414,7 @@ ravenscar_thread_target::fetch_registers (struct regcache *regcache, int regnum) > struct ravenscar_arch_ops *arch_ops > = gdbarch_ravenscar_ops (gdbarch); > > - arch_ops->to_fetch_registers (regcache, regnum); > + arch_ops->fetch_registers (regcache, regnum); > } > else > beneath ()->fetch_registers (regcache, regnum); > @@ -434,7 +434,7 @@ ravenscar_thread_target::store_registers (struct regcache *regcache, > struct ravenscar_arch_ops *arch_ops > = gdbarch_ravenscar_ops (gdbarch); > > - arch_ops->to_store_registers (regcache, regnum); > + arch_ops->store_registers (regcache, regnum); > } > else > beneath ()->store_registers (regcache, regnum); > @@ -453,7 +453,7 @@ ravenscar_thread_target::prepare_to_store (struct regcache *regcache) > struct ravenscar_arch_ops *arch_ops > = gdbarch_ravenscar_ops (gdbarch); > > - arch_ops->to_prepare_to_store (regcache); > + arch_ops->prepare_to_store (regcache); > } > else > beneath ()->prepare_to_store (regcache); > diff --git a/gdb/ravenscar-thread.h b/gdb/ravenscar-thread.h > index 8aab0a124f2..f0c163c5f0b 100644 > --- a/gdb/ravenscar-thread.h > +++ b/gdb/ravenscar-thread.h > @@ -24,9 +24,13 @@ > > struct ravenscar_arch_ops > { > - void (*to_fetch_registers) (struct regcache *, int); > - void (*to_store_registers) (struct regcache *, int); > - void (*to_prepare_to_store) (struct regcache *); > + virtual ~ravenscar_arch_ops () > + { > + } > + > + virtual void fetch_registers (struct regcache *, int) = 0; > + virtual void store_registers (struct regcache *, int) = 0; > + virtual void prepare_to_store (struct regcache *) = 0; > }; > > #endif /* !defined (RAVENSCAR_THREAD_H) */ > diff --git a/gdb/sparc-ravenscar-thread.c b/gdb/sparc-ravenscar-thread.c > index e09e453eabf..4b26f55490c 100644 > --- a/gdb/sparc-ravenscar-thread.c > +++ b/gdb/sparc-ravenscar-thread.c > @@ -25,11 +25,12 @@ > #include "ravenscar-thread.h" > #include "sparc-ravenscar-thread.h" > > -static void sparc_ravenscar_fetch_registers (struct regcache *regcache, > - int regnum); > -static void sparc_ravenscar_store_registers (struct regcache *regcache, > - int regnum); > -static void sparc_ravenscar_prepare_to_store (struct regcache *regcache); > +struct sparc_ravenscar_ops : public ravenscar_arch_ops > +{ > + void fetch_registers (struct regcache *, int) override; > + void store_registers (struct regcache *, int) override; > + void prepare_to_store (struct regcache *) override; > +}; > > /* Register offsets from a referenced address (exempli gratia the > Thread_Descriptor). The referenced address depends on the register > @@ -100,8 +101,8 @@ register_in_thread_descriptor_p (int regnum) > /* to_fetch_registers when inferior_ptid is different from the running > thread. */ > > -static void > -sparc_ravenscar_fetch_registers (struct regcache *regcache, int regnum) > +void > +sparc_ravenscar_ops::fetch_registers (struct regcache *regcache, int regnum) > { > struct gdbarch *gdbarch = regcache->arch (); > const int sp_regnum = gdbarch_sp_regnum (gdbarch); > @@ -143,8 +144,8 @@ sparc_ravenscar_fetch_registers (struct regcache *regcache, int regnum) > /* to_prepare_to_store when inferior_ptid is different from the running > thread. */ > > -static void > -sparc_ravenscar_prepare_to_store (struct regcache *regcache) > +void > +sparc_ravenscar_ops::prepare_to_store (struct regcache *regcache) > { > /* Nothing to do. */ > } > @@ -152,8 +153,8 @@ sparc_ravenscar_prepare_to_store (struct regcache *regcache) > /* to_store_registers when inferior_ptid is different from the running > thread. */ > > -static void > -sparc_ravenscar_store_registers (struct regcache *regcache, int regnum) > +void > +sparc_ravenscar_ops::store_registers (struct regcache *regcache, int regnum) > { > struct gdbarch *gdbarch = regcache->arch (); > int buf_size = register_size (gdbarch, regnum); > @@ -178,12 +179,7 @@ sparc_ravenscar_store_registers (struct regcache *regcache, int regnum) > buf_size); > } > > -static struct ravenscar_arch_ops sparc_ravenscar_ops = > -{ > - sparc_ravenscar_fetch_registers, > - sparc_ravenscar_store_registers, > - sparc_ravenscar_prepare_to_store > -}; > +static struct sparc_ravenscar_ops sparc_ravenscar_ops; > > /* Register ravenscar_arch_ops in GDBARCH. */ > >