From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5472 invoked by alias); 9 Jan 2017 20:15:01 -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 5430 invoked by uid 89); 9 Jan 2017 20:15:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=AWL,BAYES_50,KAM_LOTSOFHASH,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 spammy=PACKET_P, regcache_descr, regcachec, packet_support X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Jan 2017 20:14:50 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cQgLA-00038i-Ve from Luis_Gustavo@mentor.com ; Mon, 09 Jan 2017 12:14:49 -0800 Received: from [172.30.7.42] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Mon, 9 Jan 2017 12:14:45 -0800 Subject: Re: [PATCH 3/3] Calculate max register size References: <000B14E8-6B46-4C03-B70F-CC5E50BCFBCF@arm.com> To: Alan Hayward , "gdb-patches@sourceware.org" CC: nd Reply-To: Luis Machado From: Luis Machado Message-ID: Date: Mon, 09 Jan 2017 20:15:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <000B14E8-6B46-4C03-B70F-CC5E50BCFBCF@arm.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00137.txt.bz2 On 01/09/2017 04:58 AM, Alan Hayward wrote: > Aarch64 SVE requires a max register size of 256. The current max size in gdb > is 64. This is part of a series demonstrating the replacement of > MAX_REGISTER_SIZE. > > In cases where a buffer is created to be used multiple times to hold different > registers, then the maximum register size is required. Add a max register value > to the regcache which is calculated on initialization. > > This patch is restricted to remote.c and regcache.c. > Follow on patches will expand to other files. > > Tested on x86. > Ok to commit? > > Thanks, > Alan. > > 2017-01-09 Alan Hayward > > * regcache.c (struct regcache_descr): Add max_register_size > (max_register_size): New. > (init_regcache_descr): Find max register size. > (regcache_save): Use max_register_size. > (regcache_restore): Likewise. > (regcache_dump): Likewise. > * regcache.h (max_register_size): New. > * remote.c (remote_prepare_to_store): Allocate buffer. > > > diff --git a/gdb/regcache.h b/gdb/regcache.h > index e5a7cf553279b8cc0d546ec1b8274cbf97e246d5..4db9517a9dd464d9c43be2af0573b767b86bfb56 100644 > --- a/gdb/regcache.h > +++ b/gdb/regcache.h > @@ -202,6 +202,9 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum); > > extern int register_size (struct gdbarch *gdbarch, int regnum); > > +/* Return the size of the largest register. */ > + Spurious newline. > +extern long max_register_size (struct gdbarch *gdbarch); > > /* Save/restore a register cache. The set of registers saved / > restored into the DST regcache determined by the save_reggroup / > diff --git a/gdb/regcache.c b/gdb/regcache.c > index 9d28aa2c2114e0f1c52758bb2fbe9669a329c13e..46d017c7b2abcb18c9cdda005749071328735dbd 100644 > --- a/gdb/regcache.c > +++ b/gdb/regcache.c > @@ -73,6 +73,9 @@ struct regcache_descr > > /* Cached table containing the type of each register. */ > struct type **register_type; > + > + /* Size of the largest register. */ > + long max_register_size; Is this ever negative? Why not make it unsigned? > }; > > static void * > @@ -125,7 +128,9 @@ init_regcache_descr (struct gdbarch *gdbarch) > descr->sizeof_register[i] = TYPE_LENGTH (descr->register_type[i]); > descr->register_offset[i] = offset; > offset += descr->sizeof_register[i]; > - gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]); > + gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]); > + descr->max_register_size = std::max (descr->max_register_size, > + descr->sizeof_register[i]); > } > /* Set the real size of the raw register cache buffer. */ > descr->sizeof_raw_registers = offset; > @@ -135,7 +140,9 @@ init_regcache_descr (struct gdbarch *gdbarch) > descr->sizeof_register[i] = TYPE_LENGTH (descr->register_type[i]); > descr->register_offset[i] = offset; > offset += descr->sizeof_register[i]; > - gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]); > + gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]); > + descr->max_register_size = std::max (descr->max_register_size, > + descr->sizeof_register[i]); > } > /* Set the real size of the readonly register cache buffer. */ > descr->sizeof_cooked_registers = offset; > @@ -187,6 +194,13 @@ regcache_register_size (const struct regcache *regcache, int n) > return register_size (get_regcache_arch (regcache), n); > } > > +long Same as above, is this ever negative? > +max_register_size (struct gdbarch *gdbarch) > +{ > + struct regcache_descr *descr = regcache_descr (gdbarch); Is descr ever NULL? > + return descr->max_register_size; > +} > + > /* The register cache for storing raw register values. */ > > struct regcache > @@ -327,7 +341,7 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read, > void *src) > { > struct gdbarch *gdbarch = dst->descr->gdbarch; > - gdb_byte buf[MAX_REGISTER_SIZE]; > + gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch)); > int regnum; > > /* The DST should be `read-only', if it wasn't then the save would > @@ -369,7 +383,7 @@ regcache_restore (struct regcache *dst, > void *cooked_read_context) > { > struct gdbarch *gdbarch = dst->descr->gdbarch; > - gdb_byte buf[MAX_REGISTER_SIZE]; > + gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch)); > int regnum; > > /* The dst had better not be read-only. If it is, the `restore' > @@ -1279,7 +1293,7 @@ regcache_dump (struct regcache *regcache, struct ui_file *file, > int footnote_register_offset = 0; > int footnote_register_type_name_null = 0; > long register_offset = 0; > - gdb_byte buf[MAX_REGISTER_SIZE]; > + gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch)); > > #if 0 > fprintf_unfiltered (file, "nr_raw_registers %d\n", > diff --git a/gdb/remote.c b/gdb/remote.c > index 9247d43b094925ff397eb36b450eaba521adfc99..86856e6a6aba1967faaa8ef547f8a48fcc63c383 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -7752,9 +7752,10 @@ remote_fetch_registers (struct target_ops *ops, > static void > remote_prepare_to_store (struct target_ops *self, struct regcache *regcache) > { > + struct gdbarch *gdbarch = get_regcache_arch (regcache); > struct remote_arch_state *rsa = get_remote_arch_state (); > int i; > - gdb_byte buf[MAX_REGISTER_SIZE]; > + gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch)); > > /* Make sure the entire registers array is valid. */ > switch (packet_support (PACKET_P)) > > > My comment is the same as 2/3. Should we use a different data structure that can grow/shrink as one wishes?