This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Alan Hayward <Alan dot Hayward at arm dot com>
- Cc: Joel Brobecker <brobecker at adacore dot com>, Pedro Alves <palves at redhat dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Fri, 3 Feb 2017 10:28:19 +0000
- Subject: Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE
- Authentication-results: sourceware.org; auth=none
- References: <7CF07197-4FED-4970-BB4B-2FE828E29A63@arm.com> <45e3a5e1-a9aa-1bc0-5d08-526b89fc458e@redhat.com> <20170201124123.GA27498@E107787-LIN> <dcfc96df-1917-8371-9069-4276dc3141fa@redhat.com> <20170202094012.dge4r6rsl2skdrii@adacore.com> <D06782D9-66E9-4194-86C4-275341686DB8@arm.com>
On Fri, Feb 3, 2017 at 9:59 AM, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>> On 2 Feb 2017, at 09:40, Joel Brobecker <brobecker@adacore.com> wrote:
>>
>>> #2 - Switch to heap allocation
>>>
>>> Or std::vector or something like that that hides it. It's not clear
>>> whether that would have a noticeable performance impact.
>>
>> I would try that. I think using one of the standard C++ classes
>> looks a little more attractive to me, and would only consider
>> the lambda functions if we can show a noticeable performance
>> impact. Those two are not exclusive, by the way, but in the past,
>> we've always frowned on calls to alloca in a loop, and using
>> a xmalloc+cleanup combination has never been an issue in my cases.
>> I'd imagine that a standard C++ memory management class would be
>> fast enough for those same situations.
>>
>> --
>> Joel
>
> We're not allocating much space, so I'd hope std::vector didn't cause much
> of a slowdown. Also, the allocas with lamdba's approach feels a little
> overcomplicated.
>
> Reworking my patch that adds max_register_size (), I've replaced the allocas
> with std::vector.
>
> This patch ok? If people are happy, I'll then rework the larger patch.
I don't think we have to replace all MAX_REGISTER_SIZE with std::vector.
MAX_REGISTER_SIZE is mostly used in arch-dependent code (*-tdep.c
and *-nat.c), where the register size or max register size is known. For
example, MAX_REGISTER_SIZE is used only once in arm-tdep.c, and
it can be replaced with FP_REGISTER_SIZE, because 'buf' is to get the
contents for FPA register. Similarly, MAX_REGISTER_SIZE is used three
times in aarch64-tdep.c, all of them can be repalced by V_REGISTER_SIZE.
Also, MAX_REGISTER_SIZE can be replaced by
I386_MAX_REGISTER_SIZE in i386-tdep.c. I would like to examine the
usages of MAX_REGISTER_SIZE in each target-dependent code, and
replace MAX_REGISTER_SIZE with known constants as much as we can.
I don't think anyone has objections on replacing one constant
MAX_REGISTER_SIZE with other smaller constants :)
Then, let us discuss how to remove MAX_REGISTER_SIZE from
arch-independent code after all above is done.
--
Yao (齐尧)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 801c03d..1f82187 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1982,7 +1982,7 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
for (i = 0; i < elements; i++)
{
int regno = AARCH64_V0_REGNUM + i;
- bfd_byte tmpbuf[MAX_REGISTER_SIZE];
+ bfd_byte tmpbuf[V_REGISTER_SIZE];
if (aarch64_debug)
{
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0ae311f..42a39dc 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -69,6 +69,10 @@
#include "features/arm/arm-with-vfpv3.c"
#include "features/arm/arm-with-neon.c"
+#if GDB_SELF_TEST
+#include "selftest.h"
+#endif
+
static int arm_debug;
/* Macros for setting and testing a bit in a minimal symbol that marks
@@ -4237,6 +4241,23 @@ convert_to_extended (const struct floatformat *fmt, void *dbl, const void *ptr,
&d, dbl);
}
+#if GDB_SELF_TEST
+
+namespace selftests {
+
+static void
+arm_floatformat_test (void)
+{
+ SELF_CHECK (floatformat_totalsize_bytes (&floatformat_arm_ext_big)
+ == FP_REGISTER_SIZE);
+ SELF_CHECK (floatformat_totalsize_bytes (&floatformat_arm_ext_littlebyte_bigword)
+ == FP_REGISTER_SIZE);
+}
+
+} // namespace selftests
+
+#endif /* GDB_SELF_TEST */
+
/* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand
the buffer to be NEW_LEN bytes ending at ENDADDR. Return
NULL if an error occurs. BUF is freed. */
@@ -8153,11 +8174,10 @@ arm_store_return_value (struct type *type, struct regcache *regs,
if (TYPE_CODE (type) == TYPE_CODE_FLT)
{
- gdb_byte buf[MAX_REGISTER_SIZE];
-
switch (gdbarch_tdep (gdbarch)->fp_model)
{
case ARM_FLOAT_FPA:
+ gdb_byte buf[FP_REGISTER_SIZE];
convert_to_extended (floatformat_from_type (type), buf, valbuf,
gdbarch_byte_order (gdbarch));
@@ -9717,6 +9737,10 @@ vfp - VFP co-processor."),
NULL,
NULL, /* FIXME: i18n: "ARM debugging is %s. */
&setdebuglist, &showdebuglist);
+
+#if GDB_SELF_TEST
+ register_self_test (selftests::arm_floatformat_test);
+#endif
}
/* ARM-reversible process record data structures. */